Skip to content

fix(runtime): resume refuses a type-mismatched value on an accepted key instead of dropping it (#9416) - #9685

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-9416-resume-value-type-refusal
Aug 18, 2026
Merged

fix(runtime): resume refuses a type-mismatched value on an accepted key instead of dropping it (#9416)#9685
os-zhuang merged 2 commits into
mainfrom
claude/issue-9416-resume-value-type-refusal

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#9416

POST /api/v1/automation/:name/runs/:runId/resume type-guarded each accepted key and
silently skipped a value that failed the guard. So {"inputs": "a string"} passed
#8796's closed KEY set — the key IS accepted — lost its value in the guard, reached the
engine as an empty signal, and answered HTTP 200 success:true with the submission
treated as empty
: the run completed and the caller was told its screen input landed
when nothing did. Identical for {"output": 42}, {"branchLabel": 7}, a non-object JSON
body, and an empty-array body.

That is exactly the failure shape #8796 measured for unknown keys, reached through a
well-spelled key with a mis-shaped value.

The ruling implemented

Option A, as ruled on the card: refuse a type-mismatched value on an accepted key —
400, located, naming the key and the expected type. Non-object and array bodies refuse
the same way. It inherits #8796's closed-key-set ruling together with its reason, plus
the #3899 toggle-arm precedent (a truthy non-boolean enabled is refused there, never
coerced or dropped).

Option B — forward the raw value and let the engine judge — was rejected on the card and
is rejected in the code comment for the reason that decided it: ResumeSignal types
variables/output as Record[string, unknown] and branchLabel as string, so
forwarding hands a service a shape its own contract excludes. An array is in that
set, and the old typeof === 'object' guard forwarded it.

No new envelope and no new code: the refusal reuses the file's own validationFailure
helper and the invalid_type / unknown_field ADR-0114 catalog members the sibling arms
already answer with. Both dispatcher error exits map it to
400 VALIDATION_FAILED + details.fields[] (#3918).

What changed

packages/runtime/src/domains/automation.ts, the resume signal-assembly block only:

  1. Body shape, first.body ?? {} must be a non-array object. A JSON
    string/number/boolean body and an empty array used to normalise to {} and answer
    200 on an empty signal; they now answer 400 located at (body), naming the accepted
    keys. undefined / null stay the legal bodyless resume.
  2. Unknown keys, unchanged (automation resume: the request body's OUTER envelope is lenient — an unknown top-level key is silently ignored and the submission is treated as empty #8796), and deliberately still first among the key-level
    checks
    — a body that is both misspelled and mis-shaped reports the misspelling, which
    is the correction the caller needs first.
  3. Value shapes, new.inputs / variables / output must each be a JSON object;
    branchLabel must be a string. Every offending key gets its own invalid_type entry,
    and both the entry and the message name the key and the expected type (plus the type
    received).
  4. Assembly unchanged in substance. Still field-by-field, never a body spread, so
    automation: the generic run-resume route needs an authorization gate keyed on the suspended node #3801's symbol-keyed service-authority marker stays unforgeable. Every surviving value
    is now known to match the contract, so presence is the only test left.

A key whose value is undefined counts as absent, not mis-shaped: JSON.stringify
drops such a key, so no HTTP caller can produce one, and the in-process spelling
{ inputs: maybeUndefined } means "no inputs". An explicit nullis refused — JSON
can express it, and it was the value dropped most quietly of all.

All six shapes route through one block

Verified, because a refusal added in one place while another limb still drops silently is
a half-fix. POST /automation/:name/runs/:runId/resume has one door: the route ledger
registers it once, dispatcher-plugin.ts forwards req.body to dispatcher.dispatch
handleAutomation → this arm, and packages/rest has no resume route (the approval
decision path goes through ApprovalService in-process, not through this body assembly).
All four value-shape cases and both body-shape cases enter at the same two statements.

Verification — both directions, then ablation

Refusal direction: every refused shape answers 400 with the key and the expected type
named, and the engine is never consulted (so the suspension stays live and a corrected
retry is legitimate). It also stays off FLOW_FAILED, which the console treats as
terminal (#8684 / objectui PR #4899).

Preservation direction — the half a regression does not redden: every already-valid
submission still succeeds with byte-identical arguments at the service — all four
keys, the variables alias, inputs winning when both are sent, empty objects, an
empty-string branchLabel, the bodyless resume, and the inner bag still forwarded
verbatim for the engine to judge.

Ablation (restore the silent-skip guards, predict, run, restore):

predictedactual
red3030
green1111

Run: 30 failed | 21 passed (51) across the two resume test files — the 21 green are the
11 predicted green in the new file plus all 10 of the #8796 sibling file, which the
ablation leaves untouched and which stayed 1 passed at the file level.

The one new-file test predicted green under ablation is the ordering pin
("reports an unknown KEY ahead of a mis-shaped value"): the #8796 unknown-key arm survives
the ablation, so that test cannot redden for this defect and is not claimed to. Every
other refusal test reddened, including the non-empty-array one — its locator moves from
the key check back to (body), which is a real observable difference, not a vacuous pass.

Ablation applied with git checkout HEAD~1 -- packages/runtime/src/domains/automation.ts
(the branch point is exactly the pre-fix file) and restored with
git checkout claude/issue-9416-resume-value-type-refusal -- ...; the post-restore diff
against the branch was empty.

Gates

Local gates were derived from the actual changed paths with
node scripts/pm/dispatch-gates.mjs, re-derived after the main merge, and all run green:

  • pnpm --filter @objectstack/runtime typecheck — clean
  • pnpm --filter @objectstack/runtime test171 files, 2569 tests passed
  • pnpm check:route-envelope · check:cross-package-test-inputs · check:changeset-gate-self-tests
    · check:objectui-changeset · check:nul-bytes
  • node scripts/check-adr-0087-registration.mjs (1 declared-breaking changeset, disposition
    present) · check-changeset-no-major.mjs · check-empty-changeset.mjs
    · check-cross-package-test-inputs.mjs · docs-audit/check-affected-docs.mjs
  • convention-triggered by the new test file: check:engine-double-contract ·
    check:where-matcher · check:query-options-erasure · check:type-check-coverage ·
    check:type-check-debt (closure built first; 33 ledger entries re-measured,
    surplus: none)
  • downstream consumers (prefix direction, ...@objectstack/runtime):
    @objectstack/client 310 tests · @objectstack/hono 73 · @objectstack/http-conformance 72
    · @objectstack/dogfoodflow-durable-suspend 11 — the only consumer that POSTs a real
    resume body over HTTP. All green.

⚠️ One thing worth a reviewer's eye: packages/runtime/tsconfig.json excludes **/*.test.ts,
so pnpm --filter @objectstack/runtime typecheck does not read the new test file. That
package carries a 227-error TEST_DEBT entry for its hidden test layer, and the re-measure
above reports surplus: none — so the new file contributes zero new type errors. The
exclusion itself is pre-existing package state, out of this card's region, and not touched
here.

Verified at 155e19388.


Generated by Claude Code

…ey instead of dropping it (#9416)
`POST /automation/:name/runs/:runId/resume` type-guarded each accepted key and
silently skipped a value that failed the guard, so `{"inputs":"a string"}` passed
the closed key set (#8796), lost its value, and answered HTTP 200 `success:true`
with the submission treated as empty. Same for `{"output":42}`,
`{"branchLabel":7}`, a non-object JSON body and an empty-array body.
Option A as ruled: 400, located, naming the key and the expected type, reusing
the file's own `validationFailure` helper and the ADR-0114 `invalid_type` /
`unknown_field` catalog members. Non-object and array bodies refuse the same way.
Not Option B: `ResumeSignal` types `variables`/`output` as `Record<string,
unknown>` and `branchLabel` as `string`, so forwarding hands a service a shape
its own contract excludes.
Every already-valid submission still succeeds with byte-identical arguments at
the service, including the bodyless resume and the `variables` alias.
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 3 documentable anchor(s).

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

  • content/docs/api/error-catalog.mdx(via invalid_type (literal))
  • content/docs/api/error-handling-server.mdx(via invalid_type (literal))
  • content/docs/automation/flows.mdx(via branchLabel (literal))
  • content/docs/deployment/cli.mdx(via invalid_type (literal))
  • content/docs/protocol/objectui/concept.mdx(via invalid_type (literal))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via invalid_type (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

Coarse fallback — 23 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 fc89098aa1ceb2cca60b1c1204248f445156de5dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from be3b8d70c4f75fe24cc17bcd515808d5427a61f5 — the merge of head 155e19388105e8aafb30c0f62df17d18b6a3627c into base fc89098aa1ceb2cca60b1c1204248f445156de5d, 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 be3b8d70c4f75fe24cc17bcd515808d5427a61f5 && git checkout be3b8d70c4f75fe24cc17bcd515808d5427a61f5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fc89098aa1ceb2cca60b1c1204248f445156de5d 155e19388105e8aafb30c0f62df17d18b6a3627c && git checkout -B drift-repro fc89098aa1ceb2cca60b1c1204248f445156de5d && git merge --no-ff 155e19388105e8aafb30c0f62df17d18b6a3627c
node scripts/docs-audit/affected-docs.mjs --json fc89098aa1ceb2cca60b1c1204248f445156de5d

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 18, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 18, 2026 15:23
@os-zhuang
os-zhuang enabled auto-merge August 18, 2026 15:23
@os-zhuangClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — accepted, ready + auto-merge armed. Both judgement calls confirmed.

domain:cli seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Dev returned status: done, no open questions. CI was still in_progress at review time; auto-merge cannot fire until all six required checks pass.

⭐ The ablation is the strongest this lane has seen today

Predicted in writing before running: 30 red / 11 green in the new file, with the #8796 sibling file staying 10/10 green. Observed: Tests 30 failed | 21 passed (51), Test Files 1 failed | 1 passed (2) — an exact match on both numbers and on the file split.

What makes it more than a lucky number: it named the one new-file test it predicted would stay green (the ordering pin "reports an unknown KEY ahead of a mis-shaped value") and explained why it cannot redden for this defect — the #8796 unknown-key arm survives the ablation. A prediction that also says what will not move is the one that can be wrong; this one could have been, and wasn't.

⚠️ Clause ② — this card DOES change the accept set, and it was authorised

Refusing a JSON string / number / boolean / empty array where they previously normalised to {} and answered 200 is a change to contract accept/reject behaviour. That is Clause ② on its own terms and would normally be fable-mandatory.

It ran at opus because the maintainer's one-time downgrade named this card explicitly#9446, #9416, #9462, #9488, #9487. ⛔ That authorisation does not spread beyond those five, and reverts to default when Fable recovers. Recording it here so the tier is auditable rather than inferred. The changeset is declared-breaking with its disposition present (check-adr-0087-registration green).

✅ Both deliberate calls confirmed

  • null is refused. In scope: null is a type mismatch for an object-typed field, and the dev's note that it "was the value dropped most quietly" is the point of the card. Option A covers it.
  • undefined counts as ABSENT. Correct, and the reasoning is checkable rather than stylistic: JSON.stringify drops it, so no HTTP caller can produce one — over the wire it is indistinguishable from the toggle arm's in-based presence test. It differs only for in-process callers, where { inputs: maybeUndefined } genuinely means "no inputs".

Neither widens anything, so neither needed escalating.

Verified by me, not taken on report

Worth keeping from the report

check:type-check-debt was convention-triggered and load-bearing in a non-obvious way: packages/runtime/tsconfig.jsonexcludes **/*.test.ts, so tsc --noEmit never reads a new test file — while the package carries a 227-error TEST_DEBT entry that a new hidden test file can push up. The dev found this by re-deriving the gate union from its actual changed paths, ran it the way CI does (full closure built first, turbo 70/70), and got "33 ledger entries re-measured, none above its recorded number. surplus: none". A gate that the obvious reading says cannot apply, and that actually does.

It also merged origin/main before opening the PR — replacement-discipline rule ② under the same-package exemption, done without being reminded.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 96f397aAug 18, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9416-resume-value-type-refusal branch August 18, 2026 15:58
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

automation resume: a known top-level key with a type-mismatched value is silently dropped — the submission is still treated as empty

2 participants

@os-zhuang@claude