From 3c01fae81e8027e8298d99a088696ab5c9343cc4 Mon Sep 17 00:00:00 2001 From: "linh.doan" Date: Sat, 12 Sep 2026 23:04:05 +0700 Subject: [PATCH] devagent(TASK-mtyis3ci-7gvj): auto-cleanup snapshot --- docs/PRD.md | 1 + internal/cli/actions_ledger.go | 18 ++++---- internal/cli/root.go | 8 ++-- internal/ledger/ledger.go | 25 ++++++----- internal/loopdriver/run.go | 11 +++++ internal/loopdriver/run_test.go | 74 ++++++++++++++++++++++++++++++++ internal/version/version.go | 43 +++++++++++++++++++ internal/version/version_test.go | 22 ++++++++++ 8 files changed, 180 insertions(+), 22 deletions(-) diff --git a/docs/PRD.md b/docs/PRD.md index 8ab3fc75..6e70186c 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -1696,6 +1696,7 @@ schedule) so "the driver works perfectly" is a checkable claim, not a hope. --- *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). *Last updated: 2026-09-12 (the rescue reaches CLOSED-but-unmerged pull requests: a goal-named closed PR is reopened and merged, or lands nothing) — the driver-side verify-and-merge rescue, and the goal-text derivation feeding it, only accepted subjects that read `OPEN` before the dispatch, so the false closes were unreachable: PR #346/#347 (auto-closed by the zombie sweep's `BaseBranchGone(main)` transport-noise bug minutes after they opened, green and mergeable) and loop 282's #345 each recorded `no-pr` with a landable branch left closed. `prView` now decodes `baseRefName`/`headRefName`, and a goal-named pull request that is CLOSED, unmerged, based on `main` with its head ref still on origin becomes the merge subject: `verifyAndMergeRescue` reopens it (`gh pr reopen`, repo-scoped, 60s wall, drain-tolerant), re-reads the state (gh's reopen is verified, never believed) and only then merges through the existing `mergePRBounded` pipeline. Everything else lands nothing — another/superseded base, a head ref gone from origin, a merge stamp, a reopen gh refuses — so no ship is ever fabricated. Pinned by `TestRunLoopClosedUnmergedGoalPRReopenedAndMerged`, `TestRunLoopClosedGoalPRLandsNothingWhenNotReopenable` and `TestVerifyAndMergeRescueRefusesNonReopenablePR` (`internal/loopdriver/run.go`, `issues.go`). *Last updated: 2026-09-12 (both green heads landed straight onto main — no pull request, no re-implementation) — the zombie sweep was still armed when PR #347 and PR #346 were opened, and its `BaseBranchGone(main)` transport-noise bug auto-closed both within minutes, so the two verified heads were landed straight onto main instead: `1f2ef8c` (`devagent/TASK-mtxyflr1-fq6y` — three-valued hygiene probes plus the auto-close age floor) fast-forwarded main, then `635214d` (`devagent/TASK-mtxwr39q-nva6` — the landing-certification merge window) merged on top. The code paths are disjoint (`internal/orchestrator` vs `internal/loopdriver`) and only `docs/PRD.*` overlapped. Neither PR was reopened or closed by this landing; both stay CLOSED, unmerged as pull requests. Verified on the merged tree: `go test ./...` green, including `TestAutoMergeTreatsBaseProbeNoiseAsAlive` and `TestGhNotFoundOnlyForRealGone`, and `git merge-base --is-ancestor` true for both heads. *Last updated: 2026-09-12 (zombie-PR hygiene probes are three-valued, with an auto-close age floor) — both gh probes behind the `pr-hygiene` sweep collapsed "gh never answered" into a decisive answer: `BaseBranchGone` returned `err != nil`, so a network hiccup, 5xx, or rate-limit response on the `branches/` lookup read as a dead base and the sweep CLOSED a live PR as `base-superseded` (PR #346: created 05:27:21Z, closed 05:28:37Z, 76s later, never merged); `landingCommitOnMain` returned empty on a failed or unparseable commit listing, so transport noise read as "no landing commit" and the PR was closed as `superseded` with an explicit "not shipped" comment. `ProbeBaseBranch` (internal/orchestrator/autopr.go) now answers `BaseAlive` / `BaseGone` / `BaseUnknown`, judging "gone" only on gh's own not-found stderr (`ghNotFound` reads the diagnostic, never the argv-embedding message, whose ref names may themselves contain "404"; `GhError.Code` is the gh exit code and is never consulted), and `landingCommitOnMain` returns `(sha, ok)` with `ok` false whenever the listing failed. On `unknown` the sweep records `untouched`/`base-unknown` or `untouched`/`landing-unknown` and acts on nothing; the merge-queue gate treats `BaseUnknown` as alive, so a probe hiccup no longer parks a mergeable PR. Both auto-close arms now also wait out an age floor: `closeAgeFloorHours` (24h) capped by `prHygiene.graceHours`, with an unknown PR age treated as floored — a fresh PR is reported `untouched`/`base-superseded-age-floor` or `untouched`/`landing-age-floor` and the `--grace-hours` help states the floor. Regression-pinned by `TestSweepTaskPrHygiene` (base-probe noise, fresh and unknown-age floors), `TestSweepTaskPrHygieneLandingEvidence` (landing noise, unparseable listing, landing floor), `TestAutoMergeTreatsBaseProbeNoiseAsAlive`, and `TestGhNotFoundOnlyForRealGone` — each fails against the pre-fix behavior.* diff --git a/internal/cli/actions_ledger.go b/internal/cli/actions_ledger.go index c31f584a..9147b8a8 100644 --- a/internal/cli/actions_ledger.go +++ b/internal/cli/actions_ledger.go @@ -13,6 +13,7 @@ import ( "github.com/FreePeak/devagent/internal/ledger" "github.com/FreePeak/devagent/internal/tui" + versionpkg "github.com/FreePeak/devagent/internal/version" "github.com/spf13/cobra" ) @@ -231,15 +232,14 @@ func recordCommand() *cobra.Command { } version := strings.TrimPrefix(tag, "v") ledger.AppendReleaseRecord(repo, ledger.ReleaseRecord{ - TS: time.Now().UTC().Format("2006-01-02T15:04:05.000Z07:00"), - Kind: "event", - TaskID: "release/" + version, - Attempt: 1, - Event: "release-created", - Tag: tag, - SHA: sha, - Version: version, - Source: source, + TS: time.Now().UTC().Format("2006-01-02T15:04:05.000Z07:00"), + Kind: "event", + TaskID: "release/" + version, + Attempt: 1, + Event: "release-created", + Version: version, + Revision: versionpkg.Revision(), + Source: source, }) fmt.Printf("recorded release-created %s (%s @ %s) -> .devagent/runs/orchestration/events.jsonl\n", version, tag, sha) return nil diff --git a/internal/cli/root.go b/internal/cli/root.go index 6f332e63..63acb77a 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -235,9 +235,11 @@ func parentNames(cmd *cobra.Command) []string { // NewRoot builds the full cobra tree. func NewRoot() *cobra.Command { root := &cobra.Command{ - Use: "devagent", - Short: "Autonomous backend delivery agent: ticket to tested PR", - Version: version.Version, + Use: "devagent", + Short: "Autonomous backend delivery agent: ticket to tested PR", + // The revision rides --version so a stale driver binary is + // diagnosable from one paste (loops 290/291 died on this class). + Version: version.Version + " (rev " + version.Revision() + ")", } root.SetVersionTemplate("{{.Version}}\n") // commander prints usage on parse errors but not on action errors; cobra diff --git a/internal/ledger/ledger.go b/internal/ledger/ledger.go index 1d631fdd..67160a1f 100644 --- a/internal/ledger/ledger.go +++ b/internal/ledger/ledger.go @@ -180,16 +180,21 @@ type OperatorDegradedRecord struct { // ReleaseRecord is the Go ReleaseLedgerRecord (release/tag outcome, Q24). type ReleaseRecord struct { - TS string `json:"ts"` - Kind string `json:"kind"` - TaskID string `json:"taskId"` - Attempt int `json:"attempt"` - Event string `json:"event"` // release-created - Tag string `json:"tag"` - SHA string `json:"sha"` - Version string `json:"version"` - Source string `json:"source"` - Detail *string `json:"detail,omitempty"` + TS string `json:"ts"` + Kind string `json:"kind"` + TaskID string `json:"taskId"` + Attempt int `json:"attempt"` + Event string `json:"event"` // release-created + Tag string `json:"tag"` + SHA string `json:"sha"` + Version string `json:"version"` + // Revision is the Go-side stamp (version.Revision at write time): the + // binary that recorded the release. omitempty keeps unstamped Go rows + // byte-compatible with the TS writer; Node readers tolerate the extra + // key (TestSchemaDriftUnknownFieldsTolerated). + Revision string `json:"revision,omitempty"` + Source string `json:"source"` + Detail *string `json:"detail,omitempty"` } // StashRecord is the Go StashLedgerRecord (merge-back auto-stash, Q26). diff --git a/internal/loopdriver/run.go b/internal/loopdriver/run.go index 9186129f..7bd2bec4 100644 --- a/internal/loopdriver/run.go +++ b/internal/loopdriver/run.go @@ -21,6 +21,7 @@ import ( "time" "github.com/FreePeak/devagent/internal/orchestrator" + "github.com/FreePeak/devagent/internal/version" ) // run.go ports the main iteration loop of scripts/selfbuild-loop.sh @@ -88,6 +89,16 @@ func RunLoop(cfg LoopConfig) int { cfg = cfg.WithDefaults() d := &driver{cfg: cfg, stateDir: filepath.Join(cfg.Repo, ".selfbuild")} + // Stale-binary guard (2026-09-12): loops 290/291 burned implement-and- + // gate cycles while a driver built from an older commit held the loop + // seat, and nothing announced the mismatch. Advisory only — the warning + // names both sides and the loop still runs. + if rev := version.Revision(); rev == version.RevisionUnknown { + _, _ = fmt.Fprintln(cfg.Stderr, "[loop] WARN: stale binary: build carries no revision (go test / unstamped build) — rebuild with `make build` before trusting this loop") + } else if head, ok := d.gitQuiet("rev-parse", "HEAD"); ok && strings.TrimSpace(head) != rev { + _, _ = fmt.Fprintf(cfg.Stderr, "[loop] WARN: stale binary: built from %s, repo HEAD is %s — rebuild with `make build`\n", rev, strings.TrimSpace(head)) + } + _ = os.MkdirAll(d.stateDir+"/research", 0o755) _ = os.MkdirAll(d.stateDir+"/goals", 0o755) _ = os.MkdirAll(d.stateDir+"/logs", 0o755) diff --git a/internal/loopdriver/run_test.go b/internal/loopdriver/run_test.go index 713326c3..458fffbe 100644 --- a/internal/loopdriver/run_test.go +++ b/internal/loopdriver/run_test.go @@ -5,12 +5,14 @@ import ( "encoding/json" "fmt" "os" + "os/exec" "path/filepath" "strings" "testing" "time" "github.com/FreePeak/devagent/internal/queue" + "github.com/FreePeak/devagent/internal/version" ) // frozenClock returns a Now func advancing one second per call, starting at @@ -2180,3 +2182,75 @@ func TestRunLoopRejectsMalformedGoalShape(t *testing.T) { t.Fatalf("malformed goal reached the task dispatch: %s", calls) } } + +// TestRunLoopStaleBinaryWarns pins the driver's self-identification guard +// (2026-09-12): a binary that cannot name its revision — the go-test binary +// is exactly that shape — must WARN at startup; a revision differing from +// the fixture HEAD must WARN naming both sides; a matching revision stays +// quiet. The warn is advisory: every arm still runs and exits 0. +func TestRunLoopStaleBinaryWarns(t *testing.T) { + old := version.RevisionOverride + t.Cleanup(func() { version.RevisionOverride = old }) + + // committedFixture returns a fixture repo with one commit plus its HEAD. + committedFixture := func() (string, string) { + t.Helper() + repo := initFixtureRepo(t) + installFakes(t, repo) + writeRepoFile(t, repo, "docs/PRD.md", "# PRD\n") + runGit := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = repo + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + return string(out) + } + runGit("add", "-A") + runGit("commit", "-q", "-m", "init") + return repo, strings.TrimSpace(runGit("rev-parse", "HEAD")) + } + + runLoop := func(repo string) (int, string) { + t.Helper() + now, _ := frozenClock() + var stderr bytes.Buffer + cfg := loopConfigFor(t, repo, func(c *LoopConfig) { + c.Now = now + c.Stderr = &stderr + }) + return RunLoop(cfg), stderr.String() + } + + // Arm 1 — unknown revision (the go-test binary's own shape). + version.RevisionOverride = "" + repo, _ := committedFixture() + if rc, out := runLoop(repo); rc != 0 { + t.Fatalf("rc = %d, want 0 (the warn is advisory)", rc) + } else if !strings.Contains(out, "WARN: stale binary") || !strings.Contains(out, "no revision") { + t.Fatalf("unstamped warn missing from stderr:\n%s", out) + } + + // Arm 2 — revision differs from HEAD. + version.RevisionOverride = "0000000000000000000000000000000000000000" + repo, _ = committedFixture() + if rc, out := runLoop(repo); rc != 0 { + t.Fatalf("rc = %d, want 0 (the warn is advisory)", rc) + } else if !strings.Contains(out, "WARN: stale binary") || !strings.Contains(out, "repo HEAD is") { + t.Fatalf("mismatch warn missing from stderr:\n%s", out) + } + + // Arm 3 — matching revision: quiet, a current binary warns nowhere. + repo, head := committedFixture() + version.RevisionOverride = head + if rc, out := runLoop(repo); rc != 0 { + t.Fatalf("rc = %d, want 0 (the warn is advisory)", rc) + } else if strings.Contains(out, "stale binary") { + t.Fatalf("matching revision still warned:\n%s", out) + } +} diff --git a/internal/version/version.go b/internal/version/version.go index f51e2024..a8a554bd 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -1,7 +1,50 @@ // Package version is the single source of truth for the devagent version. package version +import ( + "runtime/debug" + "sync" +) + // Version is the CLI version reported by `devagent --version` and embedded // in ledger metadata. A var (not a const) so -ldflags -X can stamp it at // build time; the unstamped default stays 0.1.0. var Version = "0.1.0" + +// RevisionUnknown is what Revision returns when the binary was built +// without a VCS stamp (go-test binaries, builds from a tarball): it cannot +// prove which commit it drives, so consumers must not treat it as clean. +const RevisionUnknown = "unknown" + +// RevisionOverride injects the build revision in tests — go-test binaries +// carry no vcs.revision build setting, so the real stamp is unreachable +// there. A non-empty value wins over the memoized build info; production +// leaves it empty (same stampable-var shape as Version). +var RevisionOverride string + +var ( + revisionOnce sync.Once + revisionMemo string +) + +// Revision returns the VCS revision the binary was built from +// (debug.ReadBuildInfo's vcs.revision), memoized on first call, or +// RevisionUnknown when the build carries no stamp. +func Revision() string { + revisionOnce.Do(func() { + rev := RevisionUnknown + if bi, ok := debug.ReadBuildInfo(); ok { + for _, s := range bi.Settings { + if s.Key == "vcs.revision" && s.Value != "" { + rev = s.Value + break + } + } + } + revisionMemo = rev + }) + if RevisionOverride != "" { + return RevisionOverride + } + return revisionMemo +} diff --git a/internal/version/version_test.go b/internal/version/version_test.go index a18d61df..2a51b853 100644 --- a/internal/version/version_test.go +++ b/internal/version/version_test.go @@ -21,3 +21,25 @@ func TestVersionIsSemver(t *testing.T) { t.Fatalf("Version = %q, want MAJOR.MINOR.PATCH", Version) } } + +// TestRevision pins the stale-binary sentinel contract: under go test the +// binary carries no vcs.revision build setting (probed 2026-09-12), so the +// unstamped path returns RevisionUnknown, repeat calls agree (memoized), +// and an override — the test-only injection surface — wins over whatever +// was memoized. +func TestRevision(t *testing.T) { + old := RevisionOverride + t.Cleanup(func() { RevisionOverride = old }) + + RevisionOverride = "" + if got := Revision(); got != RevisionUnknown { + t.Fatalf("Revision() = %q under go test, want %q", got, RevisionUnknown) + } + if a, b := Revision(), Revision(); a != b { + t.Fatalf("Revision not memoized: %q vs %q", a, b) + } + RevisionOverride = "deadbeef" + if got := Revision(); got != "deadbeef" { + t.Fatalf("Revision() = %q, want the override deadbeef", got) + } +}