Skip to content

test(lint): pin the dispatch-contract disarm by an undeclared same-named action, and state it in the docblock - #19781

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-17923-dispatch-contract-disarm-pin
Sep 23, 2026
Merged

os-support-ai merged 1 commit into
mainfrom
claude/issue-17923-dispatch-contract-disarm-pin

Conversation

@os-support-ai

Copy link
Copy Markdown
Collaborator

Fixes #17923

Clause-②: no

What this is

validateActionDispatchContract goes silent for an action name when any same-named action in the stack declares no execution. That behaviour is deliberate and stays as it is. This PR adds pins for it and rewrites the collectDeclaredContracts docblock to explain the mechanism. No logic change: every changed line in validate-action-dispatch-contract.ts is a comment line, and the built JS and .d.ts are byte-identical to the base (see Changeset below).

The mechanism, checked on the base

collectDeclaredContracts puts global and object-embedded actions into one flat name namespace. When an action declares nothing, contractOf(a.execution) ?? 'none' adds 'none' to that name's contract set. Then if (contracts.size !== 1) continue; drops the name. So a missing declaration counts as a disagreeing one. I checked this at the claim's base 041c8cf62b and again at this branch's base 70a2e752b8. Between those two commits packages/lint has no diff.

Changes

  • packages/lint/src/validate-action-dispatch-contract.test.ts gets a new describe block with five tests. They share one stack builder: one view on task wires recalc as a bare string, and invoice is an object the view never resolves through.

    • CONTROL: a global recalc declared aggregate is refused (1 finding, at views[0].list.bulkActions[0]).
    • CONTROL: a same-named sibling that declares the SAME contract, on task and again on invoice, leaves that finding in place. So the silence comes from the sibling's missing declaration, not from a second declaration being there.
    • Disarm, three cases, each run beside the same stack without the sibling (1 finding):
      • an undeclared recalc embedded in task gives 0;
      • an undeclared recalc on the unrelated invoice gives 0;
      • an undeclared global recalc next to an object-embedded declaration gives 0.
  • packages/lint/src/validate-action-dispatch-contract.ts: the collectDeclaredContracts docblock now says it directly:

    • absence counts as disagreement;
    • one undeclared same-named action anywhere in the flat namespace turns the rule off for that name, for every view that wires it;
    • declarations are not scoped by the view's object.

    It keeps the existing reasoning (zero false positives, silence as the safe direction, "undeclared is not defaulted"). It adds one point: treating an undeclared sibling as agreement would bring back that silent default through the sibling.

The third option from the review is not in this PR: scoping declarations by the view's object would change behaviour.

Ablation: the pins can fail

  • Mutation: const declared = contractOf(a.execution) ?? 'none'; becomes const declared = contractOf(a.execution); if (declared === undefined) return;, which deletes the 'none' arm.
  • How it was applied: with scripts/ablation-replace.mjs in wrap mode. On disk: anchor 1 to 0, marker 0 to 1, blob 3be3e4678b42 to 2bcd771ce874.
  • Restore: with git checkout HEAD -- ABS_PATH. It was checked by blob hash against HEAD: plus an empty git diff HEAD, and a shell trap covers EXIT/INT/TERM.
  • No build leg: the test imports the subject by relative path from src/, not through exports/dist.
leg tree run result
before the pins 041c8cf62b (claim base) whole @objectstack/lint suite, mutated 108 files, 4106 passed, 5 skipped. No test fails, which confirms the card's claim
after the pins 4433ece870 (head) validate-action-dispatch-contract.test.ts, mutated 3 failed, 16 passed: exactly the three disarm pins. Both controls and all 14 older tests stay green
restore 4433ece870 same file, from the committed state 19 passed; blob 3be3e4678b42 equals HEAD:, and git diff HEAD is empty

Changeset: none (the skip-changeset route)

@objectstack/lint ships files[] = dist, README.md, CHANGELOG.md. It does not ship src/**.

  • Grep at the head, after a build, over those paths:
    • the docblock phrases ABSENCE COUNTS AS DISAGREEMENT, arriving through a sibling and undeclared is not defaulted get 0 hits;
    • the positive controls hit: action-dispatch-contract-mismatch in 6 files and collectDeclaredContracts in 4.
  • Byte comparison: I built the package with the rule file at base and at head. index.js, index.cjs, runtime.js, runtime.cjs and all four .d.ts/.d.cts files have identical sha256. Only the sourcemap line mappings move, and the maps carry no source text (check:sourcemap-no-sources-content green).
  • The test file is not a tsup entry and ships nowhere.

So nothing in this PR reaches a consumer. It takes the skip-changeset route, and the seat applies that label.

Verification (head 4433ece870)

  • Tests: pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2: 108 files, 4116 passed. That is the base's 4111 plus the 5 new tests. The base's 5 skips ran here because the lint dist/ was built for this run.
  • Typecheck: pnpm --filter @objectstack/lint run typecheck exits 0. check:test-typecheck is OK, its ledger is unchanged, and --listFiles shows this test file is in the tsconfig.test.json program with 0 errors against it.
  • ESLint, narrowed and proven:
    • eslint --no-inline-config --format json on the two files: 2 files, 0 errors, 0 warnings;
    • --print-config resolves 6 and 5 rules for them, so neither file is ignored;
    • type-aware linting is not enabled (parserOptions.project and projectService are both null). So this diff cannot change the verdict for any file it does not touch. The repo-wide pnpm lint is left to CI.
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derived 52 commands. --ran reconciles them as 50 run at exit 0, 2 NOT MEASURED, and 0 unrun.
    • check:lean-entry-closure first exited 3 (prerequisite). It passed after building the @objectstack/objectql closure.
    • NOT MEASURED: check:dual-build-cjs-loads and check:type-check-debt both exit 3. They need the whole workspace built (turbo run build --filter='./packages/*' --filter='./packages/*/*'). The only package this diff touches has byte-identical JS and .d.ts output, and its build-config source change is comment-only. CI's Lint & Repo Gates runs both on a fresh full build.
    • The three roster gates flagged for a packages/ path also pass: check:authz-resolver, check:error-code-casing and check:filter-alias-parity.

Acceptance notes

  • The file header's section "What this rule refuses — and what it deliberately does not" still says only that the rule "says nothing about an action that declares NO execution". The claim limited this round to the collectDeclaredContracts docblock, so the header is unchanged. The docblock is where the mechanism is now stated.
  • The branch was rebased from 041c8cf62b onto 70a2e752b8 before its first non-empty push, because the gate derivation flagged the tree as stale. The push was a fast-forward from the empty-branch push; nothing was force-pushed.

Generated by Claude Code

… and state it in the docblock

`collectDeclaredContracts` pools global and object-embedded actions into one
flat name namespace and counts an action that declares no `execution` as a
disagreeing declaration, so one undeclared same-named action anywhere in the
stack turns `action-dispatch-contract-mismatch` off for that name. The
behaviour is deliberate and unchanged; nothing pinned it (deleting the
`'none'` arm left the whole package green) and the docblock did not say it.

- Five tests: the lit control, an agreeing-sibling control, and the disarm
  from the view's own object, from an unrelated object and from the global
  list, each beside the same stack without the sibling.
- The docblock now states the mechanism: absence counts as disagreement,
  declarations are not scoped by the view's object, and why silence is the
  intended direction.

No logic change.

Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/lint/src/validate-action-dispatch-contract.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/lint/src/validate-action-dispatch-contract.ts) — pages documenting those are invisible to this run
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 4 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 67add1301aa2bce363953b3f424f416d43bf29f0 → packageMentionDocs.

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4433ece870fe9d52f758db2901f122b4c325f4a2

Card #17923, one round. Base main, merge-base 70a2e752b8. Reading moment: 2026-09-23T03:49Z. A record binds ONE head and one reading moment; it is ⛔ not a standing statement about CI.

Tier, measured by the SEAT from the reviewer's transcript (⛔ not its self-report): CONTRACT_REVIEW_TIER on origin/main is claude-fable-5-1. Governing assistant-request stamps 116 / 116 claude-fable-5-1; superset "model" + "advisorModel" 256 / 256 the same value; dark control empty; the fallback/overload sweep, read in context, is tool-schema text and repo prose — ⛔ none a notice. ⇒ AT TIER. This seat is off-tier and ⛔ did not 自审.

① Derived judgments

  • No logic change, proved three ways that formatting cannot fool. The rule file's base and head blobs differ, but stripping comments with typescript transpileModule, with esbuild transformSync, and with the TS printer over the parsed AST gives byte-identical output on both sides for all three. The .ts hunk sits wholly inside the collectDeclaredContracts docblock.
  • The pins pin. The reviewer re-ran the ablation with its own helper, removing the 'none' arm. The rule's test file then has 3 failed / 16 passed: exactly the three disarm pins (a sibling on the view's own object, on an unrelated object, and a global beside an object-embedded declaration). Both controls and all 14 pre-existing tests stay green. The whole lint suite under the mutation reads 3 failed / 4108 passed / 5 skipped, the same three. The restore was verified by blob hash with an empty porcelain. Unmutated head: 4116 / 4116.
  • Each pin carries its own lit control. Every disarm test first asserts one finding on the identical stack without the sibling. An agreeing-sibling control (a sibling that DOES declare the same contract) leaves the finding standing, which isolates the cause.
  • The docblock is true on its literal reading, clause by clause, including 「anywhere in the flat namespace」: the map pools exactly the global actions and every object's embedded actions, the same two sources validate-action-name-refs resolves against, and no view-object filter exists in the file.

② Semver level

No release; skip-changeset (route 2) is right. @objectstack/lint ships dist, README.md and CHANGELOG.md. Built at base and at head, all ten non-map dist files are byte-identical. The four .map files differ in mappings only, and none carries sourcesContent. No .changeset row is changed, so it is ⛔ not route 0.

③ Boundary flags

None tripped. Two files (+107 / −6), no governed path, Fixes #17923, line-leading Clause-②: no, model-free trailers (check-commit-card-trailers exit 0).

CI at this head, the reviewer's final reading: 34 names, 28 success + 6 skipped, 0 failure / in progress / queued / cancelled. All seven required contexts success. Check Changeset reads skipped on the post-label run; its pre-label red is superseded.

Non-blocking, recorded, ⛔ not a round:

  • The docblock says the disarm applies 「for every view that wires it」. It also silences object-embedded listViews wirings, so the sentence names fewer wiring sites than exist, but it is true as stated.
  • An execution value outside the contract vocabulary also enters the set as 'none'. The docblock does not claim otherwise; whether a schema-parsed stack can carry one is unmeasured.
  • The file header's 「What this rule refuses — and what it deliberately does not」 section still states only the undeclared-action case. It was outside this claim's surface, and the dev listed it in the PR's Acceptance notes.

Implemented-by: claude/issue-17923-dispatch-contract-disarm-pin
Reviewed-by: session_013RDBh5DqXd2xnLwvHLgLFr

VERDICT: PASS


Generated by Claude Code

@os-support-ai
os-support-ai marked this pull request as ready for review September 23, 2026 03:50
@os-support-ai
os-support-ai added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit 531689c Sep 23, 2026
39 of 40 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-17923-dispatch-contract-disarm-pin branch September 23, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

2 participants