Day 31 backlog: Go 1.26.8, backup pruning, three-runner smoke, and one honest permission answer - #1
Conversation
1.26.3 was the pin from the initial workflow and five patch releases have shipped since. The toolchain that builds the release binaries should not be the stale one. Both workflows read the same GO_VERSION key, so they move together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… had RequiresApproval returned true for the foreground tool, but every built-in profile lists collect_agent_results in permissions.readOnlyTool, and that check allows the call before RequiresApproval is ever consulted. The gate did not exist. Two sources described the tool and they disagreed. This aligns the code with the behaviour the policy actually implements and with docs/advanced-guide.md, which already documents collection as automatic with an explicit session rule as the way to re-add a gate. No decision changes for any profile: the read-only branch was already returning allow first. The existing test asserted the unreachable intent rather than the behaviour, so it is replaced by one that pins the answer for both the foreground and background constructions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The undo stack is in-memory and reset whenever a BackupManager is constructed, so a backup tree left by a session that has already exited cannot be restored from by anything. Nothing read those files again; they only accumulated under ~/.packetcode/backups until someone noticed the disk. BackupManager.Cleanup existed but had no production caller. PruneBackups removes trees whose newest write is older than the retention window, once per start, before anything writes new ones. Age is judged by the newest mtime anywhere in the tree rather than the session directory's own: backups mirror the source tree, so an active session's top directory can look stale while a file three levels down was just written. The tree belonging to the session that is starting is never a candidate, because a resumed session is routinely older than the window. Pruning is best effort and silent. A tree that cannot be walked or removed is kept, since failing to reclaim disk is not a reason to fail a startup, and only names that pass the session-id check are joined onto the base, so a candidate path cannot escape the backups directory. Two keys configure it. backup_retention_days defaults to 14 and treats a nonsensical value as unset, matching every other [behavior] cap and reported by doctor; backup_prune_disabled is the off switch, matching the loop_detection_disabled and post_edit_diagnostics_disabled idiom rather than overloading the sign of a count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The smoke job proved the binary starts. It did not exercise a single turn of the agent loop, so every behaviour the audit hardened -- credential resolution, the fail-closed approval path, the dotenv secret refusal, the compound-command deny floor -- was covered only by unit tests and by a script nobody ran. smoke.sh drives the real loop against the stdlib-only stub provider on loopback. It needs no credentials, no network and no new dependencies, and it exits nonzero when any assertion fails, so it gates rather than reports. bash is named explicitly because the Windows runner would otherwise pick PowerShell. The Makefile comment said to add smoke-e2e to `ci` once the script had run green on all three runners. This change is what produces that evidence, so the target moves at the same time and the merge is gated on the result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four of the eight items are done, one is partly answered, and three stay open. The list said what was predicted; it now says what the work actually involved, including the correction that CI moved to Go 1.26.8 rather than the 1.26.6 the entry named, because 1.26.8 was the current patch release when the change was made. K-02 and K-07 are left out of the branch on purpose and the entries say why: one decides who can build the project, the other moves a security boundary and needs its own review. The generator and the markdown carry the same list, so both move together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46d4d5e64e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| env: | ||
| GO_VERSION: '1.26.3' | ||
| GO_VERSION: '1.26.8' |
There was a problem hiding this comment.
As of this commit's 2026-09-05 date, Go 1.26.8 has not been released, so actions/setup-go cannot resolve this exact version. Every job in .github/workflows/ci.yml fails during toolchain setup, and the identical pin in .github/workflows/release.yml also prevents releases; use the current available 1.26 patch release instead.
Useful? React with 👍 / 👎.
| // Undo cannot reach a previous run's backups, so they are dead weight on | ||
| // disk. Reclaim the stale ones once per start, before anything writes new | ||
| // ones, and never touch the tree this session is about to use. | ||
| session.PruneBackups(opts.BackupsDir, rt.SessionID, backupRetention(opts.Config.Behavior)) |
There was a problem hiding this comment.
When more than one runtime is alive, age does not establish that a backup tree is unreachable. For example, an ACP server retains multiple independent runtimes in Server.sessions; if session A made a backup and remains open beyond the configured retention period, creating session B calls this pruning path and deletes A's tree because only B's ID is protected. A subsequent /undo in A then loses its still-live backup, so pruning needs to exclude all active sessions or use a process/activity marker rather than treating every non-current tree as exited.
Useful? React with 👍 / 👎.
The first CI run on this branch turned two predictions into numbers. K-01: govulncheck reports 15 reachable vulnerabilities on main at Go 1.26.3, 8 of them in the standard library, and 7 on this branch at 1.26.8 with none in the standard library. The pin cleared 8 stdlib advisories, not the 9 the audit predicted. K-02: the 7 that remain are all x/crypto v0.43.0, every one reached through internal/computers/ssh_backend.go, so vulncheck fails on main and on every branch until the decision is made. It also splits in two, which the audit did not know: five are fixed in v0.52.0 behind a go 1.25.0 floor, and only the remaining two need v0.56.0 and a go 1.26.0 floor. A one-minor raise buys five of seven, which is a cheaper option than the prepared patch and worth having on the table before deciding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Four of the eight day-31 items from the security audit, one commit each so any of them can be reverted alone. Three items are deliberately left out and are explained below.
This PR adds three CI jobs that pass and introduces no new failures. CI is nonetheless red, because it was already red on
main. The attribution is at the bottom, job by job, with evidence.What is in
K-01 — pin Go 1.26.8 (
6e8e778)ci.ymlandrelease.ymlboth read oneGO_VERSIONkey, still pinned to 1.26.3 from the initial workflow. The backlog entry named 1.26.6; 1.26.8 was the current 1.26 patch release when this was written, so it is the pin.CI measured the effect. On
mainat 1.26.3, govulncheck reports 15 reachable vulnerabilities, 8 of them in the Go standard library. On this branch at 1.26.8 it reports 7, none in the standard library. So the pin cleared 8 reachable stdlib advisories, not the 9 the audit predicted, with no code change.K-06 —
collect_agent_resultsstops claiming a gate it never had (fe9d3f5)RequiresApproval()returned true for the foreground tool. Every built-in profile listscollect_agent_resultsinpermissions.readOnlyTool, and that branch returns allow beforeRequiresApprovalis ever consulted, so the prompt never happened. Two sources described the tool and they disagreed;docs/advanced-guide.mdalready documents collection as automatic, with an explicit session rule as the way to re-add a gate.No decision changes under any profile, built-in or custom: the read-only branch was already winning. The existing test asserted the unreachable intent rather than the behaviour, so it is replaced by one that pins the answer for both constructions.
TestPolicy_SafeProfileDeniesDestructiveAndAllowsReadalready pinned it from the policy side.K-05 — prune undo backups that no run can reach (
bd9547b)The undo stack is in-memory and reset whenever a
BackupManageris constructed, so a backup tree left by a session that has already exited cannot be restored from by anything. Nothing read those files again; they accumulated under~/.packetcode/backupsuntil someone noticed the disk.BackupManager.Cleanupexisted but had no production caller.session.PruneBackupsremoves trees whose newest write is older than the retention window, once per start, before anything writes new ones. Three things are worth calling out:Two keys configure it, following the conventions already in
[behavior]rather than inventing one.backup_retention_daysdefaults to 14 and treats a negative value as unset the way every other cap there does, withdoctorreporting it.backup_prune_disabledis the off switch, matchingloop_detection_disabledandpost_edit_diagnostics_disabled. Overloading the sign of a count to mean "disabled" would have contradicted its neighbours.K-03 — run the end-to-end smoke on all three runners (
b1dd6fb)The smoke job proved the binary starts. It did not exercise a single turn of the agent loop, so every behaviour the audit hardened was covered only by unit tests and by a script nobody ran in CI.
smoke.shdrives the real loop against the stdlib-only stub provider on loopback: credential resolution, an approved write, the fail-closed approval path, the dotenv secret refusal, and the compound-command deny floor. No credentials, no network, no new dependencies. It exits nonzero when any assertion fails, so it gates rather than reports.shell: bashis explicit because the Windows runner would otherwise pick PowerShell.The
Makefilecomment said to addsmoke-e2etoci"once it has run green on all three runners". That evidence now exists: smoke passes on ubuntu-latest, macos-latest and windows-latest. The macOS and Linux runs were the first ever; the script had only been exercised on Windows before.What is out, and why
vulncheck, but it is a decision about who can build the project, not an audit call.internal/tools/safefs.goviaLocalBackend.Resolve) — moves a security boundary. It needs its own review and does not belong in a batch of small changes.K-02 is now the blocker for a green CI, and it is cheaper than the audit thought
All 7 remaining reachable advisories are in
golang.org/x/cryptov0.43.0, every one of them reached throughinternal/computers/ssh_backend.go. The fix splits in two, which the audit did not know:godirectivego 1.25.0go 1.26.0go.modis currently atgo 1.24.2. So five of the seven are reachable by a one-minor floor raise to 1.25, and only the last two need the 1.26 floor thatdocs/audit/patches/P10b-x-crypto-v0.56.0-go1.26.patchapplies. Either way the Go version inREADME.mdandHANDOFF.mdmoves with it. Both options are on the table; neither is taken here.K-04, partly answered
U-02is closed: there is no providerbase_urlset anywhere in the tree, so there is no plain-http provider endpoint to find.U-01is still open.Two incidental observations from the same sweep, neither actioned here: no session under
~/.packetcode/sessionshad read a.envfile, andinternal/doctor/exists on disk as an empty directory that git does not track, which is whatBACKLOG.mddescribes.Local verification
Run on windows/amd64 with go1.26.2 against the branch tip:
go build ./...go vet ./...go test -count=1 ./...bash smoke.shgofmton every changed Go filepacketcode doctorwithbackup_retention_days = -3The new prune tests were checked against a negative control: forcing
PruneBackupsto return early madeTestPruneBackups_RemovesStaleTreesOnlyfail, and restoring it made the suite pass again. The workbook generator was re-run and still produces the PDF.CI results, job by job
Green: all six
buildjobs,release dry run, andsmokeon all three runners (the jobs this PR adds).Red, every one of them also red on
mainat96865bb, the commit this branch forks from:vulncheckmain, where it reports 15 rather than 7lint.golangci.ymlis in the v1 format; golangci-lint v2.9.0 rejects it withunsupported version of the configuration: "". The lint job has not actually linted anything for some timemainTUI golden and protocol safetymake tui-golden-checkexits 1main, same targettest (ubuntu-latest),test (macos-latest)TestApp_Undo_RestoreAndDepth,TestKillTreeOnExitedProcessIsConfirmed,TestKillTreeBeforeReapIsUnconfirmedWithReasonmain's runtest (windows-latest)TestRunUserPromptSubmit_CollectsStdouttimes out at 5s waiting on a PowerShell hookmainin both prior runs (c1bca77,d646094)The Windows one deserves a note because it is intermittent rather than constant. Across the three most recent
mainruns it failed twice and passed once; on this branch it failed twice, including a deliberate re-run, and then passed on the final run. Same test, same 5s budget, same PowerShell startup timeout every time. It is a pre-existing flake in a package this PR does not touch.None of these are in scope here, but
lintis worth someone's attention: it is not merely failing, it is not running.🤖 Generated with Claude Code