Skip to content

feat(automation): durable packaged-flow disable — activation ledger, execute() consult, operator gate - #12296

Merged
os-support-ai merged 3 commits into
mainfrom
claude/issue-12157-flow-disable
Aug 25, 2026
Merged

feat(automation): durable packaged-flow disable — activation ledger, execute() consult, operator gate#12296
os-support-ai merged 3 commits into
mainfrom
claude/issue-12157-flow-disable

Conversation

@os-support-ai

@os-support-aios-support-ai commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Closes#12157
Closes#12158

Clause-② declared: YES, on both halves. This adds refusal behaviour to a public write surface (POST /automation/:name/toggle gains a posture-conditional 403 and a 409 subflow guard — a narrowed accept set, i.e. a breaking change, shipped as one) and to the execute contract (a disabled packaged flow now refuses at execute() on every entry path). needs:contract-review rides the PM session's PR review. ⛔ Not to be cleared, flipped ready, or auto-merged from here.

Why one PR for two cards

A deliberate PM re-sequencing, recorded in both claim comments before any code was written. The two legs share one mechanism: today's toggleFlow writes the very process-local flowEnabled map that L3 retires. Split, either order ships a broken intermediate — a door writing a ledger the engine does not read, or an engine reading a ledger nothing writes while the old toggle mechanism is already gone. They land together or not at all.

What changed

The durable switch (§7.2). Flipping a packaged flow off/on writes an install-level sys_metadata_activation row (metadata_type: 'flow', organization_id NULL, active). The durable write happens inside AutomationEngine.toggleFlow — not in the HTTP route that calls it — deliberately: that method IS the service contract's off-switch, so a route-side write would leave every non-HTTP caller on the retired in-process-only behaviour.

The runtime consult (§7.2).execute() consults the ledger beside the existing FLOW_DISABLED guard — the one seam every entry path crosses (#11665 §2.3). It reuses FLOW_DISABLED; ⛔ no new ADR-0112 ledger entry, so the distinction rides the message. The install-level row also unbinds the trigger, and re-enabling rebinds it.

The retirement (#10243).flowEnabled is gone, not shaded. Two dimensions used to share that one map — the authoring status (obsolete/invalid) and the runtime toggle — and separating them made registerFlow's reconciliation arm (wasStatusDisabled || !flowEnabled.has(name), whose whole job was keeping the two from clobbering each other) disappear rather than move. That it disappears is the sign they were always distinct facts crammed into one slot. What replaces the toggle dimension is a projection of the durable row with exactly two writers, both of which go through the ledger; a grep-level pin and a writer-set pin hold that shape.

Write authority (§5). In group/isolated the toggle route requires the platform operator; single is unchanged (install-level and org-level are the same scope there, and the #10145manage_metadata gate already ran). A durable install-wide row writable by tenants would be #10243with persistence — strictly worse than what was measured, since that leak's only limit was that a cold boot undid it.

One judgement call worth review. ADR-0126 §5 says "the platform-operator capability", but no member of PLATFORM_CAPABILITIES carries that meaning — manage_metadata is the one the tier above already requires and a tenant org admin can hold it. The platform's actual operator identity is the ADR-0068 D2 built-in positionplatform_admin, documented verbatim as "Platform operator (SaaS admin). NOT a tenant user role", unscoped, sourced from the unscoped admin_full_access grant. The gate reads that position. Minting a capability of that name would have been a packages/spec change this leg is walled out of, and re-asking manage_metadata would have been vacuous.

Subflow guard (§7.3). Disabling flow B is refused while any packaged flow calls it as a subflow, with the callers named. A definition scan at disable time; ⛔ no reference index (§9). It scans bothsubflow and map nodes — map's own descriptor calls its per-item target "the per-item subflow" and reaches it through the same engine.execute, so scanning only subflow would let a map caller break exactly the way §7.3 exists to prevent. Only packaged callers guard: a tenant's own flow must not hold a packaged one hostage. Enable is never guarded. The refusal carries DELETE_RESTRICTED + 409 — the standard catalog's "cannot, due to dependencies" member, so ⛔ no new ledger entry; its DELETE_ prefix fits because this repo's own #10243 ruling records that "disabling a shipped flow is functionally equivalent to deleting it for as long as it stays off".

The behaviour change reviewers should look at first

A disable now survives unregister-and-re-register, which is what a package upgrade, a Studio publish and the boot pull all do. This inverts an existing pinned test, and the inversion is forced rather than preferred: ADR-0126 §6 wall 3 says the ledger records the customer's choice and no upgrade un-makes a choice, so a disable that did not survive that path would be un-made by every upgrade. The old test faithfully pinned the retired map's semantics (an in-process bit with no durable home); it is replaced, with the reasoning in the test body, not re-spelled.

Serial constraint — resolved, not raced

The claims flagged PR #12026 (#11997 name-shadow diagnostics) as in flight on service-automation (engine.ts, plugin.ts, flow-precedence.ts). Its diff was read first, as instructed, and it genuinely collides with this work: it adds flowShadowing immediately above the flowEnabled declaration and rewrites getFlowRuntimeStates, which reads it. It merged at 2026-08-25T09:45:35Z, before this branch was cut — verified by content, not just by the API (flow-precedence.ts is present on this branch's base). So this is built on top of it, not beside it: the subflow guard reuses its describeFlowContender rather than re-deriving "is this packaged", which is what that module's own header asks consumers to do.

Scope walls held

⛔ No packages/spec diff. ⛔ No objectui. ⛔ No ledger schema change. ⛔ The clone route is untouched — the activation gate deliberately does not cover it (a clone creates an ordinary new artifact and takes nothing from any tenant, and it is the path this PR's own refusal message recommends), and a test pins that. One dependency was added: service-automationplatform-objects, because the ledger object is declared there per §4 and this plugin registers it for the same reason it registers sys_automation_run. No cycle (platform-objects depends only on metadata-core + spec).

Verification

Follow-up (head ddac79b8). CI reported one failure outside the derived family — check:error-status-conformance, a centrally-owned repo gate: this PR gave DELETE_RESTRICTED its first producer that DECLARES a status, so the shrink-only unpinned baseline had to drop it. Ratcheted DOWN with the sanctioned node scripts/check-error-status-conformance.mjs --update (34 → 33); ⛔ no entry added — the baseline-EXPANDING remedy is maintainer-only. Verified the diff is exactly that one removal (REMOVED: ["DELETE_RESTRICTED"], ADDED: [], order otherwise preserved); the note line is re-encoded by the writer itself and is codepoint-identical after parse. The docs already publish 409 for this code (content/docs/protocol/kernel/error-handling.mdx), matching the producer, so the gate reconciles rather than reporting a mismatch. Gate now: ✓ every derivable runtime status is documented, and every documented status is reachable. Ratchet family re-run on the NEW head: check:engine-double-contractOK — 409 pinned, check:where-matcher300/300, check:query-options-erasureratchet holds: 67 … none new, check:nul-bytes exit 0. check:type-check-debt is not re-run for this commit and does not need to be: the only change since f5e4cee3 is one scripts/*.json baseline entry, so the TypeScript tree it measures is byte-identical to the one it passed on.

All of the below ran against f5e4cee3, the head of this branch, with a clean tree (git status empty — confirmed after the runs, so no build output moved the tree under them).

  • pnpm --filter @objectstack/service-automation testTest Files 90 passed (90) / Tests 1080 passed (1080)
  • pnpm --filter @objectstack/runtime testTest Files 190 passed (190) / Tests 2805 passed (2805)
  • pnpm --filter @objectstack/runtime typechecktsc --noEmit, exit 0
  • pnpm --filter @objectstack/service-automation build (no typecheck script; the build is the type check) — DTS ⚡️ Build success
  • pnpm lint — the whole repo, not a narrowed run: eslint . --no-inline-config --format json reports 5291 files linted, 0 errors, 0 warnings, and all seven changed files are in that population (checked against the JSON, not assumed).
  • pnpm check:engine-double-contractOK — 409 pinned, 133 in the DEBT ledger, 2 exempt.
  • pnpm check:where-matcher✓ 300 matcher(s) discovered, 300 answer the combinator battery correctly or refuse it loudly (187 refuse)
  • pnpm check:test-source-aliasOK — 72 packages with tests scanned
  • pnpm check:type-check-debtOK — 32 ledger entr(ies) re-measured in 353.0s, 1843 raw tsc error(s) total, none above its recorded number. / surplus: none
  • pnpm check:type-check-coverageOK — 65/78 workspace packages type-checked
  • pnpm check:nul-bytes, check:cross-package-test-inputs, check:type-source-resolution, check:published-files, check:route-envelope, check:slot-lookup, check:query-options-erasure, check:dispatcher-error-vocabulary, check:error-code-casing, check:override-consistency, check:objectui-changeset, check:changeset-gate-self-tests, check-plugin-teardown-shape, check-empty-changeset, check-changeset-no-major, check-changeset-fixed, check-adr-0087-registration, check-osv-exemptions, check-ci-filter-parity — all exit 0.

Gates derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, which reported the list as derived from this repo at ce48da73, from the real change set rather than a hand-written diff. Three came back red on the first pass — engine-double-contract, where-matcher, test-source-alias, exactly the new-test-file convention family the deriver names — and were fixed, not baselined: the fake engine's update() now routes through assertEngineUpdateDispatch, its WHERE predicate refuses combinators it does not implement, and platform-objects is aliased to source in a new vitest.config.ts. The only ledger write was --write recording the newly pinned double, which adds coverage rather than weakening a ratchet.

Ablation

Three legs, each with the failing set predicted in writing before the run, and each observed exactly.

ablationpredictedobserved
isFlowEnabled stops consulting the ledger10 of 29 fail10 failed | 19 passed — the same ten, test for test
the §7.3 subflow guard never fires3 of 29 fail (the refusal assertions; the four negative-direction cases stay green)3 failed | 26 passed — the same three
the §5 posture gate is not called6 of 17 fail (3 refusal assertions × 2 walled postures)6 failed | 11 passed — the same six

No rebuild was needed and that is load-bearing rather than a shortcut: both suites reach the mutated files through relative imports, which vitest resolves from source — proven here rather than assumed, since the mutation changed the result with no build step between. Each mutation was proved on disk before its run (occurrence counts of the removed and injected text, plus git diff --stat), never inferred from an editor's exit code — and that guard earned itself immediately: the first attempt used a malformed perl -i invocation that exited 0 while changing nothing, and the check aborted the leg instead of reporting a vacuous green. The harness carried trap … EXIT INT TERM. Restore was verified byte-for-byte: no markers left in the tree, git status empty, and git hash-object equal to git rev-parse HEAD: for both mutated files.

Open question for the reviewer

toggleFlow on a non-packaged flow has no ledger row to write — there is no packaged base, and package_id is a required column meaning "the package that ships the base artifact". ADR-0126 models packaged artifacts only and does not charter this case either way. This PR leaves that path behaviourally identical to today (the flip applies in process and warns that it is not durable) rather than inventing a sentinel package_id, refusing the call, or silently widening the ledger's model — each of which is a contract decision above this leg's pay grade. The #10243 leak shape is closed for it regardless, because the §5 authority gate is at the route and does not care whether the flow is packaged. Flagged rather than decided.


Generated by Claude Code


Generated by Claude Code

…dger
Flipping a packaged flow off/on now writes the install-level
`sys_metadata_activation` row (ADR-0126 §4/§7.2) and the engine consults that
ledger at the `execute()` seam, reusing the existing `FLOW_DISABLED` code with
the distinction carried by the message. The install-level row also unbinds the
trigger, and re-enabling rebinds it.
This retires the process-local `flowEnabled` map (#10243) rather than shading
it: the map was not a row, so no organization wall scoped it. Because a durable
install-wide switch writable by tenants would be that leak with persistence,
the toggle route is now operator-gated in the `group`/`isolated` postures
(ADR-0126 §5); `single` is unchanged.
Disabling a flow packaged flows still call as a subflow is refused with the
callers named (ADR-0126 §7.3), by definition scan over `subflow` and `map`
nodes — no reference index.
Closes#12157Closes#12158
…bjects
- fakeEngine.update() routes through assertEngineUpdateDispatch, so the double
cannot be looser than the engine it stands in for (check:engine-double-contract);
the new pinned coverage is recorded in the pinned ledger.
- Its WHERE predicate REFUSES combinators it does not implement instead of
reading them as field names (check:where-matcher).
- vitest aliases @objectstack/platform-objects to source, so the ledger tests
are a verdict about this checkout rather than about dist build state
(check:test-source-alias).
@os-support-aiClaude

Copy link
Copy Markdown
CollaboratorAuthor

Clause-② contract review — PASS (PM session session_01KWRU3s15AJz7PGW7a7wdCh, CONTRACT_REVIEW_TIER, per the claims on #12157/#12158).

Surfaces read in full: the §5 activation gate (automation.tsisFlowActivationWrite + refuseUngrantedActivationWrite), the engine half (engine.tsflowLedgerDisabled projection, isFlowEnabled composition, toggleFlow durable-write-first + §7.3 guard, execute()/test-single consult, registerFlow/unregisterFlow reconciliation removal, hydrateFlowActivations), the store (flow-activation-store.ts), the plugin wiring (registration, probe-gated attach, post-pull hydration), and the inverted unregisterFlow pin. Test bodies and bookkeeping accepted on the PR's gate evidence and predicted-ablation record.

Three judgment calls reviewed and accepted, on the record:

  1. Operator = the ADR-0068 D2 platform_admin position, not a minted capability. Correct within this leg's walls: no PLATFORM_CAPABILITIES member carries the operator meaning, manage_metadata is the already-asked weaker question, and minting a name is a packages/spec change the card excludes. If the operator concept ever needs a wire-visible capability name, that is its own spec card — not this PR's debt.
  2. A ledger disable survives unregister + re-register (pinned test inverted, reasoning in the test body). Forced by ADR-0126 §6 wall 3 — upgrade/publish/boot-pull must not un-make the customer's choice. The old pin faithfully described the retired mechanism; the new pin describes the chartered one.
  3. toggleFlow on a non-packaged flow stays in-process with a loud non-durability warning. Correct scope discipline: ADR-0126 models packaged artifacts; a runtime-authored flow's durable off-switch is its own status, which its author can edit. The Decide whether POST /api/v1/automation/:name/toggle belongs in the manage_metadata write set — it mutates flow enablement with no authoring capability #10243 leak shape is closed for it regardless (the §5 gate keys on the route, not on packagedness). Recorded as deliberately-not-chartered; any future widening of the ledger to non-packaged artifacts is a model question for the maintainer.

Also verified: DELETE_RESTRICTED/409 and FLOW_DISABLED reuse mint no new ADR-0112 entries; the clone route is deliberately outside the activation gate with a pinning test; the §7.3 scan covers map as well as subflow nodes and reads only the canonical key; per-org rows are skipped, not merged, on the read side. Serial constraint vs #11997 resolved by building on its merged describeFlowContender rather than beside it.

Remaining before merge: CI green on this head. Per the maintainer's standing autonomy ruling this PR will be marked ready and enqueued once CI completes clean.


Generated by Claude Code

@github-actions

github-actionsBot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/service-automation, touching 42 documentable anchor(s). ⚠️3 changed file(s) yielded no anchor (packages/services/service-automation/package.json, packages/services/service-automation/src/index.ts, packages/services/service-automation/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json af58a6fbc4e51f3a1473c0c8a36cd6cbae3f5d0c.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 3 changed file(s) yielded no anchor (packages/services/service-automation/package.json, packages/services/service-automation/src/index.ts, packages/services/service-automation/vitest.config.ts) — pages documenting those are invisible to this run
  • 8 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. Of those 177: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 107 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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.

Coarse fallback — 26 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 af58a6fbc4e51f3a1473c0c8a36cd6cbae3f5d0cpackageMentionDocs.

Which tree this was computed on

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

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

…ETE_RESTRICTED
The ADR-0126 §7.3 subflow-guard refusal declares `{ code: 'DELETE_RESTRICTED',
status: 409 }`, which gives the code its first producer this gate can derive a
status from. `check:error-status-conformance` therefore required the shrink-only
unpinned baseline to drop it: 34 -> 33 entries.
Regenerated with the sanctioned `node scripts/check-error-status-conformance.mjs
--update` — the shrinking direction. No entry was added (an addition is an
expansion, and that remedy is maintainer-only). The docs already publish 409 for
this code (content/docs/protocol/kernel/error-handling.mdx), matching the
producer, so the gate reconciles rather than reporting a mismatch.
The `note` line is re-encoded by the writer (`—` -> a literal em dash); the
parsed string is codepoint-identical, and it is the generator's own output, not
a hand edit.
@os-support-ai
os-support-ai marked this pull request as ready for review August 25, 2026 17:22
@os-support-ai
os-support-ai added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit 1524927Aug 25, 2026
38 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-12157-flow-disable branch August 25, 2026 17:45
os-support-ai pushed a commit that referenced this pull request Aug 26, 2026
…on-ledger row contract (#12350)
ADR-0126 section 4 declares ONE activation ledger for the whole disable+clone
family. It had two independent implementations of that one row contract:
ObjectStoreFlowActivationStore @objectstack/service-automation #12296
ObjectStoreActionActivationStore @objectstack/objectql #12348
They agreed on every load-bearing detail because the second was written from
the first, and nothing structurally held them together. Section 8 pre-charts
`tool`, `skill` and `position` as later consumers, and a third and fourth copy
is where the row semantics start drifting — the org-row skip and the
`0`-is-false read are exactly the kind of detail a copy loses quietly, in the
direction (an artifact silently re-arming) nothing else measures.
Neither consumer could import the other: `service-automation` does not depend
on `@objectstack/objectql` (devDependency only), and the engine must not depend
on a service. `@objectstack/core` is the package BOTH already depend on, so it
is the one home that needs no new edge — the same reasoning that put
`recordNotFoundError` there for the ADR-0076 D2 closure. NOT
`@objectstack/platform-objects`, which declares the OBJECT: `objectql` does not
depend on it and that edge would invert the tiering. Where the code lives is a
module-import question; where the object's registration lives is a composition
question with its own answer.
`ObjectStoreMetadataActivationStore(engine, metadataType)` carries the row
semantics once. Each consumer keeps its own name, its own one-argument
constructor and its own docs, and fixes the discriminator — a binding, not an
implementation. A caller that had to pass the discriminator could pass the
wrong one; each leg has exactly one correct value.
Byte-equivalent semantics: install-level rows only (`organization_id` never
written), org-carrying rows skipped on read AND ignored when deciding
insert-vs-update, a driver `0` read as false, read-then-write rather than a
blind upsert, no `delete` in the engine slice because re-enabling rewrites the
row.
Both existing pin suites stay green UNCHANGED — they pin the contract from each
binding's side, so their staying green is the proof the consolidation lost
nothing. The new suite beside the implementation pins what neither of them can:
that the discriminator is a PARAMETER rather than a constant, that two bindings
over one table see none of each other's rows in either direction, and that a
type nobody has written yet behaves identically.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KWRU3s15AJz7PGW7a7wdCh
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependenciesPull requests that update a dependency filedocumentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

2 participants

@os-support-ai@claude