doc(SPEC): cover the mixed-mode contract merge; main goes green again - #112
Conversation
The agent/pr2-public-mixed-mode-contract merge (d47a0fa) landed without its SPEC updates, leaving main's freshness gate red: 18 stale pages and 4 uncovered modules. This re-verifies every page against the merged code: - Four new pages: contract.md (the one-way public consumer surface), direct_routing.md (tables+mechanism out of thin_mcp_server, known debt named), engine/worktrees.md (identity/verification/liveness, each a fixed real incident), idempotency.md (one run AND one execution per key; narrow scope by design). - Substantial corrections: mcp_policy (authorize_repo_path's identity+containment, post refused-not-restored, FULL_SHA_RE), mcp_server (unconditional ALLOW_POST=0, (run_id,created) enqueue, configure_strict_repo removal, reap, unknown-run poll contract), cli (claim-based at-most-once, frozen repo_path precedence), run_status (CAS claim vs single-writer, the narrow interrupted re-arm exception), thin_mcp_server (halved; split has happened), engine/steps/pr (one-head-governs-everything, expected_head_sha hard gate), task_spec, engine/steps/_common (repo_path is no longer side-effect-free), review/planner + engine/steps/review (planner_error causes, review_verdict as a state field), engine/lifecycle (require_file_locking fail-closed split), engine/agent_runtime (member_unreachable vs outcome_blocked routing predicate), config (allowed_repo_roots, idem_retention_days). - Date-bump re-verification where content already matched: intent, providers/{base,deepseek,harness_llm,registry}. check_spec_freshness --strict: 71 pages, 0 stale, 0 uncovered; doc links and citations green; full suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EwmmhpCK9pfEsWWzUBx3GB
![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: 4486b09b73
ℹ️ 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".
| - **quick map fail-closed**：`_direct_quick_map` 返回内嵌代码地图与状态 | ||
| `{ok, truncated, unavailable}`，`truncated` 不是装饰 —— 把残图当全图 | ||
| 与缺图同罪、且更难察觉；`_direct_route` 据此置 | ||
| `read_required = status != "ok"`（"自己去打开"是真回退，"什么都不给 | ||
| 又不许看"不是）。 |
There was a problem hiding this comment.
For supported adapter-backed repositories such as afd-plugin, _adapter_changed_file_routes bypasses _direct_route, stores _direct_quick_map(...) as a tuple, and hardcodes read_required=False. For afd_plugin/config.py, the public router therefore returns quick_map: ('', 'unavailable') while telling the host not to read the page, so the asserted fail-closed invariant and the resulting knowledge-read budget are false for an entire supported review path.
Useful? React with 👍 / 👎.
| - `materialize` 对 git 级失败**绝不抛**（调用方决定 block 还是降级）， | ||
| 但平台无文件锁**必须抛**（`lifecycle.require_file_locking` —— 序列化 | ||
| 就是保证本身）。 |
There was a problem hiding this comment.
When the injected production runner in engine/steps/_common.py::git exceeds its subprocess.run timeout, it raises subprocess.TimeoutExpired, but worktrees.materialize catches only OSError. The exception therefore escapes _pr_time_checkout instead of returning (False, detail) for the caller to block or degrade as this invariant promises; either catch subprocess failures in materialize or narrow the documented contract.
Useful? React with 👍 / 👎.
| 三个分开解决的问题：(1) **每键一个 run** —— 持久的 | ||
| `<run_root>/.idem/<key>.json` 条目 + 阻塞式逐键锁；(2) **每键一次执行** | ||
| —— `reserve` 报告自己是否*创建*了 run，只有创建才入队；(3) **崩溃安全** |
There was a problem hiding this comment.
The durable entry is not stored as .idem/<key>.json: _entry_path hashes the validated key and uses the first 32 SHA-256 hex characters as the filename. Operators or maintainers following this new layout specification to inspect or repair a key will look for a file that cannot exist, so document the hashed filename while retaining the key inside the JSON payload.
Useful? React with 👍 / 👎.
| - **`interrupted → queued` 是对"终态不再迁移"的一个刻意的、狭窄的例外**： | ||
| 仅当 `interrupted` 且 `child_pid` 为 null（属主 server 在任何子进程启动前 | ||
| 就死了，预约从未执行过 —— 不是真正的终局结果）时，`reclaim_queued` 才 | ||
| 允许同一属主身份重新武装；带 pid 的 `interrupted` 做过部分工作，是真结果， | ||
| 拒绝重占。 |
There was a problem hiding this comment.
In the crash-retry scenario described here, the original owner server is dead and the retry normally has a different server_id; reclaim_queued deliberately replaces both owner fields with that current server's identity. Saying that only the same owner identity may re-arm the run reverses the recovery contract and could lead a maintainer to add a check that makes post-crash relaunch impossible.
Useful? React with 👍 / 👎.
1 participant
Main's CI has been red since the
agent/pr2-public-mixed-mode-contractmerge (d47a0fa) landed without its SPEC updates: 18 stale pages and 4 uncovered modules on the freshness gate. This PR re-verifies every page against the merged code — one fix, one PR.contract.md(the one-way public consumer surface),direct_routing.md(tables + mechanism out of thin_mcp_server, known debt named),engine/worktrees.md(identity / verification / liveness — each a fixed real incident),idempotency.md(one run AND one execution per key; deliberately narrow scope).authorize_repo_pathidentity+containment,postrefused-not-restored), mcp_server (unconditionalALLOW_POST=0,(run_id, created)enqueue,configure_strict_reporemoval, reap, unknown-run poll contract), cli (claim-based at-most-once, frozenrepo_pathprecedence), run_status (CAS claim vs single-writer, the narrowinterruptedre-arm exception), thin_mcp_server (halved; the page's own predicted split has happened), engine/steps/pr (one-head-governs-everything,expected_head_shahard gate), task_spec, engine/steps/_common (repo_pathis no longer side-effect-free), review/planner + engine/steps/review (planner_errorcauses,review_verdictas a state field), engine/lifecycle (require_file_lockingfail-closed split), engine/agent_runtime (member_unreachablevsoutcome_blockedrouting predicate), config (allowed_repo_roots,idem_retention_days).Gates:
check_spec_freshness --strict71 pages / 0 stale / 0 uncovered; doc links + citations green; full suite green.Suggested merge order: this PR first, then #110 (whose CI currently inherits main's red freshness gate).
🤖 Generated with Claude Code
https://claude.ai/code/session_01EwmmhpCK9pfEsWWzUBx3GB