From d8d86fdafcc58b52f8d575243c316e2fa2de9b5a Mon Sep 17 00:00:00 2001 From: "linh.doan" Date: Sun, 13 Sep 2026 04:38:23 +0700 Subject: [PATCH] docs(prd): note #349 revision-stamp caveat for linked-worktree builds (#352) Per AGENTS.md the PRD references issues that matter for status. #349's footer claims 'a stale binary is loud'; that holds for normal-checkout builds, but Go's -buildvcs mis-stamps vcs.revision inside a linked worktree (proven A/B at 8ca0688), so worktree-built binaries write a wrong release-row revision and false-trip RunLoop's guard. Track that as #352 and link it; regenerate docs/PRD.html so the styled mirror matches PRD.md. --- docs/PRD.html | 22 +++++++++++++++++++++- docs/PRD.md | 1 + 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/PRD.html b/docs/PRD.html index 882248c..ce0ec3d 100644 --- a/docs/PRD.html +++ b/docs/PRD.html @@ -4945,7 +4945,27 @@

23.1 Requirements

chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope.

-

*Last updated: 2026-09-13 (release-created rows carry +

*Last updated: 2026-09-13 (caveat on #349's revision stamp: +linked-worktree builds mis-stamp — #352) — while +verifying #349's landing we found Go's -buildvcs resolves +vcs.revision from the shared common gitdir in a linked +worktree, so a devagent binary built inside a per-task +worktree stamps the primary checkout's refs/heads/main tip +rather than its own HEAD (vcs.modified=true). Proven A/B at +the same commit 8ca0688: a plain clone stamps +8ca0688, a linked worktree stamps 50b03e8. +Scope: this is a latent defect confined to worktree-built +binaries — the shipped driver is built by +scripts/self-update.sh from the primary non-worktree +checkout (go build -trimpath ./cmd/devagent after +git pull --ff-only), so it stamps correctly and +RunLoop's stale-binary guard behaves as intended in the +live loop; only a manually-built worktree binary writes a wrong +release-created revision and false-trips the +(advisory-only) WARN. #349's "a stale binary is loud" holds for the +normal-checkout build path; the worktree edge is tracked in #352. *Last +updated: 2026-09-13 (release-created rows carry tag/sha again: PR #349's Revision stamp landed as a silent data-loss regression) — #349 added a Revision field to the Go ReleaseRecord but dropped diff --git a/docs/PRD.md b/docs/PRD.md index 3c2a5fc..c121673 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -1695,6 +1695,7 @@ existing driver with validation surfaces (test, command, telemetry, chaos schedule) so "the driver works perfectly" is a checkable claim, not a hope. --- +*Last updated: 2026-09-13 (caveat on #349's revision stamp: linked-worktree builds mis-stamp — [#352](https://github.com/FreePeak/devagent/issues/352)) — while verifying #349's landing we found Go's `-buildvcs` resolves `vcs.revision` from the shared common gitdir in a linked worktree, so a `devagent` binary built inside a per-task worktree stamps the primary checkout's `refs/heads/main` tip rather than its own HEAD (`vcs.modified=true`). Proven A/B at the same commit `8ca0688`: a plain clone stamps `8ca0688`, a linked worktree stamps `50b03e8`. Scope: this is a latent defect confined to *worktree-built* binaries — the shipped driver is built by `scripts/self-update.sh` from the primary non-worktree checkout (`go build -trimpath ./cmd/devagent` after `git pull --ff-only`), so it stamps correctly and `RunLoop`'s stale-binary guard behaves as intended in the live loop; only a manually-built worktree binary writes a wrong `release-created` revision and false-trips the (advisory-only) WARN. #349's "a stale binary is loud" holds for the normal-checkout build path; the worktree edge is tracked in #352. *Last updated: 2026-09-13 (release-created rows carry `tag`/`sha` again: PR #349's Revision stamp landed as a silent data-loss regression) — #349 added a `Revision` field to the Go `ReleaseRecord` but dropped `Tag`/`SHA` from the `record release` literal while still printing `(tag @ sha)` to stdout, so every Go-written release row persisted `"tag":""` `"sha":""` (the Q24 fields the record exists to carry) and no test covered the write site, so it shipped green. Restored both fields; `internal/cli/actions_release_test.go` now pins the CLI row shape (proven red on the regressed literal, green after); regenerated `docs/PRD.html` so the styled mirror matches `PRD.md`. *Last updated: 2026-09-13 (herdr orphan-pane reaping: both probes were dead, repaired as one change) — `devagent herdr-sweep --orphans` had never reaped a live orphan: `pane process-info` went out without `--session`, so herdr answered for its own default session and "no such pane" was every devagent pane's liveness, which left the FR-VIS-07 in-flight guard, the FR-VIS-02 roster upgrade (#317) and the orphan class — gated on a live foreground worker — inert at once; the owner probe was `herdr.*pane run .*`, a process that does not exist while a pane runs (`pane run` types the script into the pane's shell and exits; verified live parent chain `omp -> -zsh -> herdr --session server -> launchd`), so `PaneRunOwnerOrphaned` always took its "no owner CLI -> orphaned" exit and the driver spare behind it was unreachable. Each half alone is a regression — a working owner probe over an inert liveness probe closes live workers — so the fix lands atomically: `PaneForegroundWorker(cli, session, paneID)` scopes the probe (roster and sweep share it), the owner is now the surviving dispatcher (argv0-anchored `devagent`/`devagent-go` `task`/`pane-run`, optionally behind the driver's `timeout` wrapper, so a prompt quoting devagent cannot impersonate its own collector), non-worktree panes need positive per-pane dispatch evidence (the capture contract on the worker's fd 1/2), and the orphan class moved ahead of the roster spare. Reaping also fails closed: `psPidsMatching` and `pidAncestryCommands` now report whether the probe answered at all — pgrep's exit-1 "no match" is an answer (no collector, reap), an absent binary / the 5s cap / a rejected pattern / an unfinished ancestry walk is not (spare) — because a reaper that read "could not inspect" as "no collector" would close every live pane on such a host. Test seams: `DEVAGENT_SWEEP_OWNER_PIDS_JSON` is keyed by the pattern asked for and a miss now counts as an unanswered probe (never a clean no-match), `DEVAGENT_SWEEP_PANE_CAPTURE_JSON` supplies pane-keyed evidence — and because the 2026-09-13 bug survived a green stub suite, both shapes are pinned from outside: `TestProcFdsCaptureSeesRealCaptureContract` runs the real `lsof` probe against a live child holding `/devagent-herdr-/out` on fd 1/2 (skipped where lsof is absent), `TestPaneRunOwnerPatternMatchesRealDispatchShapes` checks `paneRunOwnerPattern` against the command lines `internal/loopdriver` really builds and against the transient `herdr pane run` shape it used to search for, and `TestOrphanClassGoesInertWhenProbesCannotAnswer` pins the spare-on-no-answer direction at the matrix level. `TestPsPidsMatchingAnswersOnlyWhenPgrepAnswers` pins the same split against real `pgrep` exit codes: 1 means no collector (reap), 2 means the probe could not run (spare). Docs corrected: FR-VIS-02/07/10, §18 Q23, `docs/HERDR.md` sweep safety. *Last updated: 2026-09-12 (build-revision self-identification: a stale driver binary is loud, and Go-written release rows name their writer) — loops 290/291 burned implement-and-gate cycles while the driver seat ran a binary built from an older commit, and nothing announced the mismatch. `internal/version.Revision()` (new, memoized) reads `debug.ReadBuildInfo`'s `vcs.revision` and returns `version.RevisionUnknown` (`"unknown"`) when the build carries no stamp — go-test binaries carry none (probed 2026-09-12), so `version.RevisionOverride` is the injection seam. `RunLoop` compares `Revision()` against `git rev-parse HEAD` before the first iteration and warns `[loop] WARN: stale binary` when the revision is unknown or differs, naming both SHAs (advisory only: the loop still runs). `ledger.ReleaseRecord` gains `revision` (stamped at the `record release` write site; `omitempty` keeps unstamped Go rows byte-compatible with the TS writer — Node readers tolerate the extra key, pinned by `TestSchemaDriftUnknownFieldsTolerated`), and `devagent --version` prints `0.1.0 (rev )`. Pinned by `TestRevision` (sentinel, memoization, override) and `TestRunLoopStaleBinaryWarns` (unknown warn, mismatch warn, matching-revision silence, rc 0 on every arm).