Close out the audit, move what is left to the backlog - #12
Conversation
The audit read as an open to-do list for work that has since shipped. Its findings and line numbers were true against baseline c1bca77 and are left exactly as written; a Closeout section at the top now says what happened after, so nobody chases a question that already has an answer. Closed since: F-07, by x/crypto v0.56.0 and the go 1.26.0 floor, which leaves no reachable module advisories at all; and F-11, by dropping RequiresApproval so the tool and permissions.readOnlyTool finally agree. Six of the ten unresolved questions are answered or resolved -- U2, U3, U4, U6, U9, U10 -- and the four that remain say so. Section 8.5's dependency snapshot is marked superseded rather than rewritten, since it is a record of what was installed on the day. The Closeout also records four bugs the CI work found that the audit did not, because each needed a check to actually run rather than a file to be read: the MCP shutdown race, the ACP active-flag ordering, a cleanup that swallowed a job-manager shutdown error, and unchecked type assertions in procrun. Three things move to BACKLOG.md rather than being left in a chat log: internal/jobs degrading under CPU pressure, which is now the likeliest source of an unexplained red run; Go 1.27 being blocked on golangci-lint rather than on us; and audit finding F-14, which the audit marked "open: BACKLOG" but which was never actually written down there. The errcheck test-assertion exemption and audit question U1 are recorded as the open decisions they are. The catwalk rejection at BACKLOG.md:89 keeps its ruling, but one of its five stated reasons was the Go floor and that reason has evaporated, so it now says which half no longer applies. 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: d635120cb5
ℹ️ 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".
|
|
||
| | ID | Was | Now | | ||
| | --- | --- | --- | | ||
| | F-07 | `x/crypto` v0.43.0, toolchain open | **closed.** `x/crypto` v0.56.0 and `go 1.26.0`; govulncheck reports no reachable module advisories at all. Section 8.5 is superseded. | |
There was a problem hiding this comment.
F-07 also covered the reachable standard-library advisories, but raising the module floor to Go 1.26.0 does not fix those for users following README.md:68, which explicitly supports source builds with Go 1.26.0 or newer. Only CI and release artifacts are pinned to 1.26.8; builds made with 1.26.0–1.26.5 still contain advisories that the audit says require at least 1.26.6. Keep the source-build portion open or raise the supported minimum before declaring the finding closed.
Useful? React with 👍 / 👎.
| | ID | State | | ||
| | --- | --- | | ||
| | U1 | **Open.** Still a question about how PacketADE launches `packetcode acp`. F-09 depends on it. | | ||
| | U2 | **Answered: no.** No provider `base_url` is set anywhere in the tree, so no plain-http endpoint exists to find. F-10 remains a decision about whether to refuse one pre-emptively. | |
There was a problem hiding this comment.
The absence of a configured base_url in the repository cannot answer whether a real deployment uses plain HTTP, because those values live in each operator's ~/.packetcode/config.toml. The original U2 row at line 605 explicitly requires checking every deployed machine, and docs/handoff.md:66 still records that evidence as outstanding. Marking this answered may cause the F-10 decision to rely on an unsupported assumption; record only that the repository search found no evidence and leave U2 open.
Useful? React with 👍 / 👎.
| | U1 | **Open.** Still a question about how PacketADE launches `packetcode acp`. F-09 depends on it. | | ||
| | U2 | **Answered: no.** No provider `base_url` is set anywhere in the tree, so no plain-http endpoint exists to find. F-10 remains a decision about whether to refuse one pre-emptively. | | ||
| | U3 | **Resolved: yes.** The floor moved to `go 1.26.0` with `x/crypto` v0.56.0. The intermediate v0.52.0 / `go 1.25.0` option was rejected: 1.25 is itself end-of-life, and it would have left GO-2026-6354 and 6355 reachable. | | ||
| | U4 | **Answered: no.** No session under `~/.packetcode/sessions` had read a `.env`. | |
There was a problem hiding this comment.
Checking one local ~/.packetcode/sessions directory does not answer the original question of whether anyone relies on .env reads. The evidence requirement at line 607 includes asking users, and other installations' transcripts are not represented by this directory. Since P02 is a user-visible behavior change, describe the local search as finding no evidence rather than marking U4 answered.
Useful? React with 👍 / 👎.
1 participant
The audit document read as an open to-do list for work that has since shipped, and three loose ends were living only in a chat log.
The audit doc
Its findings, line numbers and reasoning were true against baseline
c1bca77and are left exactly as written. Rewriting a dated analysis to match today would destroy the record. Instead aCloseoutsection at the top says what happened after, so nobody chases a question that already has an answer.Closed since the audit:
go 1.26.0floor — no reachable module advisories at allRequiresApprovaldropped, so the tool andpermissions.readOnlyToolfinally agreeSix of ten unresolved questions are answered or resolved (U2, U3, U4, U6, U9, U10). The four that remain — U1, U5, U7, U8 — say so plainly, along with what would settle each.
Section 8.5's dependency snapshot is marked superseded rather than rewritten: it is a record of what was installed that day, and the upgrade warnings on the non-
x/*rows still hold.The Closeout also records four bugs the CI work found that the audit did not, because each needed a check to actually run rather than a file to be read: the MCP shutdown race, the ACP
active-flag ordering, a cleanup that swallowed a job-manager shutdown error, and unchecked type assertions inprocrun. Worth being honest about — reading found the other eighteen, but not these.The backlog
Three things that were only in conversation:
internal/jobsdegrades under CPU pressure. ~3.4s idle; 88s with a test failing outright when three lint passes ran alongside. Nothing is broken in the ordinary sense — it passes in isolation every time — but it is now the likeliest source of the next unexplained red run.context.Background(). The audit marked it "open: BACKLOG" but it was never actually written there.Two open decisions are recorded as decisions rather than left implicit: the errcheck test-assertion exemption (one config block to reverse), and audit question U1, which gates whether F-09 is medium or high.
One correction to an existing entry
The
charm.land/catwalkrejection atBACKLOG.md:89gave five reasons, one of which was "declaresgo 1.26.6against this repo's 1.24.2". That reason has evaporated. The ruling stands on the other four, so the entry now says which half no longer applies rather than being silently left wrong.Docs only — no code changes.
🤖 Generated with Claude Code