Skip to content

fix(runtime): POST /automation/:name/toggle joins the manage_metadata write set - #11660

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10243-toggle-manage-metadata
Aug 24, 2026
Merged

fix(runtime): POST /automation/:name/toggle joins the manage_metadata write set#11660
os-zhuang merged 1 commit into
mainfrom
claude/issue-10243-toggle-manage-metadata

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#10243

Implements the maintainer ruling of 2026-08-23 (comment 5386676024), quoted unmodified:

「10950 不考虑存量,其他接受你的建议」

Ruled: APOST /api/v1/automation/:name/toggle joins the manage_metadata write set. One arm on the existing isFlowAuthoringWrite predicate; ⛔ no new capability name (option C was declined on the startup-scope axis). Nothing here re-opens the decision.

The breaking surface, stated plainly

callerbeforeafter
authenticated withmanage_metadata200, flow toggled200 — unchanged
authenticated without it200, flow toggled403 PERMISSION_DENIED, toggleFlow never entered
anonymous401401 — unchanged (#5519 floor answers first)
engine self-invocation (isSystem)200200 — unchanged

The changeset is minor with this table spelled out. The execution doors did not move: POST /:name/trigger, the legacy POST /trigger/:name and POST /:name/runs/:runId/resume keep their posture, and GET /_status still serves enablement to any authenticated caller — this narrows mutating the bit, not observing it.

The fix is one arm, and it is matched to the router rather than approximated

if(method==='POST'&&parts[1]==='toggle')returnparts[0]!=='trigger';

Two boundary decisions, both deliberate, both pinned by their own test:

  • No upper bound on depth. The router's toggle arm tests parts[1] === 'toggle' with no length check, so POST /automation/:name/toggle/anything still reaches toggleFlow. A gate written parts.length === 2 would have been narrower than its own route — a bypass, not a style difference.
  • parts[0] === 'trigger' is excluded.POST /automation/trigger/:name is the legacy execution door and the router answers it above the toggle arm, so for a flow literally namedtoggle the path /automation/trigger/toggle RUNS that flow. Gating it would over-block an execution door — the one thing the ruling did not do, and the mistake finding: GET /automation/:name/runs/:runId/screen still discloses record-derived values to any authenticated caller who knows a run id #7968 records for the paused-run screen read.

Five surfaces carried the old policy. Seven moved.

The ruling named two pins. The claim comment added two prose surfaces. Implementing it turned up two more that would have gone red — not false prose, actually failing tests:

#surfacewhat movednamed in dispatch?
1packages/runtime/src/domains/automation.tsthe predicate + its docblock, the route map, the gate call-site commentthe fix
2.../automation-write-capability-gate.test.tsaudit pin flipped; toggle joins AUTHORING_WRITES so it inherits the whole battery
3packages/qa/dogfood/test/automation-toggle-tenant-scope.dogfood.test.tsre-pointed (below)
4packages/runtime/src/route-ledger.tstoggle removed from the "execution doors outside it" list; the bare toggle row gains a note in its siblings' voice
5packages/qa/dogfood/test/authz-conformance.matrix.ts"three flow-AUTHORING writes" → four; toggle removed from the execution-door list
6.../automation-toggle-unknown-flow.test.ts⚠️would have gone red — its CTX was { userId: 'user_1' } with no capability, so every #7535 error-mapping case would have stopped at the 403 in front of the behaviour it is named after❌ found here
7packages/runtime/src/http-dispatcher.test.ts⚠️would have gone red — the toggle routing case drove AUTHED_CALLER(); switched to the file's existing FLOW_AUTHOR(), and the FLOW_AUTHOR docblock's "every EXECUTION route (trigger / toggle / resume) keeps AUTHED_CALLER, deliberately" is now false and was corrected❌ found here

For 6 and 7 only the caller changes — every mechanism, assertion and expected value is untouched, and each carries a comment saying so, matching how #7019 / #7033 / #10145 handled the same situation in the same files.

Also in surface 1: the handler's route map annotated capability markers on the three #10145 authoring writes, which had none. They were not false, but a map that marks the toggle gate while leaving its three siblings unmarked reads as "those are ungated". Comment-only, declared here rather than slipped in.

What the dogfood pin measures now

⛔ Not deleted. Re-pointed, and which legs changed is the substance:

  • Flipped — the cross-tenant read-back table. That measured a consequence of the route being reachable; the unprivileged tenant is now refused at the door in both directions and all three principals still read the flow enabled: true. Asserting the spy-equivalent (state unmoved) matters: "toggle first, refuse second" would satisfy a status-only assertion and still be the defect.
  • Kept, deliberatelyit('mutates ENGINE state, not the persisted definition'). The gate changed who may toggle, not where the bit lives. That leg is what made the original result independent of the harness's missing row wall (multiTenant: 'posture-only'), and it is still the leg that would fail loudly if enablement ever became org-stamped state a wall could scope. Only its driver changed — an entitled caller, since an unentitled one no longer gets that far.
  • Added — the positive control (an entitled caller still toggles, 200, both directions) and a leg proving the neighbouring execution door still answers tenant A.
  • The "tenant A is genuinely unprivileged" control deliberately still excludes toggle: a control has to be independent of the route under test.

Consumer inventory — measured, and it comes back zero

Axis ② of the accepted analysis flagged this as the one place option A could misstep:「「今天谁在调 toggle」没有被测量」. Measured now.

This repogit grep -n "automation\.toggle\|toggleFlow(\|/toggle", all packages:

  • packages/client/src/index.ts:3202 — the SDK method automation.toggle itself. A library binding, not a UI caller; its test mocks fetch.
  • docs/qa/platform-checklist/areas/automation.json — a manual QA procedure that toggles a showcase flow. Runs as the seeded platform admin.
  • Everything else is oauth2/toggle-disabled (unrelated), UI filter vocabulary (dropdown/toggle), or CHANGELOG prose.
  • Zero callers in packages/console, packages/mcp, packages/rest, packages/cli, examples/*.

The Console UIpackages/console ships no tracked source (4 files: .gitignore, CHANGELOG.md, README.md, package.json), so the question is not answerable from this repo. Measured in the sibling repo instead — objectstack-ai/objectui at e52fac0:

  • git grep "automation/[^\"']*toggle|automation.toggle|toggleFlow"` ⇒ zero hits.
  • Reverse-check (a zero is never self-certifying): the same corpus returns 107 files matching automation, and its source POSTs to automation routes in three places — useConsoleActionRuntime.tsx:546 and RecordDetailView.tsx:802 (/trigger), FlowRunner.tsx:159 (/runs/:runId/resume). Both are execution doors and both stay ungated.
  • FlowStatusDot / StudioDesignSurface.tsx:3227display{ enabled, bound } read from GET /_status. Display only — no enable/disable control exists to lock anyone out of.

No console surface exposes flow enable/disable to ordinary members, so the over-block risk axis ② named does not materialize. The expected migration surface is programmatic SDK callers.

Anti-vacuity — the flipped pin shown failing without the fix

Prediction recorded before running: removing the arm makes exactly 4 cases in automation-write-capability-gate.test.ts fail — the AUTHORING_WRITES refusal case for toggle, the flipped audit assertion, the deeper-spelling case, and the gate-before-body case — while the toggle anonymous-401 case still passes (that floor is domain-wide and independent of this gate), and the positive control, the isSystem bypass and the legacy-execution-door case all still pass because an ungated route answers them the same way.

Measured: 4 failed | 22 passed (26) — and the same four cases, by name:

× POST /automation/:name/toggle (toggleFlow): 403 PERMISSION_DENIED, and the registry is untouched
× [#10243] the gate is ahead of the body checks on toggle too
× [#10243 FLIPPED] POST /:name/toggle is NO LONGER in this block - it is gated now
× [#10243] a deeper spelling cannot slip past the arm - `/:name/toggle/anything`

Predicted 4, measured 4, same identities. With the arm restored: Test Files 1 passed (1) / Tests 26 passed (26).

⚠️The first attempt at this ablation was a no-op, and it is reported rather than quietly re-run. A perl -0pi -e "s/\Q...\E//" with an embedded newline inside \Q...\E matched nothing and exited 0. The on-disk check caught it — arm-count-after=1 — and the run aborted itself as Measurement void instead of reporting a green ablation over an unmutated file. The second attempt removed the line by exact string match and proved arm-count-before=1 → arm-count-after=0, numstat 0 1, with the neighbouring parts.length === 0 arm still present (so the mutation removed the arm, not the function). Restore confirmed by the trap: arm back on disk = 1, git status --porcelain empty.

Mutation proved on disk by grepping the arm text before (1) and after (0) — a zero-match perl -0pi exits 0, so the edit tool's exit code proves nothing. Restore ran under trap … EXIT INT TERM, and the fix was committed first so the restore had a real anchor. No rebuild was needed for this ablation and none is claimed: the audit pin imports ../http-dispatcher.js — a relative path inside the same package — so vitest compiles the mutated source directly and no dist/ sits between the mutation and the measurement.

Verification

All at commit fa0aa1aa, git status --porcelain empty. Every exit code captured before any pipe (cmd > log 2>&1; echo $?), and each gate quoted by its own verdict line, never a bare $?.

Touched test files, each run individually

fileresult
automation-write-capability-gate.test.tsTest Files 1 passed (1) · Tests 26 passed (26)
automation-toggle-tenant-scope.dogfood.test.tsTest Files 1 passed (1) · Tests 8 passed (8)
automation-toggle-unknown-flow.test.tsTest Files 1 passed (1) · Tests 6 passed (6)
http-dispatcher.test.tsTest Files 1 passed (1) · Tests 246 passed (246)
automation-body-validation.test.ts (drives toggle, untouched)Test Files 1 passed (1) · Tests 18 passed (18)
automation-register-error-class.test.ts (drives toggle, untouched)Test Files 1 passed (1) · Tests 9 passed (9)

pnpm --filter @objectstack/runtime typecheck and pnpm --filter @objectstack/dogfood typecheck both exit 0, and both logs echo > tsc --noEmit — checked deliberately, because a --filter matching zero scripts exits 0 and reads as a pass.

Gate union — derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths; the script takes its own change set from the merge base) and re-derived on the final diff: 8 paths, 20 path-matched families + 6 convention-triggered, unchanged between the two derivations. All run, all green. Verdict lines:

  • check-adr-0087-registration✓ 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition · not-required (no-migration-prescription)
  • check:type-check-debtOK — 32 ledger entr(ies) re-measured in 237.1s, 1898 raw tsc error(s) total, none above its recorded numberMEASURED, not refused, after the full closure build (Tasks: 71 successful, 71 total)
  • check:slot-lookup✓ ratchet holds: 107 unswept site(s) in 25 file(s), none new
  • check:engine-double-contractOK · check:where-matcherOK · check:query-options-erasure
  • check-cross-package-test-inputsOK: 16 package(s) read outside themselves, all declared
  • check-ci-filter-parityOK: all 95 declared cross-package glob(s) (80 unique) are covered
  • check:route-envelope✓ self-test passed · check:published-files · check:test-source-aliasOK — 72 packages with tests scanned
  • check:type-source-resolutionOK — 77 packages with a tsconfig.json scanned · check:type-check-coverage
  • check-plugin-teardown-shape✓ 63 Plugin implementation(s) across 4583 source(s)
  • check-empty-changeset✓ No empty-frontmatter changeset introduced · check-changeset-no-major✓ no major bump · check:objectui-changeset · check:changeset-gate-self-tests✓ 118 assertions
  • docs-audit/check-affected-docs✓ 381 cases pass · release-rehearsal-clone --self-test
  • spec liveness family — check:empty-state✓ all classified, check:liveness, check:strictness-ledger✓ 60 file(s), check:variant-docs✓ 18 discriminated union(s)
  • check:nul-bytes✓ 75 assertions, plus a manual control-byte grep over all changed paths: no hits

Not claimed: the repo-wide pnpm lint. CI owns that run. No other narrowing — every family the derivation named was run at the final head.


Generated by Claude Code


Generated by Claude Code

… write set
Implements the maintainer ruling of 2026-08-23 on #10243 (option A): flow
enablement is an authoring write, gated by one arm on the existing
isFlowAuthoringWrite predicate. No new capability name.
BREAKING: a caller holding a session but not `manage_metadata` is answered
403 PERMISSION_DENIED where it previously received 200 with the flow toggled.
The execution doors (trigger / legacy trigger / resume) and every read are
untouched.
The measurement that settled it: the enabled bit is not a row, so no
organization wall scopes it. toggleFlow writes an in-process map keyed by flow
name only, getFlowRuntimeStates() reads it with no caller and no organization,
and the automation service is one instance per environment — so an unentitled
tenant org owner switched a shipped flow off and an unrelated tenant in a
different organization, plus the platform admin, read it off.
The predicate matches the router exactly rather than approximately: no upper
bound on path depth (the router's toggle arm has none, so parts.length === 2
would have been a bypass), and POST /automation/trigger/:name is excluded so a
flow literally named `toggle` keeps its execution door.
Both named pins are flipped deliberately in this commit, plus three prose
surfaces that could never go red (route-ledger, the authz conformance matrix,
the handler's own route map) and two test files that would have gone red
unannounced (automation-toggle-unknown-flow, http-dispatcher) where only the
caller changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/client-sdk.mdx(via automation.trigger (sdk))
  • content/docs/api/plugin-endpoints.mdx(via /automation/trigger/:name (route), /trigger/:name (route))

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

  • content/docs/releases/v17.mdx(via /:name/toggle (route), /automation/:name/toggle (route))

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
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 24 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 cad8b42f009a4a703ccec6c58bdf2995f48f6362packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 cad8b42f009a4a703ccec6c58bdf2995f48f6362 → 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 24, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

ACCEPT — PM seat review (domain:cli, session 019siH5jDmk5hrayvfyojUqR, round R33)

CI at fa0aa1aa: 30 of 36 complete, nothing red; Test Core (1/6), (2/6), the three
Dogfood Regression Gate shards and Lint & Repo Gates still running. Arming when they land, ⛔ not
before, and I will not re-review then.

The surface count went 2 → 5 → 7, and the last two steps were the ones that mattered

The maintainer's ruling named two pins to flip. My dispatch measured five surfaces and made a
point of the two that were prose — a route ledger and a conformance matrix — because prose can never
go red, only become false.

You found two more, and both would have gone RED:

So the lesson I wrote into the dispatch — git grep the predicate, hand the dev the surfaces, not
the ruling's list
— was right in direction and still short by two. The general form, which I am
taking rather than the specific: a grep for the predicate finds the code that references it; it does
not find the tests whose fixtures assume the old answer.
Those are found by asking which callers
change status, not which files name the symbol.

⭐ And the handling was right: only the caller changes in both, every mechanism and expected value
untouched, each with a comment saying so, matching how #7019 / #7033 / #10145 handled the same
situation in the same files. That is a repair, not a test edit to get green.

⭐ The depth bypass — the sharpest thing in this PR

The arm is matched to the router, not approximated to it:

parts.length === 2 would have left POST /:name/toggle/anything reaching toggleFlowungated
— a bypass

On a card that exists because a route was ungated, shipping a gate with an off-by-one that leaves a
neighbouring spelling ungated would have been the same defect wearing the fix's clothes. The router's
toggle arm tests parts[1] === 'toggle' with no length check, and the gate now matches that exactly.

The paired exclusion is equally well-reasoned in the other direction: parts[0] === 'trigger' is
excluded, because POST /automation/trigger/:name is answered above the toggle arm — so for a flow
literally named toggle, /automation/trigger/toggleruns that flow, and gating it would
over-block an execution door the ruling did not touch. Both boundaries pinned.

The consumer inventory — the axis-② risk, measured and cleared

The accepted analysis named this as the one place option A could misstep («「今天谁在调 toggle」没有被
测量»). Answer: zero callers, and the method is what makes it trustworthy:

  • packages/console ships no tracked source in this repo (4 tracked files, bundle gitignored), so
    the question is not answerable here — and instead of reporting a zero from a repo that cannot see
    the code, you read objectstack-ai/objectui at e52fac0 in a shallow read-only clone. Zero hits,
    reverse-checked: 107 files match automation there, and its three POST sites
    (useConsoleActionRuntime.tsx:546, RecordDetailView.tsx:802/trigger; FlowRunner.tsx:159
    /runs/:runId/resume) are all execution doors that stay ungated. FlowStatusDot only
    displays{enabled, bound}.
  • The one in-repo hit that could have mattered — the manual QA procedure in
    docs/qa/platform-checklist/areas/automation.json — runs as the seeded platform admin, and the
    dogfood positive control passing through that principal is what proves it still works.

So nobody is locked out on day one. That is the fact the ruling's weakest axis was waiting on.

⭐ The ablation aborted itself rather than reporting a false green

a perl -0pi -e "s/\Q...\E//" with an embedded newline inside \Q...\E matched nothing and exited
0. The on-disk grep caught it (arm-count-after=1) and the script ABORTED itself as 'Measurement
void'
instead of reporting a green ablation over an unmutated file.

This is the exact failure this lane keeps writing rules about — a zero-hit edit exits 0 — and it is the
first time this round the guard fired on itself and stopped rather than being caught downstream.
Reported rather than quietly re-run, which is the half that makes it useful. Predicted 4 red, measured
4, identities matched; positive control at both layers (an AUTHOR toggles 200 both directions,
isSystem bypasses, platform admin toggles 200 over real HTTP and the engine state actually moves).

The ruling's riders — both discharged

Changeset is minor with the 200 → 403 breaking surface spelled out as a table, plus the ADR-0087
disposition marker. And a platform fact worth keeping: check-adr-0087-registration reads the
changeset via git show HEAD:<file>, not the working tree, so a marker written to disk reads as
absent until it is committed.

Docs — checked, nothing to do

The drift check listed content/docs/api/plugin-endpoints.mdx and client-sdk.mdx. Both were pulled
in by the siblingtrigger anchors: neither mentions toggle at all (git grep -n toggle on each
returns nothing at origin/main; reverse-checked — automation appears 5× in plugin-endpoints.mdx,
so the search runs). Neither states a permission for the route this PR changes. releases/v17.mdx
names the route and is read-only, correctly untouched.

Your open question — A, and B is filed rather than dropped

Keep the shared FLOW_WRITE_DENY_MESSAGE. It is what the ruling authorizes literally (one predicate,
no new surface), it names the capability that would admit any caller — #7450's actual requirement — and
it is accurate about the policy even where it is not literally about the verb.

You are right that B is the better long-term shape and that this domain already has the precedent
one screen over (SCREEN_READ_DENY_MESSAGE beside RUN_READ_DENY_MESSAGE). A user-visible copy change
the ruling did not ask for should not ride this PR. Filed as a finding so it is graded rather than
lost with your report.


Generated by Claude Code

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

2 participants

@os-zhuang@claude