Skip to content

fix(approvals): keep an undifferentiable stranded row in the report, and stop a malformed host verdict aborting the scan - #16739

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-16709-strand-inspection-residues
Sep 8, 2026
Merged

fix(approvals): keep an undifferentiable stranded row in the report, and stop a malformed host verdict aborting the scan#16739
os-zhuang merged 2 commits into
mainfrom
claude/issue-16709-strand-inspection-residues

Conversation

@os-trump

@os-trump os-trump commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16709

The three §5 non-blocking residues of the #15358 contract review, taken together. Item 2 is executed per the PM seat's ruling on the card (comment 5577982860): a thrown third read reports 'failed', it does not leave the stranded list.

Every line number and count on the card was measured at head 1b4dad617, before #16704 landed. All of it was re-measured here against origin/main.

Item 1 — the stale-hot drop, pinned, and shown to fail first

restoreConsumedSuspension drops a hot consumed-suspension copy the durable row proves stale, in one line behind one helper flag. The reviewer's ablation E2 deleted that behaviour and the whole package stayed green.

New file: packages/services/service-automation/src/stale-hot-consumed-suspension.test.ts.

The pin is written on the outliving, because that is the only place the drop is distinguishable from a no-op: with the terminal row still present, a dropped and a kept hot copy both answer NO_CONSUMED_SUSPENSION (the row supersedes the copy on every read). So replica A strands a run, replica B restores and finishes it, A's own restore attempt reads the row and drops its copy — then run-history retention evicts the row, and a kept copy would be the only witness left, answering repairable: true, witness: 'journal' for a run that already COMPLETED. Two pins: the read-only verb answers NO_CONSUMED_SUSPENSION, and the restore verb refuses instead of re-arming a finished run.

Red first — E2 re-run against the new pin. Mutation: all three staleHot: hot !== undefined in resolveConsumedSuspensionWitnesses to staleHot: false.

leg evidence
on disk anchor count 3 to 0; blob 45dbeba18 to 8033426867
in dist/ ablation-dist-preflight.mjs @objectstack/service-automation 'staleHot: hot !==' --absent exit 0 — "marker absent from all 6 built files"
ablated run full suite: 1 failed file / 125 passed (126), 2 failed / 1487 passed (1489) — the only failing file is the new pin
restore blob back to 45dbeba18, whole-tree git status --porcelain empty, preflight in present mode exit 0, suite back to 1489 passed

That reproduces the reviewer's reading exactly (every pre-existing test stayed green under E2) and shows the new pin is the thing that notices. The file's third test is a deliberate CONTROL and correctly stays green under E2: it drives a store with no run history at all, where nothing can ever supersede the hot copy.

Item 2 — a thrown third read stays in the report

Per the ruling. The catch arm keeps undetermined++ as telemetry and no longer continues, so the row is reported as the undifferentiated 'failed'.

The reason it is not the same call as the two earlier oracles is in the code and in the changeset: a throw from hasSuspendedRun or getRun leaves it unknown whether the row is stranded, and a storage outage must not be published as a lost run. By the time the third oracle is asked, both have answered — no live pause, terminal failed — and it is asked only which of the three shapes. A read that could not be made is the textbook "could not differentiate", which is what 'failed' already means under the #15358 ruling's item 1.

⛔ No sixth StrandedRunState member; the union is untouched at five.

Item 3 — a malformed host verdict no longer aborts the scan

refineFailedRunState(verdict) moved inside the try that wraps the read. A host resolving undefined used to throw TypeError: Cannot read properties of undefined (reading 'repairable') out of inspectStrandedRequests, so the scan enumerated nothing. It now costs its own row the differentiation and every other row nothing.

The pins for items 2 and 3, and their ablations

packages/plugins/plugin-approvals/src/stranded-request-inspection.test.ts: the existing thrown-read test is re-pinned to the ruled behaviour (it used to assert expect(out.stranded).toEqual([]), and the comment says so), plus a new #16709 block — the malformed-verdict case, a mixed population where one thrown and one malformed row cost only themselves a label, a control that item 2 did not widen to the two earlier oracles (they still skip), and a pin that every label emitted is still one of the five members.

Both ablations proved on disk and in dist/ (ablation-dist-preflight exit 0 on each mutate leg), restored with the blob hash back at HEAD and a clean whole-tree status:

ablation mutation result
A2 — item 2 undone differentiated = false; removed, continue; restored in the catch 4 failed / 37 passed — the thrown-read pin, both mixed pins, and the two-earlier-oracles control
A3 — item 3 undone the refinement moved back outside the try 3 failed / 38 passed — the malformed-verdict pins, with the card's own TypeError in the failure text; item 2's thrown-read pin stays green, which is the attribution

Clause ② — no, derived from the real export surface

Not from a path test. @objectstack/plugin-approvals' published dist/index.d.ts was built at origin/main and at HEAD and compared:

  • declarations only, comments stripped: 11247 lines both sides, diff exit 0 — byte-identical published type surface.
  • Positive control, same pipeline with comments left in: 39 changed lines — so the comparison can report a difference and its silence is a measurement.
  • scripts/pm/check-widening-tells.mjs --declaration no over git diff origin/main...HEAD: no widening tell on any declared surface (its own --self-test, 131 cases, green first).
  • @objectstack/service-automation has no source change at all — item 1 adds one .test.ts and nothing else.

StrandedRunState keeps its five members; ApprovalResumeSurface is unchanged; no barrel is in the change set.

Changeset

@objectstack/plugin-approvals patchAGENTS.md:1029 verbatim: a bug fix in a released package takes a patch changeset, never skip-changeset. Item 3 changes the runtime behaviour of a released package and item 2 changes what the report contains, so "it is mostly tests" does not apply. @objectstack/service-automation is not named: it publishes nothing from this diff.

Gates

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 58 families; all 58 were run and --ran reconciles 58 derived / 58 run / 0 UNRUN. 57 exit 0.

The derivation was first taken on the pre-merge tree and reported STALE TREE; origin/main was fetched and merged into this branch, and the derivation redone at 7e5b34c44.

  • pnpm check:i18n and pnpm check:type-check-debt first answered exit 3 PREREQUISITE NOT MET. Both were cleared locally by building their stated closure, and both then exit 0.
  • ⚠️ One declared narrowing: pnpm check:dual-build-cjs-loads stays exit 3 — it reads built output for 12 packages this diff does not touch (studio, client-react, the connectors, the embedders) and its prerequisite is a whole-tree pnpm build, which CI runs as its own step. Read as NOT MEASURED locally, not as a pass.

Package verification, at the final commit 7e5b34c44: @objectstack/service-automation 126 files / 1489 tests passed, @objectstack/plugin-approvals 42 files / 709 tests passed; typecheck green for both.

验收备注

  • The undetermined counter and stranded now overlap on purpose — a third-read failure is both reported and counted. Stated in the field's own TSDoc and in the changeset, because neither number alone sizes the scan's blind spot any more.
  • No out-of-scope findings were filed. Nothing was noticed that meets the filing bar (a reproducible defect, a violated declared contract, or a trap that makes an AI write metadata the runtime rejects).

🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37


Generated by Claude Code

…and stop a malformed host verdict aborting the scan

Three residues of the #15358 contract review (#16709).

Item 1 (test-only) — the restore verb's drop of a stale hot consumed-suspension
copy was unpinned package-wide: the reviewer's E2 ablation deleted the
behaviour and left the whole service-automation suite green. Pinned where the
drop is distinguishable from a no-op — after the durable row that proves the
copy stale is evicted by run-history retention, a kept copy would be the only
witness left and would offer an operator a restore of a run that already
COMPLETED on another replica.

Item 2 (PM ruling, 2026-09-08) — a thrown third read counted `undetermined`
and dropped the row. By the time that oracle is asked the first two have
already answered (no live pause, terminal `failed`); it is asked only WHICH of
the three shapes the row is, so a read that could not be made is exactly the
"could not differentiate" case `'failed'` already means. The row now stays in
the report; `undetermined` is kept as telemetry.

Item 3 — `refineFailedRunState(verdict)` ran outside the `try`, so a host
resolving `undefined` threw a `TypeError` out of `inspectStrandedRequests` and
the scan enumerated nothing. The refinement now runs inside that `try`: a
malformed verdict costs its own row the differentiation and no other row
anything.

No new `StrandedRunState` member and no widened export.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 3 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 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 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 — 6 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 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7dc2545a2c9e97809d6e2664c086a4622fee2cd2 — the merge of head 7e5b34c444d6f56635cb0d07d35257e9dbc976d3 into base 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871, 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 7dc2545a2c9e97809d6e2664c086a4622fee2cd2 && git checkout 7dc2545a2c9e97809d6e2664c086a4622fee2cd2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871 7e5b34c444d6f56635cb0d07d35257e9dbc976d3 && git checkout -B drift-repro 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871 && git merge --no-ff 7e5b34c444d6f56635cb0d07d35257e9dbc976d3

node scripts/docs-audit/affected-docs.mjs --json 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871

⚠️ 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 1ea349f0ebc4b9a0090af03ecbdea9b1abcef871 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-zhuang
os-zhuang added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 8c7cca1 Sep 8, 2026
35 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-16709-strand-inspection-residues branch September 8, 2026 06:48
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

3 participants