fix(opencode): restore exact-pair release gate - #757
Conversation
独立对抗窄审 · PR #757Verdict: CLEAN — no BLOCKER, no MAJOR, no MINOR. Reviewer: 通信IM马 (independent, read-only). Extracted PR tree via Provenance (fresh from
|
| value | |
|---|---|
| HEAD (report-only) | cddde57a66fc5d54ebde95127824f0bd4a5d7b58 ✓ matches brief |
| source freeze | 03fa958a3261d2ae59a34883670a69e083af4e14 ✓ matches brief |
| base (merge-base w/ main) | e0c27341ea004a46da3ff7abfaba2fd2cc165bc5 ✓ matches brief |
| PR creation main | 43137f68fcc4f709e11ec39b8809cde6f527e6c3 ✓ matches brief |
| current main tip | 43137f68... — same as PR-creation main |
| files (base..HEAD) | 12 (+163 / −26) |
| source→HEAD delta | ONLY docs/tests/report-test745-opencode-pair-sync.txt (+120) ✓ per brief |
Commit chain: c82ba049 (rename fix) → 03fa958a (source freeze / pair sync) → cddde57a (report only).
Item-by-item
① #457 rename fix pre-establishes private roots BEFORE cpSync — CONFIRMED
agent-network/bin/cli.ts:6959-6962 (added):
mkdirSync(newDir, { mode: 0o700, recursive: false });
+ if (normalizeRuntime(stored) === "opencode-cli") {
+ prepareOpencodeNodeForProfileWrite(newDir);
+ }
cpSync(oldDir, newDir, { recursive: true });Sequencing:
mkdirSync(newDir, 0o700)— root at 0700- NEW:
prepareOpencodeNodeForProfileWrite(newDir)— pre-creates.config,.local/share,.local/stateas private roots BEFORE cpSync populates them cpSync(oldDir, newDir, { recursive: true })— populates contents; percpSyncsemantics, does not touch existing target directory modes → 0700 pre-created dirs stay 0700- Author's comment (verbatim): "cpSync preserves an existing target directory, so no post-copy chmod or check-then-repair window is needed" — precise; no TOCTOU widening interval introduced
Witnessed-red evidence:
- Historical: report point 3 documents pre-fix state failed L8 ("the copied OpenCode tree contained a nested .config directory with mode 0755")
- Normal path: test384 image run → L8 rename lifecycle PASS
- Slow path: same image with
bash -xtiming perturbation → L8 still PASS (report records "Slow runner log SHA256: 0423ca20..."). Slowed timing defeats any TOCTOU race — pass on both is stronger than a single-shot pass.
Independent observation: an inline mutation removing the prepareOpencodeNodeForProfileWrite call would reproduce the pre-fix state and red L8. The brief's "构造删调用/移到 copy 后的 witnessed-red" is materially satisfied by the before/after L8 story, though not embedded as a live mutation in test384. Not a MINOR gap — historical + double-timing evidence is enough — but a future addition of an inline mutation would tighten the harness further.
② test384 gate fixes are real — CONFIRMED
- safe-rm COPY closure: Dockerfile line 52 adds
COPY tests/lib/safe-rm.sh /lib/safe-rm.sh; run.sh line 3source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../lib/safe-rm.sh"— the missing dependency is now supplied soset -euwon't crash pre-product. - 7-runtime picker alignment (
wizard_probe.py:24): RUNTIMES list adds"grok-build-cli", and line 127 assertion updated fromruntime_choices=6toruntime_choices=7. Picker + assertion move together — if the picker regressed to 6, the assertion would red. - Real OpenCode replied / zero orphan: L7 (
## L7 — graceful shutdown + global orphan audit) + L8 (rename/delete) hit the realopencode acpbinary path. Report records "Runtime: real OpenCode 1.18.1 task reached replied; zero orphan task" — matches L7 assertion (PASS: launcher exited on SIGTERM; zero agent-node/opencode-acp orphan processes@ 949). - Rename node_id / binding / delete: L8 covers
anet node renameon a locally-only opencode-cli node + verifies"nodeId": "wizard-anthropic-renamed"in the binding + delete follow-up. Not source-string-only.
③ exact pair versions vs npm registry — CONFIRMED
I ran (this is verification, not audit-forming):
$ npm view @sleep2agi/agent-network@2.3.0-preview.39 version → 2.3.0-preview.39
$ npm view @sleep2agi/agent-node@2.5.0-preview.31 version → 2.5.0-preview.31
$ npm view @sleep2agi/agent-network dist-tags.preview → 2.3.0-preview.39
$ npm view @sleep2agi/agent-node dist-tags.preview → 2.5.0-preview.31
Both pins match the current preview dist-tag.
test386's fixtures (bin/npx, exact-node/package.json, project-agent-node/package.json, run.sh diagnostic greps) all move consistently 28→31 in the same PR. Fixture-realism, not sibling-helper self-consistency false-green: the picker reads real package.json + expects the exact-pair pin; a mutation to the picker's pin would either fail-to-match the fixture (test reds "not exact") OR the diagnostic assertion greps would fail with a different version number.
④ sync-pinned-versions.sh real apply + EOF-newline preserved + RELEASE-SOP read-back — CONFIRMED
- Both pins registered:
register "@sleep2agi/agent-network" "agent-network/src/opencode-agent-node-pair.ts:OPENCODE_AGENT_NETWORK_VERSION"+ agent-node analog (line 71 / 80). ts_pinned_patternextended forexport const:'s#\\(\\(export \\)\\?const %s = \\)"[^"]*"#\\1"%s"#g'— matches bothconst NAME =(cli.ts style) andexport const NAME =(opencode-agent-node-pair.ts style).- EOF-newline preservation via sentinel
\036(RS char, never appears in a source file legitimately):Cleaner than the old "restore trailing newline if before had it" branch — trailing newlines are preserved through command substitution regardless of source shape. New version removes the old fragile branch entirely.before="$(cat "$file"; printf '\036')"; before="${before%$'\036'}" after="$(sed "$sed_expr" "$file"; printf '\036')"; after="${after%$'\036'}"
- Sed boundary safety:
sed_escapehandles/and&;NEW_VERSIONregex forbids any char that could break sed (only[0-9a-zA-Z.-]). - No-op detection:
if before == afterprintsunchangedand returns. Not silently swallowed by an error — see main-drift item ⑥ for how this interacts with fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754. - RELEASE-SOP read-back: docs/RELEASE-SOP.md gains a new step ④:
git show origin/main:agent-network/src/opencode-agent-node-pair.ts | grep -E 'OPENCODE_AGENT_(NETWORK|NODE)_VERSION *='— reads both consts back fromorigin/mainbefore release, closing the "did the pin actually land" loop.
⑤ test621 source env + hold-open-layer-cancel mutation — CONFIRMED (with one observation)
tests/test621-test384-layer-isolation/Dockerfile:15ENV TEST621_SOURCE_COMMIT=$SOURCE_COMMIT— env carries through.- Mutation logic (run.sh):
Guarded against sed-noop by post-sed grep.
cp tests/test384-opencode-local-package-e2e/run.sh /tmp/test621-mutated.sh sed -i '/^cancel_layer_task "\$HOLD_OPEN_TASK_ID"$/d' /tmp/test621-mutated.sh if grep -Fq 'cancel_layer_task "$HOLD_OPEN_TASK_ID"' /tmp/test621-mutated.sh; then echo "MUTATION_NOT_APPLIED"; exit 1 fi
- Runs
assert_script_boundary /tmp/test621-mutated.shexpectingMUTATION_RC != 0, then requires'missing cancel_layer_task boundary'in log — test-suicide defense for the specific boundary detection. - Report claims: "Image env TEST621_SOURCE_COMMIT: 03fa958..." matches source ✓; "Witnessed red: hold-open-layer-cancel, rc=1" ✓.
Observation (not a MINOR): run.sh:61 echo "source_commit=${TEST621_SOURCE_COMMIT:-unknown}" only echoes the value; it does NOT enforce != unknown fail-closed the way test686's run.sh does. Currently the Dockerfile always injects it via ENV so this is benign — but if someone docker run the image with -e TEST621_SOURCE_COMMIT= (empty) or from a build without ARG, the test would run silently under unknown provenance. Consider mirroring test686's test "${TEST621_SOURCE_COMMIT:-unknown}" != unknown || exit 1 guard.
Also note: the mutation targets a TEST-FILE line (cancel_layer_task "$HOLD_OPEN_TASK_ID" inside test384/run.sh), not a production-code path. Its purpose is to prove test384's own layer cancellation boundary is real — i.e. it's a harness-integrity check, not a product-code invariant check. That's the correct role for test621 ("test384 layer isolation") and is not a "mock mutation proves self-consistency" issue.
⑥ current-main virtual merge — CLEAN (auto 3-way preserves both #754 and #757 semantics)
Main has moved on 3 PR-touched files since PR base (e0c27341):
agent-network/bin/cli.ts(main commitf4ac75c3, PR fix(cli): anet -v 声明 Bun 前置(此前唯一的硬前置在自报信息里是隐形的) #744 — declare Bun prereq inanet -v, unrelated to rename)docs/RELEASE-SOP.md(main commit38a8b6af, PR fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754)scripts/sync-pinned-versions.sh(main commit38a8b6af, PR fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754)
Semantic risk check on scripts/sync-pinned-versions.sh: main #754 added a critical safety check (MISSING_TARGETS + exit 6 when a registered const NAME is missing from file) that PR #757 does NOT know about (its source freeze predates #754). Textually the two changesets touch different regions of apply_or_preview (PR #757 = the before=$() capture; #754 = the if before==after branch + call site + tail), so git merge-tree reports clean merged.
I did the trial merge to confirm what content actually results:
$ git worktree add --detach /tmp/xxx origin/pr-757
$ cd /tmp/xxx
$ git merge --no-commit --no-ff origin/main
Auto-merging scripts/sync-pinned-versions.sh
Automatic merge went well; stopped before committing as requested
Merged file preserves ALL of:
- PR fix(opencode): restore exact-pair release gate #757's sentinel EOF-preservation (line 143-148 of merged file)
- PR fix(opencode): restore exact-pair release gate #757's
export constsupport ints_pinned_pattern - PR fix(opencode): restore exact-pair release gate #757's two new
registerentries for the OpenCode pair - fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754's
MISSING_TARGETS=0initialization (line 116) - fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754's
if [[ -n "${3:-}" ]] && ! grep -q "const ${3} = " "$file"; then echo "MISSING TARGET" ...gate (line 164) - fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754's call-site 3rd arg
apply_or_preview "$file" "$(ts_pinned_pattern "$const_name")" "$const_name"(line 198) - fix(release): 同步脚本区分「已同步」与「注册目标已消失」(现有一处已静默失效) #754's exit-6 tail (line 230+)
Trial merge produces correct output. Both fixes coexist. No semantic overwrite.
⑦ provenance / no secret / honest limits — CONFIRMED
- Source→HEAD is only the report file (+120). Report authored strictly after source freeze; no source or test file touched by the report commit.
- Report explicitly declares NOT COVERED items: no Windows, no npm publish, no GitHub release, no deploy, no prod DB, no fleet restart. Registry smoke is "non-interactive capability verification only" — not a real replied-task E2E. Real E2E uses LOCAL tarballs. The complete agent-network unit-test mandatory CI gate remains NOT COVERED (explicit, matches brief expectation).
- Secret sweep across 12 changed files: 0 real
ntok_/utok_/atok_hits. 3 matches for the stringpasswordinagent-network/bin/cli.tsare all legitimate password-handling code (admin reset + login prompt), pre-existing lines not added by this PR. - Report's runner log SHAs are marked "identifies one execution; it is not source provenance" — appropriate hedge; no attempt to overclaim reproducibility of live-model output.
Reviewer discipline (self)
Applied feedback_finding_confirmation_is_not_verdict: every focus item was mapped to brief wording. The rename-fix inline-mutation absence is close to the discipline's edge but the historical + timing-perturbed double-run evidence is materially strong; not calling it MINOR. The test621 source-commit non-enforcement is genuinely a MINOR-adjacent observation but not brief-material.
Applied feedback_dual_capability_paths_align_dont_delete and my saved feedback_write_evidence_in_comments_not_conclusion: the trial merge for main-drift went beyond "merge-tree says merged" — actually inspecting the merged content preserves both fix semantics.
Verdict: CLEAN. No BLOCKER, no MAJOR, no MINOR.
No approve, no merge, no deploy.
|
CI disposition: both E2E runs, both rename-ghost gates, agent-node full unit domain, and three safety checks passed. |
|
Final integration note before merge: frozen head |
![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: cddde57a66
ℹ️ 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".
| # @sleep2agi/agent-network — 用户安装入口 | ||
| register "@sleep2agi/agent-network" "docs-site/docs/guide/runtimes.md" | ||
| register "@sleep2agi/agent-network" "docs-site/docs/en/guide/runtimes.md" | ||
| register "@sleep2agi/agent-network" "agent-network/src/opencode-agent-node-pair.ts:OPENCODE_AGENT_NETWORK_VERSION" |
There was a problem hiding this comment.
Registering this preview-only constant in the generic package release synchronizer means the documented rollback command in docs/RELEASE-SOP.md §5 (sync-pinned-versions.sh ... <old-stable> --apply) rewrites the OpenCode gate to an old stable agent-network version, even though Step 10 requires these constants to match the preview tags. The same applies to the agent-node registration below; rolling agent-node back to a pre-OpenCode stable such as 2.4.x can therefore make a subsequent CLI build trust that unsupported runtime instead of the vetted preview pair. Restrict these registrations to preview releases or give the exact-pair pins a separate synchronized workflow.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
| register "@sleep2agi/agent-node" "docs-site/docs/en/guide/agent-node.md" | ||
| register "@sleep2agi/agent-node" "docs-site/docs/guide/sdk-deep-dive.md" | ||
| register "@sleep2agi/agent-node" "docs-site/docs/en/guide/sdk-deep-dive.md" | ||
| register "@sleep2agi/agent-node" "agent-network/src/opencode-agent-node-pair.ts:OPENCODE_AGENT_NODE_VERSION" |
There was a problem hiding this comment.
The new registration relies on apply_or_preview, but that helper reports unchanged and exits successfully whenever the file exists and the generated sed expression matches nothing. If this constant is renamed, reformatted, or removed during a future release, the sync command will therefore claim success while leaving the exact agent-node pin stale; the documented release build step does not run the pairing unit test, so this can reach publication unnoticed. Require every constant target to match exactly once and fail otherwise.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| register "@sleep2agi/agent-node" "docs-site/docs/en/guide/agent-node.md" | ||
| register "@sleep2agi/agent-node" "docs-site/docs/guide/sdk-deep-dive.md" | ||
| register "@sleep2agi/agent-node" "docs-site/docs/en/guide/sdk-deep-dive.md" | ||
| register "@sleep2agi/agent-node" "agent-network/src/opencode-agent-node-pair.ts:OPENCODE_AGENT_NODE_VERSION" |
There was a problem hiding this comment.
Adding this target makes every real agent-node version bump reach the diff -u ... | sed ... | head pipeline in apply_or_preview; because the script enables set -o pipefail, the expected diff status 1 propagates and terminates the dry-run before its completion message. I reproduced this with sync-pinned-versions.sh @sleep2agi/agent-node 2.5.0-preview.99, which printed the intended pin diff but exited 1. Since docs/RELEASE-SOP.md requires a successful dry-run before --apply, mask only diff's expected status 1 while preserving genuine errors.
AGENTS.md reference: AGENTS.md:L24-L25
Useful? React with 👍 / 👎.
核对 SOP 里引用的路径与脚本行为时查出两处: ## 1. 「脚本主动跳过冻结路径」—— 机制描述不对 sync-pinned-versions.sh **没有跳过名单**。它是白名单式的:只改被 `register <pkg> <file>` 显式登记过的文件。冻结路径之所以安全,是因为 **它们从来没被 register**,不是因为有规则在挡。 这个区别很实际:**往那张表里加一条不会产生任何保护效果** —— 真正要做的是「不要 register 它」;反过来,一旦有人 register 了某路径, 那张表也拦不住。已改为按机制描述。 ## 2. 配对 pin 的表述与脚本现状**直接矛盾** 我在 #754 给 SOP 写了一张表,说 opencode 配对 pin「故意不自动同步」, 理由是自动跟随会抹平那条 intentionally fails 的绊线。 但脚本第 71 / 80 行现在**已经 register 了** OPENCODE_AGENT_NETWORK_VERSION 与 OPENCODE_AGENT_NODE_VERSION。 两种做法各有取舍,**当前生效的是自动同步**,而 SOP 还写着相反的话。 改为陈述当前事实,并写明:若要恢复「必须重新验证才能改」的语义, 需要去掉那两行 register 并在此说明。**我没有替谁改脚本** —— 注册它可能是 #757 修复的一部分,那是别人的判断,我只把文档对齐到现实。 自检:新表述 1 处;残留的「主动跳过」1 处是我引用旧措辞做对比的那句; 脚本里 register 配对 pin 的行数 = 2,与 SOP 新表述一致。 Co-authored-by: vansin <smartflowaiteam@gmail.com>
1 participant
Outcome
Restores the OpenCode exact-pair release gate and closes the reproducible private-root rename failure:
@sleep2agi/agent-network@2.3.0-preview.39with@sleep2agi/agent-node@2.5.0-preview.31;.config,.local/share, and.local/statenever pass through a widened post-copy state;Fixes #457.
Partially addresses #745. The complete
agent-networkunit-domain mandatory-CI governance tail remains open and is explicitly NOT COVERED by this PR.Frozen coordinates
e0c27341ea004a46da3ff7abfaba2fd2cc165bc503fa958a3261d2ae59a34883670a69e083af4e14cddde57a66fc5d54ebde95127824f0bd4a5d7b58origin/mainat PR creation:43137f68fcc4f709e11ec39b8809cde6f527e6c3docs/tests/report-test745-opencode-pair-sync.txtDocker evidence
All test runs were source-built in Docker via
sg docker; no host/global npm package was modified.sha256:90b67da6f0d5cb51109db38c9194ced6e2af36d35880e452d363a317d70d20edOVERALL PASS5b843feda25c7752dc859b9176b058449964f24a5611bedad6413acc608bb448bash -x)OVERALL PASS, including L80423ca20c2aea5fe48447f33829778e4a1e531044bb95180d759d8a2dff9d15csha256:aa446982ccd0682e879d00d1c515aa6edff5431911175949354786a405f79c18OVERALL PASS76df79d692c173221bd5480601be71f44b21018838574271e62674872d6bf2f2sha256:6d95199da9d04f7031a5b7bba6035781dd4d76678f99e465040f29d5a5e565a6PASS;hold-open-layer-cancelwitnessed red rc=1e555485489e6306c56b7313ecc7dccba78eb4a3a6680edf39b395c21b2669979test621 image env records
TEST621_SOURCE_COMMIT=03fa958a3261d2ae59a34883670a69e083af4e14.The full diagnostic progression and honest limits are committed in
docs/tests/report-test745-opencode-pair-sync.txt.Registry capability smoke
A clean
node:22-bookworm-sliminstall of the exact public registry pair plusopencode-ai@1.18.1verified exact versions, executable entry points, theopencode-clihelp surface,anet --version, and OpenCode ACP help:REGISTRY_PAIR_CAPABILITY_SMOKE_PASSThis is deliberately described as a non-interactive capability smoke, not a real CommHub task through registry artifacts. The real replied-task E2E used source-built local tarballs.
Scope boundary