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

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.
  • body — HookBodySchema (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/spec minor (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

claude added 3 commits August 24, 2026 19:03
#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 0392a120d5114d1e0389cec8de6c3932c7307df3 → packageMentionDocs.

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.

@github-actions github-actions Bot added documentation Improvements or additions to documentation protocol:ui tests tooling labels Aug 24, 2026
@os-warren
os-warren marked this pull request as ready for review August 24, 2026 21:45
@os-warren
os-warren added this pull request to the merge queue Aug 24, 2026
Merged via the queue into main with commit 387e231 Aug 24, 2026
33 checks passed
@os-warren
os-warren deleted the claude/issue-11519-action-doubled-redirect branch August 24, 2026 22:19
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 protocol:ui size/m tests tooling

Projects

None yet

2 participants