Skip to content

feat(gs)!: replace bin/gs and bin/gs-official with one deterministic Go gs - #119

Merged
barbatdev merged 5 commits into
mainfrom
feat/gs-go
Nov 4, 2026
Merged

barbatdev merged 5 commits into
mainfrom
feat/gs-go

Conversation

@barbatdev

Copy link
Copy Markdown
Owner

Why

Two different gs scripts (bin/gs, bin/gs-official) disagreed on how ~/.local/bin should look (symlinks vs regular files), and gentle-ai/gentle-shell could go install Engram v1 over ours. Hosts drifted and broke.

What

One Go binary in gs/, same on Mac, agent-dev and agent-gentle:

  • gs sync: gentle-ai and engram built only by gs from a pinned commit (GitHub main via gh, or the clean clones on workshop), verified (engram instance-id), installed as regular files via atomic rename; every other step runs with GOBIN in a quarantine dir; gentle-shell upgrade only when behind main; Engram service restart on change; ends with the gs doctor checks (INCOMPLETE + exit 1 on failure); rebuilds gs itself from the clean dotfiles clone on main.
  • gs doctor: read-only host report.
  • Worktree commands ported: pr, issue, new, review, base, run, ls, done.
  • install.sh builds gs; CI runs gofmt/vet/test; docs in docs/gs.md. Old scripts and their tests removed.

Tests

cd gs && go test ./... (offline, temp HOME, stubs). Real Mac run of gs sync: pinned builds installed, Engram restarted, doctor checks ok.

Feature log: odd/tasks/gs-go-binary.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01A9D7phtzRBjJGKsHtaiVWj

One binary for every host. gs builds gentle-ai and engram itself from a
pinned commit (GitHub main through gh, or the clean clone on workshop
hosts), verifies the build (engram instance-id) and installs regular files
in ~/.local/bin with an atomic rename. Every other sync step runs with
GOBIN in a quarantine dir, so stray go installs can never replace them.
The sync ends with the same checks as gs doctor.
gs sync rebuilds gs itself from the clean dotfiles clone on main;
install.sh builds it; CI runs gofmt, go vet and go test. Docs move to
docs/gs.md.
@barbatdev
barbatdev merged commit 2bc0c47 into main Nov 4, 2026
1 check passed
@barbatdev

Copy link
Copy Markdown
Owner Author

Fleet review (mimo)

Lens: requirements. Second-line review of #119 against the acceptance criteria stated in the PR body and in odd/tasks/gs-go-binary.md (S1-S5): one deterministic Go binary in gs/ with sync/doctor and the ported worktree commands, installed by install.sh, covered by gofmt/vet/test in CI, documented in docs/gs.md, and the old scripts plus their tests removed. Verified from the diff: dispatch (gs/main.go) routes sync, doctor, pr, issue, new, review, base, run, ls, done, help and the plain launch; cmdSync pins gentle-ai/engram to one commit (gh api on consumer, clean clones on workshop), installs them as regular files through an atomic rename after verifying the build (engram instance-id), runs every other child with GOBIN in a quarantine dir, upgrades gentle-shell only when behind gentle-pi main, restarts the Engram service on change or stale process, pulls the dotfiles clone only when clean and on main and rebuilds gs from it, and ends with the shared checkHost/doctor report that prints INCOMPLETE and exits 1; gs doctor is read-only; bin/gs, bin/gs-official, scripts/test-gs-sync.sh, tests/gs_test.sh, tests/gs_official_test.py, docs/gs-official.md and docs/gs-sync.md are gone; install.sh builds gs atomically into ~/.local/bin/gs; CI runs gofmt/vet/test in gs/. No functional reference to the removed scripts survives in install.sh, zsh/, scripts/ or the workflow (grep of the head tree), and gofmt -l on gs/ is clean. No blocking finding: every criterion the diff can satisfy is satisfied, and the remaining items are precision/rollout questions rather than defects with a concrete failure sequence in the diff.

Suggestion (5)

  • gs done doc does not match the deletion rule in the code (docs/gs.md:57): docs/gs.md:57 says 'gs done now also drops untracked odd/tasks/ records when the whole odd/ directory is untracked'. gs/worktree.go:395-407 instead drops every untracked '?? odd/tasks/' entry whenever the worktree has no other leftovers, regardless of whether odd/ contains tracked files (odd/ does contain tracked records in this repository), and it does so only after refusing when any other status line exists. The documented condition and the implemented condition are different predicates over the same operation.
  • syncedFiles indexes Fields(rest)[0] with no guard (gs/sync.go:326): syncedFiles matches the prefix 'Sync actions executed: ' and then reads strings.Fields(rest)[0]. If gentle-ai ever prints that line with the marker and nothing after it, Fields returns an empty slice and gs panics. The path is untested: no test feeds a 'Sync actions executed:' line to the real parser (the stubs log to a file instead), so the parser only runs against live gentle-ai output.
  • The legacy-retirement script now retires the replacement binary (scripts/retire-gentle-legacy.sh:26): scripts/retire-gentle-legacy.sh was not updated by this PR: its paths list still retires '.local/bin/gs' and its usage text still tells the operator to 'persist this alias in your dotfiles: alias gs=gentle-shell'. After this PR that path holds the Go binary this PR installs, so the runbook removes the replacement and replaces it with an alias that bypasses sync, doctor and every worktree command - the exact two-sources-of-truth drift the PR exists to end.
  • The new Go step depends on the runner's preinstalled toolchain (.github/workflows/ci.yml:72): The 'Check gs (Go)' step runs go version/gofmt/vet/test without an actions/setup-go step and without a go-version pin, so it uses whatever Go ships in ubuntu-latest, while gs/go.mod declares 'go 1.22'. The step works today (ubuntu images ship Go) but the version that vets and tests the new binary is not recorded or pinned anywhere in the workflow.
  • Dead links to the deleted gs-official script and doc (docs/gentle-official-migration.md:269): The PR adds a top-of-file note that the mentions below are history and repairs the link on line 27, but line 269 still links 'bin/gs-official' and 'doc' - both files are deleted by this PR, so both links 404 in the rendered document. Same file, same section, one of the two links was fixed and the other was not.

Question (1)

  • The 3-host equivalence claim is out of the diff (T5 rollout pending) (odd/tasks/gs-go-binary.md:22): The PR body presents the binary as 'same on Mac, agent-dev and agent-gentle', but its own task table marks T5 (S2) 'code done, rollout pending'. Nothing in the diff can confirm S2: consumer vs workshop detection (gs/workshop.go isWorkshop), the self-rebuild and the doctor checks are only exercised by offline stub tests, and no post-rollout run of gs sync on the three hosts is recorded. This does not contradict any implemented criterion; it asks for the evidence the criterion needs before the claim is treated as met.

Verdict: ready to merge

@barbatdev

Copy link
Copy Markdown
Owner Author

Post-merge QA (glm)

Lens: requirements + adversarial. El gs en Go es determinista y reemplaza bin/gs y bin/gs-official en un solo binario; los tests cubren los caminos principales y la salida binaria. Sin hallazgos.

No findings.

Verdict: clean

@barbatdev

Copy link
Copy Markdown
Owner Author

Post-merge QA (deepseek)

Lens: evidence + test design. QA of the merge that landed: gs sync now builds gentle-ai and engram itself from a single resolved commit, stages the build, verifies it (engram instance-id, <bin> version) and installs it with an atomic rename over a possibly symlinked target; every other child runs with GOBIN in a quarantine dir; the run ends with the same checks as gs doctor and returns INCOMPLETE. I traced the main paths (buildAndInstall, installAtomic, syncDotfiles/rebuildSelf, upgradeShell/shellFresh, cae-free doctor checks) and found no wrong-behaviour path: the same-content fast path is only taken for a regular file at the destination, the temp file is created next to the destination so the rename never crosses filesystems, and a destination symlink is replaced rather than followed. The ChildEnv list grows a GOBIN=<stage> entry per build and appends GOBIN=<quarantine> after it, which is correct because os/exec keeps the last value of a duplicated key. Only evidence note below: the consumer build path is exercised through a shell stub, so the real go install contract (module path, version pin, GOBIN precedence) rests on the manual Mac run quoted in the body, which is not reproducible from the PR.

Suggestion (1)

  • Real go install behaviour is stubbed, so the pin and GOBIN precedence are only covered by the manual run (gs/sync_test.go:30): consumerHost's go stub writes $GOBIN/<name> straight from mkdir -p "$GOBIN" and resolves GOBIN through the shell. That keeps the suite offline, but it cannot catch the two things the design depends on: that go install <module>/<pkg>@<sha> actually resolves for the v4/v3 module paths recorded in Managed, and that a real Go child resolves the duplicated GOBIN key the same way the assertions assume. Both were validated once by the "Real Mac run of gs sync" quoted in the body, which the repository cannot replay. A single opt-in (build-tagged, skipped by default) test that runs go list -m <import>@<sha> against the network would record that the pinned import paths resolve without turning the default suite into a network test.

Verdict: clean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant