Uh oh!
There was an error while loading. Please reload this page.
docs(architecture): audit Recovery and resume group against implementation - #4518
Conversation
…ation Audit the seven Recovery and resume documents under apache#3522 against current main. Four documents needed corrections; three were verified accurate as written. Corrections: - recovery-resolver ADR: the Phase 3 decision fact was implemented as the `actions.toolRecovery` envelope (`maka.tool.recovery_decision`, protocol `tool_recovery_v1`) committed through the atomic recovery bundle transaction, not a `tool_recovery_decided` RuntimeEvent; the call+dispatch-without-response row resolves to `indeterminate` with a resolution-level `requiresReconciliation` (reconciled operations settle as terminal `parked` with `reconcile_*` reasons), not a `reconcile_required` status; and the journal projection states are now `prepared | outcome_committed | recovery_completed | recovery_parked`. - extraction ledger and Phase 3-4 design: `PROVIDER_REPLAY_PROJECTION_VERSION` was frozen at 1 by PR B and advanced to 2 by apache#4286 (cross-model reasoning replay gating). - resume architecture (en + zh-CN): the prior-state recovery park reason is the durable `reconcile_matches_prior_state`; `redo_disabled_pending_cas` never became a durable code (the Phase 3-4 design keeps it as a UI mapping note only). Verified accurate with no changes: phase0 crash contract (P0-P11 failpoint table, decision vocabulary, twelve-failpoint harness claim), phase1 safe-boundary contract (flag, planner gates, lifecycle event names, host entry points), and the extraction ledger's file inventory, capability names, and schema milestones. The paired resume-architecture documents move together and both carry `last_verified: 2026-09-02`; `translation_status: synced` is preserved. Refs apache#3522 Generated-by: GLM-5.3-Flash (ZCode)
8c59979 to
c04ecddCompare
Astro-Han
left a comment
There was a problem hiding this comment.
Checked all twelve lines against main at the source each one describes. Ten are right and each is an improvement: the recovery decision now goes through the bundle transaction and maka.tool.recovery_decision (tool-recovery-fact.ts, tool-recovery-bundle.ts), the park reason is reconcile_matches_prior_state (ToolRecoveryParkReason), and PROVIDER_REPLAY_PROJECTION_VERSION is 2 since #4286. Nice work tracing those.
Two things to fix before this merges, both inside the same subsystem the PR audits:
runtime-recovery-resolver-adr.zh-CN.md:100now lists the journal states asprepared | outcome_committed | recovery_completed | recovery_parked. The union insqlite-runtime-store.ts:158has five members;reconcile_observedis missing, and it has a real writer (sqlite-runtime-store.ts:2521,:2706,:2854) plus a covered migration sequence inrecovery-persistence-authority.test.ts. The old line was incomplete; the new wording ("the set reachable by write paths") turns that into a wrong claim.runtime-resume-phase3-phase4-workspace-checkpoint-design.zh-CN.md:456still sayspark, reason=redo_disabled_pending_cas, the same assertion this PR corrects in both resume-architecture files. That name has zero hits inapps/andpackages/, so the sentence at:478about a UI mapping to it describes code that does not exist either. Fix:456toreconcile_matches_prior_stateand drop or reword:478; leaving them is exactly the intra-group contradiction #3522 asks each pass to remove.
Smaller: the decision table at ADR :84-94 has no row for "operation already carries a recovery decision fact", which the resolver handles as its own branch (recovery-resolver.ts:252-264); and the body says four documents changed and three were verified, but the diff touches five and the verified list has four entries.
Evidence boundary: static read of c04ecdda against main92fa5281 and later; no tests run, none needed.
AI-assisted review: drafted with Maka; I verified the journal union, the park-reason grep and the phase3-phase4 lines myself.
简体中文
十二处改动逐条对照 main 源码,十处正确且更准确。合并前两处要改:ADR 第 100 行的 journal 状态集合漏了 reconcile_observed,它在 sqlite-runtime-store.ts:158 的联合里且有真实写入路径,改后的措辞把不全变成了错误断言;phase3-phase4 设计文档第 456 行仍写 redo_disabled_pending_cas,跟本 PR 在两份 resume-architecture 里修掉的是同一条断言,代码里零命中,第 478 行说的 UI 映射也不存在。小项:ADR 决策表缺「已有 recovery decision fact」一行;正文说改了四个文档,实际五个。
| ### 5. Journal 是可重建投影,不是第二份账本 | ||
| Phase 2.5 保留现有表以降低查询成本,但状态缩窄为当前确有写入路径的 `prepared | outcome_committed`。未来 `indeterminate`、`reconciled`、`parked` 若需要查询状态,先定义对应 RuntimeEvent,再扩展 projector。 | ||
| Phase 2.5 保留现有表以降低查询成本,状态缩窄为写入路径可达的集合(现为 `prepared | outcome_committed | recovery_completed | recovery_parked`)。未来新状态若需要查询表示,先定义对应 RuntimeEvent,再扩展 projector。 |
There was a problem hiding this comment.
ToolJournalState at packages/storage/src/sqlite-runtime-store.ts:158 has five members; reconcile_observed is missing here and is written at :2521, :2706 and :2854. With "the set reachable by write paths" added, the line is now wrong rather than incomplete.
me2seeks
left a comment
There was a problem hiding this comment.
Request changes — one residual miss; everything else I checked is verified accurate. The audit's value is real: these documents are treated as architectural authority, and the corrected claims were factually wrong on main. I re-verified each correction against the code:
tool_recovery_decided: zero hits acrosspackages/andapps/— the RuntimeEvent the ADR described never existed. The implemented mechanism is themaka.tool.recovery_decisionfact (tool_recovery_v1;tool-recovery-fact.ts:21,44) committed through the recovery bundle (tool-recovery-bundle.ts:157,167). Correction accurate.reconcile_required: does not exist; the implementation isrequiresReconciliation+ reasondispatch_without_response(packages/runtime/src/recovery-resolver.ts:122,244). Accurate.- Journal projection states:
sqlite-runtime-store.ts:238is exactlyprepared | outcome_committed | recovery_completed | recovery_parked. Accurate. PROVIDER_REPLAY_PROJECTION_VERSION: currently 2 (model-history.ts:82); git history confirms #4286 advanced it from 1. Accurate.reconcile_matches_prior_stateis the durable park reason (tool-recovery-fact.ts:39,77);redo_disabled_pending_cashas no code presence. Accurate.- The single-language docs (ADR, extraction ledger, phase3-4 design) have no English counterpart, so editing zh-CN only breaks no sync; the paired resume-architecture docs moved together with
last_verifiedbumped in both. Correct handling.
Requested fix — the same drift survives in a document this PR audits.runtime-resume-phase3-phase4-workspace-checkpoint-design.zh-CN.md:456 still reads:
| `matches_prior_state` | park,reason=`redo_disabled_pending_cas` |
That is the identical error this PR fixes in the resume-architecture pair, in the same doc group. The PR body says the Phase 3-4 design "keeps it only as a UI mapping note" — line 478 is indeed that note, but line 456 is the recovery-action table, where readers will take redo_disabled_pending_cas as the durable park reason. By this PR's own standard it should become reconcile_matches_prior_state. One-line fix; happy to approve right after.
Verification boundary. Verified by reading origin/main code and git history directly (symbol greps, projection union, version history). Not verified: all 14 extraction-ledger file paths individually (spot-checked symbols only); the #3522 drift greps (taken on trust, low-risk); CI status and prior review threads.
Review prepared with AI assistance; all claims above were re-verified against origin/main before posting.
Per review by Astro-Han and me2seeks on apache#4518: - The ADR journal-state list now matches the full five-member ToolJournalState union; reconcile_observed was missing while the surrounding wording claimed write-path completeness. - The ADR decision table gains the row for operations that already carry a recovery decision fact: settled from the recovery bundle (completed -> recovery_bundle_completed, else parked with the fact's reasonCode; corrupted facts stay corruption), matching recovery-resolver.ts. - The Phase 3-4 design park-reason table and its UI note now use the durable reconcile_matches_prior_state code; the redo_disabled_pending_cas mapping note is removed as it has no code presence. Refs apache#3522 Generated-by: GLM-5.3-Flash (ZCode)
ggbdpq
commented
Sep 2, 2026
Both requested fixes are pushed in f6f187d:
The PR body's document counts were corrected as well (five corrected / two verified-unchanged). Ready for re-review. |
me2seeks
left a comment
There was a problem hiding this comment.
Re-verified after f6f187d: the ADR journal-state list now matches the five-member ToolJournalState union including reconcile_observed (sqlite-runtime-store.ts:158, with real writers), the decision table's new recovery-bundle row matches recovery-resolver.ts:252-264 (completed → recovery_bundle_completed, otherwise parked with the fact's reasonCode, corrupt fact → corruption), and the Phase 3-4 design's park-reason table and UI note now use reconcile_matches_prior_state. All stale terms (tool_recovery_decided, reconcile_required, redo_disabled_pending_cas) return zero hits across the five touched documents. Approving; the requested changes are fully addressed.
Uh oh!
There was an error while loading. Please reload this page.
…ation (apache#4518) * docs(architecture): audit Recovery and resume group against implementation Audit the seven Recovery and resume documents under apache#3522 against current main. Four documents needed corrections; three were verified accurate as written. Corrections: - recovery-resolver ADR: the Phase 3 decision fact was implemented as the `actions.toolRecovery` envelope (`maka.tool.recovery_decision`, protocol `tool_recovery_v1`) committed through the atomic recovery bundle transaction, not a `tool_recovery_decided` RuntimeEvent; the call+dispatch-without-response row resolves to `indeterminate` with a resolution-level `requiresReconciliation` (reconciled operations settle as terminal `parked` with `reconcile_*` reasons), not a `reconcile_required` status; and the journal projection states are now `prepared | outcome_committed | recovery_completed | recovery_parked`. - extraction ledger and Phase 3-4 design: `PROVIDER_REPLAY_PROJECTION_VERSION` was frozen at 1 by PR B and advanced to 2 by apache#4286 (cross-model reasoning replay gating). - resume architecture (en + zh-CN): the prior-state recovery park reason is the durable `reconcile_matches_prior_state`; `redo_disabled_pending_cas` never became a durable code (the Phase 3-4 design keeps it as a UI mapping note only). Verified accurate with no changes: phase0 crash contract (P0-P11 failpoint table, decision vocabulary, twelve-failpoint harness claim), phase1 safe-boundary contract (flag, planner gates, lifecycle event names, host entry points), and the extraction ledger's file inventory, capability names, and schema milestones. The paired resume-architecture documents move together and both carry `last_verified: 2026-09-02`; `translation_status: synced` is preserved. Refs apache#3522 Generated-by: GLM-5.3-Flash (ZCode) * fix(docs): close review gaps in the recovery audit group Per review by Astro-Han and me2seeks on apache#4518: - The ADR journal-state list now matches the full five-member ToolJournalState union; reconcile_observed was missing while the surrounding wording claimed write-path completeness. - The ADR decision table gains the row for operations that already carry a recovery decision fact: settled from the recovery bundle (completed -> recovery_bundle_completed, else parked with the fact's reasonCode; corrupted facts stay corruption), matching recovery-resolver.ts. - The Phase 3-4 design park-reason table and its UI note now use the durable reconcile_matches_prior_state code; the redo_disabled_pending_cas mapping note is removed as it has no code presence. Refs apache#3522 Generated-by: GLM-5.3-Flash (ZCode)
Summary
main. Five documents needed corrections; two (the phase0 and phase1 contracts) were verified accurate as written and are listed below with what was checked.actions.toolRecoveryenvelope (maka.tool.recovery_decision, protocoltool_recovery_v1) committed through the atomic recovery-bundle transaction - not atool_recovery_decidedRuntimeEvent (no such event exists anywhere in the repo). The call+dispatch-without-response row resolves toindeterminatewith a resolution-levelrequiresReconciliation; reconciled operations settle as terminalparkedwithreconcile_*reasons - there is noreconcile_requiredstatus. The journal states reachable by write paths areprepared | reconcile_observed | outcome_committed | recovery_completed | recovery_parked(the fullToolJournalStateunion;reconcile_observedis written by reconcile event appends).PROVIDER_REPLAY_PROJECTION_VERSIONwas frozen at 1 by PR B and advanced to 2 by fix(runtime): gate provider reasoning replay by source model #4286 (cross-model reasoning replay gating).reconcile_matches_prior_state(ToolRecoveryParkReasoninpackages/core/src/tool-recovery-fact.ts);redo_disabled_pending_casnever became a durable code - the Phase 3-4 design previously kept it as a UI mapping note; that note now uses the durable code as well.RUNTIME_RESUME_FAILPOINTSP0-P11 table matchespackages/runtime/src/runtime-resume.ts:234field by field;safe_replay/blocked,dangling_tool_state,runtime_offset_mismatchall exist; the twelve-failpoint harness exists (runtime-resume-crash.test.ts).MAKA_RUNTIME_SAFE_BOUNDARY_RESUME,RuntimeContinuationPlanner,continuationSource/continuation_already_exists, theplan_approved/plan_parked/execution_started/execution_completed/execution_failedlifecycle union (session-manager.ts),sessions:resumeLatest, and the CLI/resumeroute all exist as described.runtime_recovery_authority@1,runtime_continuation_authority@1,continuation_start_v2,continuation_source_v2,PROVIDER_REPLAY_PROJECTION_VERSION, and the workspace-authority file inventory all match.runtime_continuation_claims,runtime_partial_snapshots,commitContinuationStart/commitContinuationRepairStart, the 64-segment lineage cap (runtime-resume.ts:696),retriedFromRunId,continuation_abandoned_before_provider_dispatch,branch_runtime_fact_rewrite_unsupported, PR C observation vocabulary (matches_expected_state/matches_prior_state/diverged/unreadable), and the linked managed-mutation-lifecycle document all resolve.last_verified: 2026-09-02;translation_status: syncedis preserved. The tracking issue's drift greps (run.json,events.jsonl,PermissionEngine,tasks.json,task-events.jsonl) return zero hits across the group.Review fixes (per Astro-Han's and me2seeks's reviews): the ADR journal-state list now matches the five-member
ToolJournalStateunion (addsreconcile_observed), the ADR decision table gains a row for operations already carrying a recovery decision fact (settled from the recovery bundle), and the Phase 3-4 design's park-reason table and UI note now usereconcile_matches_prior_state(theredo_disabled_pending_casmapping note is removed).Verification
git grepper symbol againstpackages/(list above)tool_recovery_decided,reconcile_required,redo_disabled_pending_cas) confirmed absenttranslation_status/last_verifiedfields of the paired docssynced/2026-09-02npm run format:checknpm run check:asf-headersAI use
Audit, cross-checking, and edits were produced with GLM-5.3-Flash (ZCode) under the contributor's direction; the contributor reviewed and is the human contributor of record.
Checklist