Skip to content

fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909) - #13934

Merged
os-steve merged 4 commits into
mainfrom
claude/issue-13909-resume-strand-visibility
Sep 1, 2026
Merged

fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934
os-steve merged 4 commits into
mainfrom
claude/issue-13909-resume-strand-visibility

Conversation

@claude

@claude claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Part of #13909 — slice 1 of two deliverables. The resume ordering is deliberately unchanged; deliverable (b) is a measured report, below, not a code change.

(a) The inspector can now see the shape — and still nothing else

ApprovalService.inspectStrandedRequests scans ['approved','rejected','returned'], so it always saw the row. Its second oracle threw the row away:

if (terminal) continue;    // the run ran to a terminal state — it is not dangling

The existence of any run-history row read as health. But in this shape the terminal row is written by the failure that stranded the request: the engine consumes the suspension before running the downstream nodes (AutomationEngine.resumeInternal calls forgetSuspendedRun(run, 'resumed') and only then traverseNext), so a node that merely threw threw with the pause already gone, and the catch arm recorded failed. The inspection therefore reported 0 for exactly the shape an operator most needs.

The second oracle now classifies the run instead of merely detecting it (classifyStrandedRunState). failed is reported with runState: 'failed'; the pre-existing "no history row at all" shape is reported as runState: 'missing'.

The negatives are the load-bearing half, and each has its own reason and its own test:

run history verdict why
failed reported (runState: 'failed') pause consumed, node threw, nothing can resume it
absent reported (runState: 'missing') the original shape this inspection was built for
completed skipped the decision advanced the flow and the flow finished
cancelled skipped deliberate operator termination (ADR-0044) — the run-side twin of a recalled request
paused skipped ambiguous: "suspension gone, no terminal row yet" is what a resume in flight looks like; condemning it would name every concurrently resuming approval
unrecognised skipped the spec's ExecutionStatus vocabulary is wider than the four statuses the engine writes; a future status must not become a silent false positive

The first oracle is untouched: a run the suspension store still holds is alive, and an unreadable store is still counted undetermined, never condemned. The method is still read-only — no status is changed, no run is cancelled.

The sweep's own warning now splits its counts (runMissing / runFailed) because the two shapes need different remedies: a missing run has no history to read, a failed one has a step log and an error naming the node that threw.

Ablation — two mutations, both directions, direction predicted first

Committed first, mutated in the working tree, mutation confirmed on disk by blob hash and marker counts (never an editor's exit code), restored and proven by state. Baseline: 22 tests green.

Ablation A — revert the widening (if (terminal) continue restored, runState dropped from the report). Predicted: the four pins that depend on the widening redden; every negative stays green, because the old code was strictly more conservative.

  • HEAD blob 2580f37c… → mutated blob 740a3ef9… (differs, so the mutation landed); marker classifyStrandedRunState(terminal) 1 → 0.
  • Measured: Tests 4 failed | 18 passed (22) — exactly the predicted four: the two positives, "the OLD oracle would have skipped it", and the mixed-population pin.

Ablation B — over-widen (classifier short-circuited to "anything not completed is stranded"). Predicted: the negatives redden, the positives stay green.

  • mutated blob 3b577cc8…; injected marker present once on disk.
  • Measured: Tests 4 failed | 18 passed (22)cancelled, paused, the unrecognised status, and the mixed-population pin.

Restore proven by state after each leg: git diff HEAD empty, git hash-object back to the HEAD blob 2580f37c…, markers back to their HEAD counts.

Declared controls, not ablation evidence — green in both directions and reported as such: the five negatives are green under A (the reverted code skips them too), and completed + "failed but still suspended" are green under B (B keeps completed skipped, and the first oracle short-circuits before the classifier runs). "NEVER rewrites a stranded row" is green in both.

(b) The ordering question, measured — REPORT ONLY, nothing here changes it

1. What #5512 requires, and whether running traverseNext first would violate it

#5512 is not a requirement about ordering relative to downstream work. Verbatim from the contract it created (NodeExecutor.onSuspensionReleased, engine.ts): a pausing executor arms something external on entry (a one-shot wake-up job, a reminder, a lease), and until #5512 only its own wake path tore that down — so a pause ended by anything else left the armature live. What it requires is:

  1. Exactly one notification per consumption, whichever path consumed it;
  2. routed through the single choke pointforgetSuspendedRun is the only place every consumption (resumed / failed / cancelled) passes, "which is why it — and not any individual caller — notifies the paused node's executor";
  3. delivered after the suspension is gone from cache and store, so "a slow or broken job service can neither delay the continuation nor resurrect the pause", and so teardown failures can be swallowed.

None of those three mentions traverseNext. ⇒ Running the downstream nodes before consuming the suspension would not violate #5512 as written. What it would violate is the requirement stated in the line directly above the call, which is a different one:

// Consume the suspension *before* running downstream work — a run resumes
// exactly once per pause, and a duplicate resume after a partial restart must
// not double-run side effects.

That is an exactly-once-across-a-crash property, and #5512's teardown is a rider on it, not its purpose. The in-process this.resuming guard already covers concurrent duplicates (RESUME_IN_PROGRESS); the ordering is what covers a process that dies mid-traversal.

There is one #5512 clause the ordering does carry, and it is decisive for shape 3 below — from wait-node.ts's own onSuspensionReleased:

The pause is already consumed when this runs, so cancelling cannot strand the run.

The teardown is allowed to cancel the wake-up only because the pause is definitively gone.

2. The third shapes, and what each costs

Shape 2 — consume on success only (traverseNext first, forgetSuspendedRun in the success path).

  • Buys: a thrown node leaves the pause intact, so the run stays resumable. Directly answers the card.
  • Costs: (i) the exactly-once property inverts — a crash during the downstream traversal now leaves the durable suspension row alive, so the next restart re-resumes and re-runs every side effect already performed before the crash. Today's failure mode is "a thrown node strands the run"; this one's is "a crashed process silently double-runs the work", which is worse and invisible. (ii) hasSuspendedRun answers true for the whole traversal window, which changes ApprovalService.assertRunResumable: a second decision arriving mid-resume is refused today (RESUME_TARGET_LOST) and would be admitted under shape 2, landing a durable decision row that then collides with RESUME_IN_PROGRESS. (iii) the wait one-shot stays armed for the traversal window and can fire mid-traversal — harmless in-process (RESUME_IN_PROGRESS), not across processes. (iv) it silently changes what a map re-entry and a re-suspend see in the store, since persistSuspendedRun would now overwrite a row that is still live rather than write a fresh one.

Shape 3 — consume, then re-arm on a caught throw (compensating re-persist in the catch).

Shape 4 — leave the ordering alone; add an explicit operator verb (the parent card's deliverable 2: re-arm / retry a terminally failed run, as a deliberate action).

  • Buys: no change to resume semantics for any pausing node type, so no blast radius; a state a deployment can enter and leave; it composes with either shape above later.
  • Costs: it is a repair verb, not a prevention — a run still passes through the unresumable state, and something must make that state visible first. That "something" is this PR.

⇒ Recommendation for the next slice, on the four axes: shape 4 first, and shape 2 only with a durable claim/lease that keeps exactly-once. Shape 3 is the one to rule out early — its true cost is a new mandatory hook on the NodeExecutor contract, which is a public-surface widening bought to paper over an ordering.

3. What else depends on the current ordering

Five resume callers, all reaching the identical arm (re-verified on this tree by symbol):

  1. plugin-approvals serviceResume / resumeRecordedOutcome — reports RESUME_FAILED, HTTP 500, decision already durable.
  2. Generic REST door POST /api/v1/automation/:name/runs/:runId/resume (packages/runtime/src/domains/automation.ts) — HTTP 400 FLOW_FAILED. Its own in-place comment is the decisive evidence: "Every arm above is a REFUSAL that left the suspension intact and can be retried; what reaches HERE consumed its pause and ran." Any ordering change moves the boundary that comment describes, and the route's arm-by-arm status mapping (404 / 409 / 503 / 400) is written against it.
  3. wait-node.ts — the timer job firing. Its keepArmed / STORE_UNAVAILABLE branch is already the precedent for "a wake-up that fired without consuming the pause stays armed"; a new "the pause survived a throw" outcome needs its own answer here or the one-shot is spent.
  4. wait-node.ts rearmSuspendedWaitTimers — cold-boot re-arm of an overdue run. This is the caller that reads the durable rows shape 2 would leave behind after a crash.
  5. Engine-internal recursion — subflow delegation (resumeInternal(childRunId, signal, true)) and bubbleToParent (resumeInternal(parentRunId, sig, false, summary)). A parent and child both mid-resume mean two suspensions in flight in one call stack, and failAncestors fails every ancestor on a terminal child failure.

Readers of hasSuspendedRun mid-resume (the whole non-test population, measured):

  • ApprovalService.assertRunResumable — the pre-flight that refuses to record a decision against a run that cannot advance (RESUME_TARGET_LOST). It reads false during the whole downstream traversal today, and would read true under shape 2. This is the single most behaviour-visible dependency outside the engine.
  • ApprovalService.inspectStrandedRequests — the first oracle, in this PR. Under shape 2 a run mid-traversal reads as suspended, so this inspection would skip it — correct, but it means the window this PR makes visible is defined by the ordering too.

Verification

  • pnpm --filter @objectstack/plugin-approvals test34 files, 641 tests passed; the inspection suite alone 22 passed (13 pre-existing + 9 new).
  • pnpm --filter @objectstack/plugin-approvals typecheck → clean. ⚠️ Boundary: that package's tsconfig.json excludes **/*.test.ts, measured with tsc --listFiles (edited test file: 0 hits; positive control approval-service.ts: 1), so typecheck says nothing about the test file. The test file is measured by check:type-check-debt --re-measure, which builds a temp project over the hidden test files: OK, 29 ledger entries re-measured, none above its recorded number.
  • Gate family re-derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths passed — the script takes the change set from the merge base), both sections read whole: 30 path-matched families + the convention-triggered sets for a test-file edit and for a package owning an i18n-extract.config.ts.
  • 35 of 36 derived commands green at 8e2ec8fa9, exit codes captured before any pipe. Includes every convention-triggered ratchet: check:engine-double-contract (OK — 727 pinned; no new double, this PR extends an existing test file), check:where-matcher, check:query-options-erasure, check:cross-package-test-inputs, check:type-check-coverage, check:type-check-debt, plus check:i18n and check:dual-build-cjs-loads (both re-run green after turbo run build cleared their prerequisite), check:published-files, check:test-source-alias, check:nul-bytes and a manual control-byte grep over all four changed files (no hits).
  • NOT MEASURED, by its own printed verdict (never a pass, never a red): check-test-completeness.mjs exit 3 — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log", which CI tees and a local run cannot produce. CI's to run.
  • pnpm lint (repo-wide eslint . --no-inline-config) → green at 8e2ec8fa9. Run whole, so no narrowing to declare.
  • All of the above at 8e2ec8fa9, which is this branch's final commit.

Measured / NOT MEASURED

Measured in-repo: the oracle's old blindness and its new verdicts, both directions, by ablation; the five resume callers and the hasSuspendedRun readers, by symbol on this tree; what #5512 requires, from the contract it created.

⚠️ NOT MEASURED, and not measurable from this repository: how many runs are already in this state in any real deployment. This PR makes the condition visible going forward; it is not a census, and an in-repo zero is not a deployment answer — that is the shape the parent card explicitly names. Who must measure it: the deployment operator, with a query over sys_automation_run (status = 'failed') joined against sys_approval_request (terminal status — approved / rejected / returned — with flow_run_id set). Until that number exists, the remedy in the next slice must not be sized from zero.

Scope and surface

  • ⛔ The resume ordering, forgetSuspendedRun and traverseNext are untouched — verify from the diff: no file under packages/services/service-automation is in it.
  • ⛔ The approvals reject door is untouched (that is POST /api/v1/approvals/requests/{id}/reject returns 500 while its effect lands AND strands the workflow run — three inconsistent outcomes from one call #13807's, deliberately sequenced after this card).
  • ⛔ No door was made to return a success code.
  • ⚠️ Declared for the reviewer's tier judgement, not self-cleared: this diff does not touch packages/spec/** (dispatch-gates confirms "no path-derived mandate"), but it does add to a published package's public surface — one exported type alias StrandedRunState and one field on the exported StrandedApprovalRequest. That interface is an output-only reporting shape the service produces and no caller in this repo constructs. Giving the condition itself a platform-level name would land in packages/spec/src/contracts/automation-service.ts, so it is deliberately not done here and stays with the next slice — the report label is documented in place as "not a run state".

Generated by Claude Code


Generated by Claude Code

#13909)

`inspectStrandedRequests` ended its check at `if (terminal) continue` — the
existence of any run-history row read as health. The engine consumes a
suspension before running downstream nodes (`forgetSuspendedRun(run,
'resumed')` precedes `traverseNext`), so a node that merely threw threw with
the pause already gone and the catch arm wrote a terminal `failed` row: the
evidence of the defect was being read as evidence of health, and the inspection
reported 0 for the shape an operator most needs.

The second oracle now classifies the run instead of merely detecting it.
`failed` is reported with `runState: 'failed'`; `completed`, `cancelled` and
`paused` are each still skipped for their own named reason, and an unrecognised
status is skipped too. The first oracle and the read-only posture are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 6 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 479ace68938388ab93b4e865cc3ff6a632de8a74packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9a130ac8b8567b405e3f58f863705387a5713b24 — the merge of head d3f53f0b7485fee93543d373c6380add0aec0558 into base 479ace68938388ab93b4e865cc3ff6a632de8a74, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9a130ac8b8567b405e3f58f863705387a5713b24 && git checkout 9a130ac8b8567b405e3f58f863705387a5713b24
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 479ace68938388ab93b4e865cc3ff6a632de8a74 d3f53f0b7485fee93543d373c6380add0aec0558 && git checkout -B drift-repro 479ace68938388ab93b4e865cc3ff6a632de8a74 && git merge --no-ff d3f53f0b7485fee93543d373c6380add0aec0558

node scripts/docs-audit/affected-docs.mjs --json 479ace68938388ab93b4e865cc3ff6a632de8a74

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 479ace68938388ab93b4e865cc3ff6a632de8a74 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… line shift rotted (#13909)

Pure line rot, repaired by `node scripts/check-system-context-census.mjs --fix`:
the slice-1 edit to approval-service.ts inserted 77 lines above every anchored
elevation read in that file, so all 8 anchors on census row 42 moved by exactly
+77. The row's declared count (8 sites) is unchanged — the population did not
move, only the lines did, which is why --fix accepted the repair instead of
refusing it as a population change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs
@claude

claude Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Anchor rot repaired — check-system-context-census green at e138bf981

A fifth file joins the diff: content/docs/permissions/system-context.mdx, one line, anchors only.

It was pure line rot, and the shift is uniform. Slice 1 inserted 77 lines above every anchored elevation read in approval-service.ts, so all 8 anchors on census row 42 moved by exactly +77 — 850, 959, 2916, 3062, 3229, 3300, 3489, 3529 becoming 927, 1036, 2993, 3139, 3306, 3377, 3566, 3606. That is the 8 [site-without-a-row] + 8 [anchor-is-not-a-read-site] pair CI reported: the same eight sites, seen from both sides.

--fix accepted the repair rather than refusing it, which is itself the evidence that this is a shift and not a population change — the row's declared count (8 sites) is untouched, and no elevation read site was added or removed by slice 1.

  re-anchored content/docs/permissions/system-context.mdx:148  `plugin-approvals/src/approval-service.ts:850` -> `plugin-approvals/src/approval-service.ts:927`
  re-anchored content/docs/permissions/system-context.mdx:148  `:2916` -> `:2993`
  re-anchored content/docs/permissions/system-context.mdx:148  `:3062` -> `:3139`
  re-anchored content/docs/permissions/system-context.mdx:148  `:3229` -> `:3306`
  re-anchored content/docs/permissions/system-context.mdx:148  `:3300` -> `:3377`
  re-anchored content/docs/permissions/system-context.mdx:148  `:3489` -> `:3566`
  re-anchored content/docs/permissions/system-context.mdx:148  `:3529` -> `:3606`
  re-anchored content/docs/permissions/system-context.mdx:148  `:959` -> `:1036`
check-system-context-census --fix: 8 anchor(s) rewritten

Verified without --fix, and with its self-test:

check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read.
check-system-context-census --self-test: all cases passed

The gate family was re-derived, and the .mdx moved it a lot

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, both sections read whole: 30 families before, 53 after (36 to 59 runnable commands) — 23 new, none dropped. The convention-triggered set is unchanged at 8.

All 23 new families were run and are green: check-doc-frontmatter, check-doc-route-spelling, check-docs-section-name, check-section-landing-index, check-system-context-census, lint's check:doc-formula-expressions and check:doc-security-posture, spec's check:docs, check:empty-state, check:liveness, check:skill-examples, check:strictness-ledger, check:variant-docs, check:yaml-examples, and check:corpus-claim-drift, check:doc-anchors, check:docs-audit-scope, check:docs-redirects, check:docs-single-h1, check:merge-driver, check:published-readme-links, check:react-page-adapter-contract, check:role-word.

Four of them first answered PREREQUISITE NOT MET rather than red (the worktree had been torn down after slice 1, so nothing was built) — check:doc-formula-expressions says it verbatim: "Nothing was measured: this gate exited before running a single check … It is NOT a finding." All four are green after pnpm exec turbo run build --filter=./packages/* --filter=./packages/*/*.

Re-verified at the new head, not carried over from 8e2ec8fa9

pnpm --filter @objectstack/plugin-approvals test at e138bf981 -> Test Files 34 passed (34) · Tests 641 passed (641), and the ratchet family re-run green there: check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:cross-package-test-inputs, check:type-check-coverage, check:test-source-alias, check:published-files, check:nul-bytes, check:doc-authoring, check-keyed-text-bounds, check-tenant-audit-census, check-undeclared-dep-imports.

Nothing else changed: the slice-1 repair, its pins and the report stand exactly as reviewed.

Generated by Claude Code


Generated by Claude Code

Copy link
Copy Markdown
Collaborator

条款② 认定(domain:services PM 席 · 载体挂标)

依主文件闸门段:取 PR 实际 diff 为事实,不取卡片语义。本 PR 的路径肢按你自己的 dispatch-gates 读数未命中(不触 packages/spec/src/**),但内容肢命中——

证据 位置
已发布包 barrel 新增导出 packages/plugins/plugin-approvals/src/index.ts +export { type StrandedRunState }
已导出 interface 新增必填字段 StrandedApprovalRequest.runState: StrandedRunState(非可选)

扩大公开面,契约增量真实存在且可复审。你在 PR 正文里写的 "Declared for the reviewer's tier judgement, not self-cleared" 是对的做法——这一行判给 PM,现在裁定如下:

命中内容肢 + 派发档位低于契约复审档位 ⇒ ⛔ 本 PR 暂不入队。 已在 PR 与卡 #13909 双载体同笔挂 needs:contract-review;复审通过后由复审链同笔收口落地(清标即落地),⛔ 不需要你再做任何事。

补一句归因,不是对你的指摘:这张卡本该一开始就派在契约复审档位,派在默认档是 PM 席的选择失误,复审这一趟由此而来。#13951 已因同一原因走了一次 FAIL 返工。

⚠️ 复审只审契约增量 diff(上表两行),不重审 ablation、不重审否定面测试——那部分本 PR 的证据形状已达标:两条 ablation 均先声明方向、on-disk blob 哈希与 marker 计数证实变异、restore 以状态证明,且五条否定面各自一测。


Generated by Claude Code

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

契约复审:FAIL —— 逐字采纳,标签保留

复审席资格,机读在案(⛔ 不是自述): 降档保险丝在本席自会话读回的 last_served_modelCONTRACT_REVIEW_TIER ⇒ 自会话不得复审,改派达档子代理。采信前的转录核验:

assistant 轮次:                    47
盖章 "claude-fable-5":             47
非 fable 盖章(回退证据):            0

⇒ 每一轮都在档、零回退 ⇒ 裁决合法。按「逐字采纳或整体作废」,取逐字采纳。以下为复审席原文,⛔ 未改写、未删节、未润色:


契约复审:FAIL

结论一行

唯一阻塞:runId 字段注释被本次拓宽证伪,改一行注释即可发布。

阻塞项

  1. 已发布接口内两条相邻字段注释互相矛盾,矛盾正是本增量自己造成的。 StrandedApprovalRequest.runId 的注释(PR head packages/plugins/plugin-approvals/src/approval-service.ts:330)仍写着 "The flow_run_id that resolves to neither a suspension nor a run history row." ——这是 [approvals] 存量僵尸请求无人认领:已终态但 run 悬空的请求落在 releaseDeadRunRequests 的盲区里 #4469 时代的定义,main 上原样存在(origin/main 同文件 :260),本 PR 未动它。但本增量的全部意义就是让 runId 确实解析到一条 history row(终态 failed 行)的请求也进入报表:runState: 'failed' 的每一行都直接证伪该句,且与两行之下新增的 runState 注释(:333–:336,"a failed one has a step log and an error message")在同一个已发布接口内互相矛盾。作者对同类锈蚀做过清扫——日志文案从 "flow run gone" 改成了 "flow run unrecoverable"(:3897)——唯独漏了这条对外发布的字段文档。清除方式:将 :330 改为按 runState 分叉的表述,例如:/** The flow_run_idthat resolves to no live suspension and no recoverable run — seerunState: no history row at all ('missing'), or a terminal 'failed' row ('failed'). */。一行注释,不动任何行为。

非阻塞

  1. 已发布方法文档块里 {@link classifyStrandedRunState}(:3759、:3780)指向模块私有、未导出的函数,在面向消费者的 API 文档里是死链。改为反引号代码字面即可。(:3856 是普通注释,不算。)
  2. 命名归属指针漂移:StrandedRunState 文档块末句(:316)说平台级命名是 "service-automation: a resume consumes the pause BEFORE running downstream nodes, so any node that throws leaves the run terminally unresumable — and the only inspector for it reports all clear #13909's own deliverable",而同卡 slice 2(PR feat(service-automation): an operator can put back a suspension a failed resume consumed #13951)已把命名改述为 [Decision] Workflow resume ordering: a thrown node today leaves the run terminally unresumable — which of three shapes, given that the current order buys exactly-once across a crash? #13937 的 same-batch 子项。命名落地时这条已发布指针会锈;届时对齐即可,现在不阻塞。
  3. changeset 声明 patch(.changeset/approvals-inspector-sees-failed-mid-resume.md:2),而增量新增一个导出类型加一个已发布接口的必填字段。仓库先例支持:StrandedApprovalRequest 本身当初就落在 17.0.0 的 ### Patch Changes 节(CHANGELOG.md :1123 节标题、:1917 条目),且该形状自诞生即注明 "Reporting shape only"。仅记录,不改判。

我核了什么 / 我没核什么

Q1 — 必填字段对消费者是否破坏性:是纯输出型,增量为加法。 搜索:git grep -n "StrandedApprovalRequest" e138bf9(全提交树;git ls-files | grep -c "/dist/" = 0,树里没有 dist,搜索与构建产物无关)。命中仅:定义 :326、包内使用 :3799/:3803/:3823、桶文件 index.ts:27,加 changeset/CHANGELOG 散文。全仓唯一构造点是 :3877–:3890 的那一次 stranded.push({...})(runState 在 :3881)。测试(stranded-request-inspection.test.ts:148、:289)是 toMatchObject 匹配器,读不是构造。spec 契约 IApprovalService(packages/spec/src/contracts/approval-service.ts:583)不声明 inspectStrandedRequests(该文件 grep 无命中),所以没有任何实现者被迫产出此类型。反向对照,让零可读:同一 grep 形状查 IApprovalService 命中跨包真实消费者(packages/rest/src/rest-api-plugin.ts:319–:321),查 ApprovalNodeConfig 命中跨包 import(content/docs/references/automation/approval.mdx:16)——同形搜索在别处非零,故此处的零是读数不是盲区。结论:仓内无构造者,必填字段为加法

Q2 — 词表、层级、落包:合格,不预占平台命名。 卡片交付 3 与 #13951 的表述都把平台命名留给后续(spec automation-service.ts / #13937)。本 PR 铸的 'missing' | 'failed' 命名的是证据形状(history row 不存在 vs 存在且 failed),不是条件本身;文档块 :313–:316 明确圈栏 "REPORT only / not a run state"。与 #13951 的拒绝词表(RUN_NOT_FOUNDNO_CONSUMED_SUSPENSION 等)是第三条轴(restore 为何拒绝),映射自洽:missingRUN_NOT_FOUND,旧 strand 的 failedNO_CONSUMED_SUSPENSION;被 restore 救回的 run 重新有活 suspension,经 :3842 的 if (suspended) continue 自动退出本报表——两套词表可共存,无一处已经打架。迁移成本:类型别名纯 type-only,生产点一个(:3857/:3881),日志键两个(:3899–:3900);平台词表若不同,一个 deprecated 别名加 classify 内一次映射即完成,成本低。

Q3 — 值集与产码一致:核实。 classifyStrandedRunState 签名(:267)返回类型显式 StrandedRunState | undefined,编译期封闭;非 undefined 返回恰为 :269 'missing'、:273 'failed',四个 undefined 臂在 :278/:283/:292/:294。唯一生产点 :3857 经 :3858 if (!runState) continue 守卫后写入 :3881,别无写点(全文件 StrandedRunState grep 复核)。default 臂沉默与别名封闭一致——但要照实说:未来引擎若新增一个需要上报的状态,确实需要新的别名成员(拓宽已发布闭合联合,对做穷举 switch 的消费者是破坏),classify 文档块(:259–:264)已明示沉默是有意设计,故记录、不阻塞。

Q4 — 跨包机制断言:在今天的 origin/main(62a137b)上为真。 packages/services/service-automation/src/engine.ts:resumeInternal 声明于 :4438,:4635 await this.forgetSuspendedRun(run, 'resumed'); 先于 try 块内 :4649 traverseNext(map 重入臂 :4647 走 executeNode,同样是暂停被消费后的下游工作,机制断言不受影响);catch 臂 :4734 记 failed。附带核了同文档块另两条跨包断言:引擎写入的 run 状态恰四种——paused(:3322/:3834/:3843/:4563/:4718)、completed(:3764/:4664)、failed(:3858/:3961/:4734/:4960)、cancelled(:5038);AutomationResult.status'completed' | 'paused' | 'failed'(packages/spec/src/contracts/automation-service.ts:281);spec ExecutionStatus 确实更宽(packages/spec/src/automation/execution.zod.ts,test :20 列全八员,含 timed_out/retrying)。均与发布文档块相符。

Q5 — 除阻塞项 1(正是低层级会漏的:diff 里没有 +/− 的一行被增量语义证伪)外,无制造发现。

我没核:⛔ 仓外 npm 消费者是否有人构造 StrandedApprovalRequest —— NOT MEASURED,本仓测不了,需下游消费者普查或 npm 侧搜索,归 PM/维护者。测试套件未实际运行(CI 与已评过的 ablation 复审所有)。消融方法、负例覆盖、changeset 散文、system-context.mdx 行号刷新——按派单划界,未复审。命名最终落 #13937 还是 #13909 的下一片,是 PM 的裁量,不在此判。


PM 侧处置


Generated by Claude Code

…d, and de-link a module-private symbol (#13909)

Contract-review FAIL rework for #13934, blocking item 1 + non-blocking 1:

- `StrandedApprovalRequest.runId` still carried the #4469-era sentence
  ("resolves to neither a suspension nor a run history row") that this PR's
  own widening falsifies — every `runState: 'failed'` row DOES resolve to a
  terminal history row, and the adjacent `runState` docblock said so two
  lines below. The field doc now forks on `runState`: no history row at all
  (`missing`), or a terminal `failed` row (`failed`). Comment only.
- `{@link classifyStrandedRunState}` in the published
  `inspectStrandedRequests` docblock pointed at a module-private,
  non-exported function — a dead link in consumer-facing API docs. Both
  occurrences are now backtick code literals; the ordinary inline comment at
  the call site is unchanged.

No behaviour change. Not done, per the PM's ruling on the review:
the `StrandedRunState` closing pointer (#13937's unruled outcome) and the
changeset's `patch` bump (recorded, not overturned).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Contract-review FAIL rework — landed at d987bbbe9, comments only

Scope = blocking item 1 + non-blocking 1 from the review, exactly as the PM's dispatch drew it. One file, packages/plugins/plugin-approvals/src/approval-service.ts, 7 insertions / 3 deletions, zero behaviour change.

Blocking item 1 — the falsified runId field doc

StrandedApprovalRequest.runId (was :330) carried the #4469-era sentence — "The flow_run_id that resolves to neither a suspension nor a run history row." — which this PR's own widening falsifies: every runState: 'failed' row resolves to a terminal history row, and the adjacent runState docblock said so two lines below, inside one published interface. It now forks on runState, in the file's own backtick voice:

The flow_run_id that resolves to no live suspension and no recoverable run — see runState: no history row at all (missing), or a terminal failed row (failed).

Non-blocking 1 — dead {@link} to a module-private symbol

The two {@link classifyStrandedRunState} occurrences inside the published inspectStrandedRequests docblock (were :3759, :3780) are now backtick code literals — the function is module-private and non-exported, so the links were dead in consumer-facing API docs. The :3856 occurrence is an ordinary inline comment, already a backtick literal, and is untouched.

Deliberately NOT done, per the PM's ruling

Verification at d987bbbe9, exit codes captured before any pipe

  • pnpm --filter @objectstack/plugin-approvals test → exit 0, Test Files 34 passed (34) · Tests 641 passed (641) — identical to the reviewed baseline; the comment-only diff moved nothing.
  • pnpm --filter @objectstack/plugin-approvals typecheck → exit 0. ⚠️ Boundary, as before: that package's tsconfig.json excludes **/*.test.ts, so this says nothing about test files — irrelevant here, no test file touched.
  • Gate family re-derived from the actual diff (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, exit 0, both sections read whole): 53 families (32 pnpm + 21 node), convention-triggered set unchanged at 8 (test-file edit + i18n-owning package) — identical to the accounting already on this PR after the .mdx joined the diff (30 → 53, none dropped). The change-set paths are the same five files; this commit adds no path. ⚠️ The script printed its STALE TREE notice (branch tree is behind origin/main and 24 gate-source files changed across that range) — a property of deriving on the PR branch's tree, same as every prior derivation on this PR; CI derives on the merge tree.

needs:contract-review stays on, the PR stays draft, no auto-merge — landing and label clearance are the PM's after re-review.


Generated by Claude Code

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

契约复审(返工复核):PASS —— 逐字采纳,标签已清

转录核验(采信前必做):

assistant 轮次:                    41
盖章 "claude-fable-5":             41
非 fable 盖章(回退证据):            0

⇒ 每一轮都在档、零回退 ⇒ 裁决合法,按「逐字采纳或整体作废」取逐字采纳。以下为复审席原文,⛔ 未改写、未删节、未润色:


契约复审(返工复核):PASS

结论一行

阻塞项已清:runId 注释对两形态皆真且互证,返工经逐 token 核实为纯注释,越界三项未动。

我核了什么 / 我没核什么

核了(全部在 diff e138bf981d987bbbe9 范围内):

R1 — 阻塞项已清,且不是"删了了事"。 新 docblock(packages/plugins/plugin-approvals/src/approval-service.ts:330-334,新 head):

The flow_run_id that resolves to no live suspension and no recoverable run — see runState: no history row at all (missing), or a terminal failed row (failed).

三点都成立:(1) 对两个形态皆真 —— missing 无 history 行、failed 有终态行但 "no verb moves the run out of that state"(StrandedRunState 自身 docblock 原话),"no recoverable run" 同时覆盖两者;(2) 显式指向相邻的 runState 字段并逐一点名两个 tag,与 :337-341 的 runState docblock 互证而非矛盾;(3) 不是删除旧句留空 —— 旧的单行假句被一段枚举两形态的完整定义替换,读者无需猜。

R2 — 纯注释,机器核实。 命令:用 TypeScript 编译器 scanner 对两版全文件逐 token 重排(排除 comment trivia 与 JSDoc AST 节点,类型 token 全保留),diff 两侧 token 流。读数:两侧各 26,446 token,diff 为空。⭐ 注意第一轮(未排除 JSDoc 节点)恰好只泄出那四处注释 hunk,反向确认 diff 里除这四处注释外别无他物。git diff --stat 亦为 1 文件 7+/3-,与上报一致。

R3 — 无新增契约内容。 token 流全同 ⇒ 无新 export、无类型变更、无 optionality 变更。两处 {@link classifyStrandedRunState} → 反引号:该函数确为模块私有(:267 无 export,barrel src/index.ts 不含它),所在 docblock 属公开方法 inspectStrandedRequests(:3801)—— 死链已除,全 plugin 内不再残留任何 {@link classifyStrandedRunState}。同一 docblock 余下的 {@link releaseDeadRunRequests} 指向公开方法(:3911),可解析,非死链。新注释无一句为假。

R4 — 三个排除项均未动。 (a) StrandedRunState 收尾句 "Giving the condition a platform-level name is #13909's own deliverable."(:316)逐字未变;(b) changeset .changeset/approvals-inspector-sees-failed-mid-resume.md 仍为 patch,返工 commit 的 name-only diff 只有 approval-service.ts 一个文件;(c) 行为零变更,由 R2 直接推出。无越界"顺手修"。

STALE TREE 上报 — 属实,且不影响本复审。 git rev-list --count d987bbbe9..origin/main = 54(≥50 ✓);git diff --name-only d987bbbe9...origin/main -- .github/workflows package.json scripts(即 dispatch-gates.mjsDERIVATION_SURFACE)= 25 个文件(上报 24,origin/main 在其运行后又前移,一致)。关键读数:该 54 commit 区间内 packages/plugins/plugin-approvals 变更文件数为 0 —— 陈旧树影响的是 gate 推导,不影响本增量发布面的任何一句;不构成阻塞。

没核什么:

  • 首轮已裁定的 Q1–Q5、消融方法、负例测试、changeset 行文、system-context.mdx 行号 —— 按令不复审。
  • 编译/测试是否通过:NOT MEASURED(本地无依赖安装;纯注释 diff 使其风险趋零,但绿灯应由 CI 出)。
  • 54 commit 区间内 DERIVATION_SURFACE 之外、plugin 之外的语义漂移:NOT MEASURED,归合并前的 CI 与 merge queue。

PM 侧收口

⭐ 复审席那条 STALE TREE 读数值得单独记一笔,它把上报从「可能有影响」收敛成了「无影响」:54 个提交里 packages/plugins/plugin-approvals 的变更文件数是 0。⇒ 陈旧树动的是 gate 推导面,不是本 PR 的发布面。这正是「零必须可读」该有的样子 —— 没有停在「树旧了」,而是量了旧的那部分和本卡有没有交集。


Generated by Claude Code

…4 shift rotted (#13909)

`check-system-context-census` was red on this branch: 16 problem(s) over 145
anchors and 109 census sites — 8 [site-without-a-row] plus 8
[anchor-is-not-a-read-site], all in
packages/plugins/plugin-approvals/src/approval-service.ts, pairing up exactly
and every pair moved by the same +4 lines
(927/931, 1036/1040, 2993/2997, 3139/3143, 3306/3310, 3377/3381, 3566/3570,
3606/3610).

The cause is this branch's own previous commit: the StrandedApprovalRequest
`runId` docblock was rewritten from one line to five (7 insertions, 3
deletions — net +4), shifting every elevation read site below it in that file.
Row 42 of content/docs/permissions/system-context.mdx had already been
re-anchored once for the earlier head; the rework invalidated it again.

Repaired mechanically with `node scripts/check-system-context-census.mjs
--fix`, which rewrote exactly those 8 anchors and refused nothing — the same 8
sites, one uniform delta, none added or removed, which is the pure-shift case
that tool is for. No anchor was hand-edited. The gate now exits 0 (109
elevation read sites in 20 packages across 45 files, all anchored; 145 anchors
resolve, 27 declared non-read) and its --self-test leg passes.

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

os-dev-report

{
  "issue": 13934,
  "status": "done",
  "branch": "claude/issue-13909-resume-strand-visibility",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/13934",
  "premise_still_valid": true,
  "summary": "CI-red repair on PR #13934 (card #13909). The dispatch diagnosis was exact: check-system-context-census was red with 16 problems that pair into 8 stale page anchors, every pair a uniform +4 line shift in packages/plugins/plugin-approvals/src/approval-service.ts, caused by this branch own previous commit growing the StrandedApprovalRequest runId docblock from 1 line to 5. Repaired mechanically with the gate --fix, which rewrote exactly those 8 anchors on row 42 and refused nothing - a pure shift, not a population change. No anchor was hand-edited; the only file changed is content/docs/permissions/system-context.mdx (1 insertion, 1 deletion). The PR contract files (approval-service.ts, index.ts), the changeset and the tests were left untouched, so the contract-review PASS stands. Pushed as d3f53f0b7485fee93543d373c6380add0aec0558; PR left as-is (not flipped ready, auto-merge not enabled).",
  "tests": "All runs in a dedicated worktree at the PR head (d987bbbe9), pnpm install exit 0 (9.4s).\n\n1) REPRODUCE — node scripts/check-system-context-census.mjs  -> exit 1 (captured before any pipe)\n   verdict line: \"check-system-context-census: 16 problem(s) over 145 anchors and 109 census sites.\"\n   The 16 = 8 [site-without-a-row] (931,1040,2997,3143,3310,3381,3570,3610) + 8 [anchor-is-not-a-read-site]\n   (927,1036,2993,3139,3306,3377,3566,3606), all in plugin-approvals/src/approval-service.ts.\n   Diagnosis CONFIRMED: the shift is uniform +4 on every one of the 8 pairs, no exceptions.\n\n2) --fix — node scripts/check-system-context-census.mjs --fix  -> exit 0. VERBATIM output:\n   re-anchored content/docs/permissions/system-context.mdx:148  `plugin-approvals/src/approval-service.ts:927` -> `plugin-approvals/src/approval-service.ts:931`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:1036` -> `:1040`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:2993` -> `:2997`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:3139` -> `:3143`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:3306` -> `:3310`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:3377` -> `:3381`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:3566` -> `:3570`\n   re-anchored content/docs/permissions/system-context.mdx:148  `:3606` -> `:3610`\n   check-system-context-census --fix: 8 anchor(s) rewritten\n   check-system-context-census: OK — 109 elevation read sites in 20 packages across 45 files, all anchored; 145 anchors resolve, 27 declared non-read.\n   Nothing was refused: grep -niE \"refus|population|skip|left alone|cannot\" over the --fix log\n   returned exit 1 (no match). All 8 rewrites landed on page line 148 (row 42), each exactly +4.\n\n3) RE-RUN without --fix -> exit 0:\n   \"check-system-context-census: OK - 109 elevation read sites in 20 packages across 45 files,\n    all anchored; 145 anchors resolve, 27 declared non-read.\"\n\n4) --self-test -> exit 0: \"check-system-context-census --self-test: all cases passed\"\n   (includes \"FIX: a pure shift is rewritten\" and \"FIX: a population change is REFUSED, never guessed\").\n\n5) SCOPE — git diff --stat: content/docs/permissions/system-context.mdx | 2 +-, 1 file changed,\n   1 insertion(+), 1 deletion(-). git status --porcelain listed that one file and nothing else.\n   The hunk is a single line (148): the 8 anchor numbers in row 42, prose untouched.\n   approval-service.ts, index.ts, the changeset and the tests were NOT touched.\n\n6) DERIVED GATE FAMILY — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack\n   (it derives over the whole branch changeset, 5 paths). Ran the docs-family gates my one-line\n   edit could actually move, exit codes captured before any pipe, all exit 0:\n   check:merge-driver 0 | check:corpus-claim-drift 0 | check:doc-anchors 0 | check:docs-single-h1 0 |\n   check-doc-frontmatter 0 | check-docs-section-name 0 | check-doc-route-spelling 0 | check:nul-bytes 0.\n   DECLARED NARROWING: the remaining families in that derivation are moved by the branch commits I did\n   not touch (approval-service.ts, index.ts, the test, the changeset) and are unchanged by this commit;\n   CI runs the full farm anyway.\n\n7) POST-COMMIT UNION — re-ran the census gate and its --self-test on the committed head\n   d3f53f0b7 (git rev-parse --short HEAD from that run): both exit 0.",
  "mcp_calls": "2 - one add_issue_comment for this report plus one read-back of it",
  "open_questions": [],
  "out_of_scope_findings": [],
  "notes": [
    "WORKTREE NOTE: the local branch claude/issue-13909-resume-strand-visibility was already checked out in a sibling agent worktree (/home/user/objectstack-13934-rework, clean, at d987bbbe9), so git worktree add with that branch name is refused by git. I used a detached worktree at d987bbbe9 and pushed with an explicit refspec (git push origin HEAD:refs/heads/claude/issue-13909-resume-strand-visibility) instead of push -u. The remote branch advanced normally; that sibling worktree local branch ref is now one commit behind and needs a fetch before it pushes again.",
    "My worktree was removed cleanly afterwards (node_modules deleted first, git worktree remove without --force, exit 0).",
    "FORMAT CONFLICT, declared per the standing dev contract: the dispatch asked for a prose report; the contract requires the final message to be exactly this JSON and requires a GitHub copy first. Every item the dispatch asked for is carried inside the tests field above."
  ]
}

Generated by Claude Code


Generated by Claude Code

os-steve commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

PM 裁定:CI 红已修,契约复审 PASS 依然成立,不需要第三轮复审

d987bbbe9d3f53f0b7 只动了 content/docs/permissions/system-context.mdx 一行(1 insertion / 1 deletion),approval-service.tsindex.ts 逐字未动。⇒ 复审席核过的契约增量与它当时读的是同一份东西,PASS 不受影响。

⭐ 而且这不是本席现在替复审席开脱 —— 复审席在裁决里就把这类改动划在了范围外,原话:「首轮已裁定的 Q1–Q5、消融方法、负例测试、changeset 行文、system-context.mdx 行号 —— 按令不复审」。行号刷新本来就不在它管的面上。

这次红是什么,以及为什么它不是 flaky

不是 flaky,是本 PR 自己造成的,而且成因可以一句话说清:返工把 runId 的 docblock 从 1 行改成 5 行,净 +4,于是 approval-service.ts 里那 8 个 isSystem 读点全部下移 4 行,system-context.mdx 第 42 行的锚点集体失锚。

页面锚点(陈旧) 实际读点
927 / 1036 / 2993 / 3139 / 3306 / 3377 / 3566 / 3606 931 / 1040 / 2997 / 3143 / 3310 / 3381 / 3570 / 3610 全部 +4

纯位移(同样 8 个站点,同一个 delta,无增无减),不是 population change。这正是该门 --fix 负责的那一类,而它的 self-test 里就有一条 "FIX: a population change is REFUSED, never guessed" —— 所以「--fix 没拒绝任何东西」本身就是「这确实是位移」的机读证据,不是猜的。

验收读数(dev 报告,退出码均在管道之前捕获):--fix exit 0、逐条列出 8 次 re-anchor 且拒绝为空;不带 --fix 重跑 exit 0(109 elevation read sites … all anchored; 145 anchors resolve);--self-test exit 0;git diff --stat 只有那一个文件。⛔ 没有任何锚点被手改 —— 手改锚点去凑绿是仓内明令禁止的,因为那恰好会把「population 变了」伪装成「位移」。

⭐ 这一轮真正的教训,记在账上

Lint & Repo Gates 不在 required 集里。 如果本席在其余 30 个 check 全绿时就挂了 auto-merge,这个红会跟着 PR 进队列。仓内已有实测先例:一张 required 全绿、非必查门红着的 PR 经队列落地,该仓 main 红了约一小时,逐 PR 连环红到 fix-forward 才止。

⇒ 「入队资格 = PR 上每一个 check 全绿,⛔ 不是 required 子集」这条不是洁癖,它今晚兑现了一次。本 PR 会等 d3f53f0b7全部 check 出结论后再转 ready + 挂 auto-merge。


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review September 1, 2026 00:58
@os-steve
os-steve enabled auto-merge September 1, 2026 00:58
@os-steve
os-steve added this pull request to the merge queue Sep 1, 2026
Merged via the queue into main with commit 59c0891 Sep 1, 2026
35 checks passed
@os-steve
os-steve deleted the claude/issue-13909-resume-strand-visibility branch September 1, 2026 01:21
zhuangjianguo pushed a commit that referenced this pull request Sep 1, 2026
The merge of origin/main routed content/docs/permissions/system-context.mdx
through the os-regen driver, which exits 0 without text-merging and leaves
git's pre-filled OURS side in place. That silently dropped the 16 anchor
re-points main had landed (#13829, #13934, #13910, #13857) while keeping this
branch's single re-point.

This commit takes main's side of the page and re-derives every anchor from the
merged tree with `pnpm gen:system-context-census`, which re-pointed row 21's
metadata-protocol/src/protocol.ts anchor to 1736. Prose is byte-identical on
both sides once line numbers are normalised, so nothing but line numbers moved.
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Sep 1, 2026
…, so `rollbackToPackageCommit` stops planning off the weekday name (objectstack-ai#14036)

* fix(metadata-protocol): order the ADR-0067 commit timeline by instant, not by the weekday name

`created_at` is an engine-injected audit column: not in `datetimeFields`, and
`SqlDriver#formatOutput` repairs it only inside `if (this.isSqlite)`. The live
SQL dialects therefore hand it out of the record read door as a JS `Date` while
the SQLite family hands out canonical ISO-Z text.

Both ADR-0067 commit-timeline consumers compared `String(created_at)`, and
`String(aDate)` is `"Sun Aug 30 2026 18:19:25 GMT+0800 (China Standard Time)"` —
the LEADING token is the weekday NAME, so lexicographic order over those strings
is `Fri < Mon < Sat < Sun < Thu < Tue < Wed`. Unrelated to chronology, and
stable across the whole set, so it is wrong on every run and wrong the same way.

- `listCommits` returned the timeline in weekday-name order while claiming
  newest-first; its own comment stated the assumption ("sort by the ISO
  timestamp") and it was false on the production default driver.
- `rollbackToPackageCommit` both consumed that ordering and re-derived the same
  comparison itself, so neither site could correct the other: it reverted
  `apply` commits OLDER than the target and skipped the newer ones it exists to
  undo.

Both sites now compare canonical absolute instants through `compareAuditInstants`,
a sibling of the `canonicalVersionInstant` helper objectstack-ai#13382 landed one seam over in
this same file. The canonicalisation is reused; the ordering is new, because
`versionTokensAgree` answers equality between client-supplied version tokens and
an ordering question needs `<`/`>`. When either side does not denote an instant
the two are compared verbatim exactly as before, so only instant-bearing pairs
change verdict.

The pin drives a hand-made `Date` — `@objectstack/metadata-protocol` has no
driver dependency and must not grow one — over four consecutive days, the
smallest fixture for which no timezone alignment can make the old weekday
comparison agree with chronology.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L

* chore(gates): re-point the isSystem census anchor and register the new engine double

Both are the gates' own sanctioned repairs for the line/ledger movement the fix
caused, applied with their own tooling and inspected:

- `check-system-context-census --fix` RE-POINTED row 21's anchor
  `metadata-protocol/src/protocol.ts:1664` -> `:1736`, the 72-line shift the new
  `compareAuditInstants` helper block introduced above it. No row was deleted and
  no needle changed; the gate then reports 109 elevation read sites, 145 anchors
  resolving.
- `check-engine-double-contract --write` ADDED one row recording that the new pin
  file pins 1 `findOne` double ("1 added or grown, 0 lost"). The shrink-only
  baseline is untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L

* chore(docs): re-derive the isSystem census after merging origin/main

The merge of origin/main routed content/docs/permissions/system-context.mdx
through the os-regen driver, which exits 0 without text-merging and leaves
git's pre-filled OURS side in place. That silently dropped the 16 anchor
re-points main had landed (objectstack-ai#13829, objectstack-ai#13934, objectstack-ai#13910, objectstack-ai#13857) while keeping this
branch's single re-point.

This commit takes main's side of the page and re-derives every anchor from the
merged tree with `pnpm gen:system-context-census`, which re-pointed row 21's
metadata-protocol/src/protocol.ts anchor to 1736. Prose is byte-identical on
both sides once line numbers are normalised, so nothing but line numbers moved.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants