Skip to content

feat(spec,runtime): refuse the doubled post-success navigation channel on script actions - #11841

Merged
os-warren merged 3 commits into
mainfrom
claude/issue-11519-action-doubled-redirect
Aug 24, 2026
Merged

feat(spec,runtime): refuse the doubled post-success navigation channel on script actions#11841
os-warren merged 3 commits into
mainfrom
claude/issue-11519-action-doubled-redirect

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#11519

Ruling carried out

Maintainer ruling 2026-08-24 (issue comment 5393507405), batch acceptance verbatim: 「四维分析一致的,接手你的建议。」 ⇒ refuse the doubled channel; ⛔ no new precedence contract field. Branch chosen by measurement of static knowability, recorded below. The interim renderer behaviour (declared onSuccess wins, objectstack-ai/objectui#5933) stands until this lands; no renderer code is touched.

Clause-②: yes — the PR stays draft, the contract-review chain runs before enqueue.

The measurement — is "the handler can return redirectUrl" statically knowable at parse time?

In general: NO. A type: 'script' action's handler is one of:

  • target — a string naming a handler registered at runtime via engine.registerAction(...); the schema sees only the name, and dispatch resolves it from a registry (packages/runtime/src/domains/actions.ts:419-423, executeRegisteredAction in packages/runtime/src/action-execution.ts). The return value is runtime-only.
  • bodyHookBodySchema (packages/spec/src/data/hook-body.zod.ts) declares language, source, and the L2 knobs capabilities/timeoutMs/memoryMb. No declared return contract exists. Grepping the source text for the word would be a heuristic (false positives on mentions, false negatives on computed/spread keys), not a declared shape.

But a declared marker DOES exist on the action itself:opensInNewTab (packages/spec/src/ui/action.zod.ts, the flag documented as "pre-opens the tab synchronously on click … then drives the tab to the handler's returned redirectUrl"). An author who declares opensInNewTab: true has declared, in schema-visible metadata, that the handler-redirect channel is in use.

So the measured property partitions, and each region lands in its pre-approved branch:

  1. Branch 1 (authoring-time superRefine) for the statically-knowable subset: a type: 'script' action declaring onSuccess beside opensInNewTab: true is refused at parse time, with guidance naming both channels, the interim winner, and the remedy. (packages/spec/src/ui/action.zod.ts, new refine on the ActionSchema chain.)

  2. Branch 2 (loud runtime diagnostic) for the runtime-only remainder — a handler that actually returns { redirectUrl } with no marker declared (the shipped convention honours it regardless: objectui packages/app-shell/src/utils/consoleServerAction.ts, the redirectUrl branch below the spec: SettingsManifest 的 SpecifierSchema 新增 valueDomain 闭合枚举 —— 声明存在时标准域为执行边界,options 退化为 UI 便利列表(#5712 裁决的 spec 半边) #5933-marked precedence block). The seam lives in this repo, not only in objectui — no cross-repo stop needed: the script dispatch holds both the resolved declaration (with onSuccess) and the handler's return value, at two surfaces sharing one helper:

    The new doubledPostSuccessNavigationWarning helper returns the [action-contract] warning text (string-or-null, the actionPermissionError convention); both seams console.warn it. Observe-only: the wire is untouched, and the interim declared-wins precedence stays the decider — the warning says so and names the remedy. warn, not error, per the degradation-log-level rule (nothing claimed-persisted is lost).

Declared surface extension (per the dispatch instruction): the diagnostic's landing site is packages/runtime/src/action-execution.ts + the two dispatch call sites above — measured, not assumed.

Pins

  • Doubled case refused with guidance naming both channels + remedy: packages/spec/src/ui/action-doubled-redirect.test.ts (direct + through getMetadataTypeSchema('action')).
  • Single-channel cases byte-identical: only onSuccess (parse output deep-equal, materialized defaults included), only opensInNewTab, opensInNewTab + newTabUrl, and opensInNewTab: false beside onSuccess all stay accepted.
  • Scope pins: the pair on type: 'api' stays accepted — the ruled sentence is about script actions; recorded as a deliberate boundary.
  • Diagnostic fires exactly on the doubled case, silent otherwise; wire untouched (handler return reaches the client intact); interim declared-wins outcome recorded in the test text: packages/runtime/src/http-dispatcher.actions-doubled-redirect.test.ts (dispatcher seam + 10 predicate pins).

Reverse verification (both legs recorded)

  • Spec half — RED against unfixed source: the 3 refusal pins failed exactly ("expected true to be false" — the doubled declaration parsed clean), all 11223 other spec tests green (pins don't over-fire). Full-suite reading: Test Files 1 failed | 420 passed (421) · Tests 3 failed | 11223 passed (11226).
  • Spec half — GREEN after the refine: 4 files / 204 tests passed across action-doubled-redirect, action-on-success, action, inline-action; then full spec suite 421 files / 11226 tests passed.
  • Runtime half — RED with the helper present but the seams deliberately unwired: the dispatcher-seam pin failed exactly as "no diagnostic on the doubled case" (expected [] to have a length of 1 but got +0), 13 predicate/silent pins green.
  • Runtime half — GREEN after wiring both call sites: 14/14, and the neighbouring dispatch suites (actions-type-dispatch, actions-global-key, actions-identity-addressing, actions-validation-envelope) 64/64.

Verification (readings quoted from head 45c0d4473; the final commit touched only the runtime test file's type annotations)

  • @objectstack/spec tests: 421 files / 11226 passed (verdict line: os-verify-lock: VERDICT command-exit 0).
  • @objectstack/spec typecheck + check:test-typecheck: OK — ledger byte-identical (55 files / 263 errors held).
  • @objectstack/runtime tests: 187 files / 2765 passed; typecheck exit 0 (re-run at head).
  • Consumer sweep (downstream): @objectstack/lint tests 81 files / 2291 passed; @objectstack/example-showcase typecheck clean + 26 files / 362 tests passed — the narrowing reds nothing, matching the measured zero doubled producers.
  • pnpm --filter @objectstack/spec check:generated: 13/14 current; the one stale (test-typecheck-debt.json) regenerated byte-identical and its own gate reads OK.
  • Gate derivation quoted: dispatch-gates: gate list derived from the tree of 'objectstack-ai/objectstack' at commit 45c0d4473 (/home/user/objectstack-11519). All 24 path-matched families run locally green (changeset-gate-self-tests, cross-package-test-inputs, doc-formula-expressions, empty-state, liveness, merge-driver, objectui-changeset, published-files, route-envelope, slot-lookup, spec-parsed-alias, strictness-ledger, test-source-alias, type-source-resolution, variant-docs, adr-0087-registration, changeset-no-major, dev-prereqs, empty-changeset, plugin-teardown-shape, affected-docs, release-rehearsal self-test) plus the convention-triggered set (query-options-erasure, type-check-coverage, engine-double-contract, where-matcher — re-run at head 45c0d4473 after the test-file edit, all exit 0).
  • Declared narrowing on one family: check:type-check-debt --re-measure (re-runs tsc over all 13 DEBT + 19 TEST_DEBT entries) exceeded the container's 10-minute foreground cap twice (SIGTERM at ~10m; its coverage half completed OK both times). Scoped measurement instead: runtime — the only ledgered package this diff adds test code to — probed with its test exclusion removed (tsc --noEmit over extends-config): 227 raw errors, exactly the frozen ledger number, 0 attributed to the new file (the file initially owed 8 and was annotated clean in 45c0d4473). Spec's per-file ledger re-recorded byte-identical. CI runs the full re-measure regardless.
  • Changeset: @objectstack/specminor (launch-window breaking-as-minor; accept-set narrows) + @objectstack/runtime patch, with the ADR-0087 disposition not-required (no-migration-prescription) — zero doubled producers measured in every corpus, and the channel choice is an authoring decision no migration entry can perform.

Out of scope, deliberately

  • No renderer changes (objectui#5933 interim stands until this lands and the pin bump follows normally).
  • No precedence field, no new authorable keys (ruled).
  • The type: 'api' pair and the newTabUrl-requires-opensInNewTab co-constraint are outside the ruled sentence and untouched.

Generated by Claude Code


Generated by Claude Code

#11519)
A type:'script' action could carry two post-success destinations — the
declared ActionSchema.onSuccess block and the handler-returned
{ redirectUrl } — with the spec ruling neither, leaving renderer-side
precedence to decide silently (interim: declared wins, objectui#5933).
Maintainer ruling 2026-08-24: refuse the doubled channel; no precedence
field. Measured static knowability partitions the fix:
- Statically knowable half: opensInNewTab: true is the schema-visible
marker of the handler-redirect channel, so onSuccess beside it on a
script action is refused at authoring time by a new refine, with
guidance naming both channels and the remedy.
- Runtime-only remainder: a handler that returns redirectUrl with no
marker is diagnosed loudly at the dispatch seam
(doubledPostSuccessNavigationWarning), wired at both surfaces that
hold the declaration and the handler result — the REST /actions route
and the MCP run_action bridge. Observe-only: the wire is untouched
and the interim renderer precedence stays the decider until the
author takes the remedy.
Single-channel cases (only onSuccess, only opensInNewTab, opensInNewTab
+ newTabUrl) stay accepted byte-identically, pinned; the corpus was
measured at zero doubled producers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy
The runtime TEST_DEBT ledger entry is frozen at 227 raw tsc errors with
the test exclusion removed; the new test file initially owed 8 (TS18048
x2, TS7006 x6). Annotated the spy-call lambdas and optional-chained the
dispatcher response so the scoped re-measure reads exactly 227 with zero
attributed to this file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/spec, touching 6 documentable anchor(s).

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

  • content/docs/deployment/troubleshooting.mdx(via ActionSchema (symbol))
  • content/docs/protocol/objectui/actions.mdx(via ActionSchema (symbol))
  • content/docs/ui/react-pages.mdx(via onSuccess (literal))

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

  • content/docs/releases/v17.mdx(via ActionSchema (symbol), onSuccess (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
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 132 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 0392a120d5114d1e0389cec8de6c3932c7307df3packageMentionDocs.

Which tree this was computed on

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

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

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationprotocol:uisize/mteststooling

Projects

None yet

2 participants

@os-warren@claude