From 8ca0688993bf777ae74f7e11028932e33aee3e9a Mon Sep 17 00:00:00 2001 From: "linh.doan" Date: Sun, 13 Sep 2026 04:24:20 +0700 Subject: [PATCH] fix(ledger): release-created rows carry tag/sha again (regressed in #349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #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 empty tag/sha — the exact Q24 fields the record exists to carry. No test covered the CLI write site, so it shipped green. - restore Tag/SHA in the ReleaseRecord literal (actions_ledger.go) - add internal/cli/actions_release_test.go pinning the CLI row shape (proven red on the regressed literal, green after) - update docs/PRD.md footer + regenerate docs/PRD.html (pandoc) so the styled mirror matches PRD.md and gains #349's build-revision line too --- docs/PRD.html | 56 ++++++++++++++++++++------ docs/PRD.md | 1 + internal/cli/actions_ledger.go | 2 + internal/cli/actions_release_test.go | 59 ++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 internal/cli/actions_release_test.go diff --git a/docs/PRD.html b/docs/PRD.html index ea0b69e..882248c 100644 --- a/docs/PRD.html +++ b/docs/PRD.html @@ -4945,8 +4945,20 @@

23.1 Requirements

chaos 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) — +

*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 @@ -4995,15 +5007,37 @@

23.1 Requirements

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 (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. +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 <sha>). 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 diff --git a/docs/PRD.md b/docs/PRD.md index 6e70186..3c2a5fc 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 (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). *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`). diff --git a/internal/cli/actions_ledger.go b/internal/cli/actions_ledger.go index 9147b8a..cfc8bab 100644 --- a/internal/cli/actions_ledger.go +++ b/internal/cli/actions_ledger.go @@ -237,6 +237,8 @@ func recordCommand() *cobra.Command { TaskID: "release/" + version, Attempt: 1, Event: "release-created", + Tag: tag, + SHA: sha, Version: version, Revision: versionpkg.Revision(), Source: source, diff --git a/internal/cli/actions_release_test.go b/internal/cli/actions_release_test.go new file mode 100644 index 0000000..d578b87 --- /dev/null +++ b/internal/cli/actions_release_test.go @@ -0,0 +1,59 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestRecordReleasePersistsTagAndSHA pins the row shape at the `record +// release` CLI write site: every Q24 field the command reads from flags +// must land in the appended release-created row. PR #349 added a Revision +// field but silently dropped Tag/SHA from the ReleaseRecord literal while +// still printing "(tag @ sha)" to stdout, so rows persisted "tag":"" +// "sha":"" — the exact fields the record exists to carry. No test covered +// this write path, which is why it stayed green; this closes that gap. +func TestRecordReleasePersistsTagAndSHA(t *testing.T) { + repo := t.TempDir() + + root := NewRoot() + root.SetArgs([]string{ + "record", "release", + "--tag", "v1.2.3", + "--sha", "deadbeefcafe", + "--repo", repo, + "--source", "cli", + }) + root.SilenceUsage = true + if err := root.Execute(); err != nil { + t.Fatalf("record release: %v", err) + } + + data, err := os.ReadFile(filepath.Join(repo, ".devagent", "runs", "orchestration", "events.jsonl")) + if err != nil { + t.Fatalf("read events.jsonl: %v", err) + } + line := strings.TrimSpace(string(data)) + if line == "" { + t.Fatal("no release-created row written") + } + + var row map[string]any + if err := json.Unmarshal([]byte(line), &row); err != nil { + t.Fatalf("unmarshal row %q: %v", line, err) + } + + for key, want := range map[string]string{ + "event": "release-created", + "tag": "v1.2.3", + "sha": "deadbeefcafe", + "version": "1.2.3", + "source": "cli", + } { + if got, _ := row[key].(string); got != want { + t.Errorf("row[%q] = %q, want %q", key, got, want) + } + } +}