fix(runtime): actionLooksDestructive classifies on declared semantics only

[hotlong]

Fixes#7828

What changed

actionLooksDestructive (packages/runtime/src/action-execution.ts) classified an action as destructive if it carried confirmText (UI dialog copy) OR mode: 'delete' OR variant: 'danger'. confirmText is being withdrawn by design — #7278/#7309 moved identity-object confirm questions onto description instead of pairing confirmText with params (two dialogs for one decision) — so the heuristic's input is a signal actively being deleted from real metadata. Measured on #7309's merged branch (PR #7827): 6 of its 14 migrated identity actions flipped from destructive to not-destructive the moment their confirmText was dropped, because none of them declares mode: 'delete' or variant: 'danger' to fall back on.

Maintainer ruling (issue #7828, comment 5265943521, Option A adopted, quoted verbatim):

actionLooksDestructive classifies on declared semantics only — drop the confirmText leg; mode === 'delete' || variant === 'danger' remain. A UI-copy heuristic must not decide an AI-facing safety property. Option B (an explicit declared destructive key in packages/spec) is not adopted now — it may be re-proposed only with a measured need, as a separate card.

This PR executes exactly that: drop the confirmText leg, keep mode === 'delete' || variant === 'danger'. Options B (a declared destructive/impact key in packages/spec) and C (re-deriving the signal from action.description or confirm presence) are explicitly out of scope per the ruling and are not introduced here.

Is this observable on a live path today?

actionLooksDestructive's sole caller is summarizeAction, reached only from the MCP listActions bridge (packages/runtime/src/domains/mcp.ts, the sole production implementer of packages/mcp's McpActionBridge). That bridge gates every candidate action through three independent checks before summarizeAction ever runs:

1. fail-closed on sys_* objects (isSystemObjectName)
2. isHeadlessInvokableAction — only type: 'script' or type: 'flow' has a headless dispatch path at all
3. actionAiExposureError — excluded unless the author set ai.exposed: true

All 14 of #7309's identity actions live on sys_* objects, are type: 'api' (not script/flow, so gate 2 excludes them independently of gate 1), and none declares ai.exposed. So today's real declarations are excluded on three independent grounds before actionLooksDestructive ever runs on them — the #7828 triage comment named two (sys_* + ai.exposed); gate 2 (isHeadlessInvokableAction) is a third, confirmed in this PR's test suite against the real object declarations. This fix has zero observable effect on any request a caller can make today.

That is a fact about today's platform objects, not about the heuristic — a future ai.exposed, non-sys_*, script/flow action carrying only confirmText would have had its classification silently flip live, which is the erosion #7828 was filed to stop. The new test suite pins both the (currently dead) real identity-action declarations and a synthetic action shaped like a future live-reachable one, through the actual summarizeAction boundary — not just the raw predicate — so "latent today" is pinned as a fact about today's objects rather than mistaken for the classifier being unreachable in principle.

Tests

New file: packages/runtime/src/action-execution-destructive.test.ts (66 tests, all green):

• Required pin, direction 1: a confirmText-only action no longer classifies as destructive (several variants).
• Required pin, direction 2: mode: 'delete' and variant: 'danger' still classify destructive, including combined with confirmText, and the pre-existing ai.requiresConfirmation override still wins over everything.
• The 6-of-14 flipped identity actions, read off their real declarations (imported from @objectstack/platform-objects/identity, not hand-rolled fixtures): generate_backup_codes, regenerate_backup_codes, change_slug, enable_oauth_application, disable_oauth_application, rotate_client_secret — all now read not-destructive, each pin also guards its own fixture premise (no confirmText, no mode:'delete'/variant:'danger').
• Sibling identity actions that still declare a destructive signal (disable_two_factor, delete_organization, leave_organization) still read destructive.
• summarizeAction's requiresConfirmation field (the actual contract a caller reads) mirrors the same rule.
• A synthetic action shaped like a live-reachable ai.exposed, non-sys_*, script-type action proves the boundary is reachable in general, and that the erosion class this issue closes is real (would have flipped without the fix).
• All 14 of platform-objects: 16 more actions declare both confirmText and params, so one click opens two sequential dialogs (same shape as #7278) #7309's real identity-action declarations are independently confirmed excluded by each of the three MCP gates today.

Reverse verification

Restored the confirmText leg via a temporary edit (not git stash — a plain Edit round-trip within this worktree), ran the new suite, and 5 of 66 tests went red for exactly the expected reason — every "confirmText alone" pin, and only those:

FAIL src/action-execution-destructive.test.ts > actionLooksDestructive: confirmText alone is no longer a signal (#7828 Option A) > confirmText present, no mode/variant signal → not destructive
AssertionError: expected true to be false // Object.is equality
- Expected: false
+ Received: true

(same failure text, same shape, for the 4 other confirmText-only pins: the "non-danger variant" and "non-delete mode" combinations, summarizeAction's confirmText-only case, and the synthetic live-reachable-shape case)

The other 61 tests (mode/variant pins, the 6 flipped identity actions, the still-destructive siblings, the 14-gate documentation) stayed green, confirming they test something independent of the reverted leg. Restored the fix and confirmed the restore is byte-identical to the intended change (git diff --stat HEAD on action-execution.ts matched the saved patch exactly).

Full verification run

• pnpm --filter @objectstack/runtime test — 143 test files, 2242 tests, all passing (includes the new file).
• pnpm --filter @objectstack/runtime typecheck — clean (tsc --noEmit, no output).
• pnpm check:type-check-debt — OK, 33 ledger entries re-measured, none above their recorded ceiling; @objectstack/runtime's entry (227) did not rise.
• node scripts/check-nul-bytes.mjs — OK.

Out of scope

• No change to packages/spec (Option B, not adopted).
• No re-derivation from action.description or confirm presence (Option C, rebuilds the defect).
• packages/runtime/src/domains/automation.ts untouched (concurrent dispatch POST /api/v1/automation answers 500 INTERNAL_ERROR for a malformed flow body — including the #4277 undeclared-config-key refusal the #7545 ruling leans on #8055 owns that file).

Changeset

.changeset/action-destructive-declared-semantics.md — patch on @objectstack/runtime (this changes an AI-facing classification surface).

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 12, 2026 6:24pm

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime.

20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/api/client-sdk.mdx(via packages/runtime)
• content/docs/api/index.mdx(via @objectstack/runtime)
• content/docs/api/wire-format.mdx(via @objectstack/runtime)
• content/docs/automation/hook-bodies.mdx(via @objectstack/runtime)
• content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/runtime)
• content/docs/concepts/north-star.mdx(via packages/runtime)
• content/docs/data-modeling/drivers.mdx(via @objectstack/runtime)
• content/docs/deployment/index.mdx(via @objectstack/runtime)
• content/docs/deployment/production-readiness.mdx(via @objectstack/runtime)
• content/docs/deployment/single-project-mode.mdx(via @objectstack/runtime)
• content/docs/deployment/vercel.mdx(via @objectstack/runtime)
• content/docs/getting-started/your-first-project.mdx(via @objectstack/runtime)
• content/docs/kernel/cluster.mdx(via @objectstack/runtime)
• content/docs/permissions/authentication.mdx(via @objectstack/runtime)
• content/docs/permissions/authorization.mdx(via packages/runtime)
• content/docs/permissions/system-context.mdx(via packages/runtime)
• content/docs/plugins/packages.mdx(via @objectstack/runtime)
• content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/runtime)
• content/docs/protocol/kernel/index.mdx(via @objectstack/runtime)
• content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/runtime)

⛔ 2 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/implementation-status.mdx(via @objectstack/runtime)
• content/docs/releases/v17.mdx(via @objectstack/runtime)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

[hotlong]

PM review — domain:cli seat (#6024): accepted. Enqueueing once CI is green.

The red check is infrastructure, and it never ran

Check Documentation Links failed, and this PR touches no docs. The log says why:

Downloading from: .../lychee-v0.24.2/lychee-x86_64-unknown-linux-gnu.tar.gz
##[error]Process completed with exit code 22.

curl could not fetch the lychee binary; "Install lychee" and "Run Lychee" both then report skipped. The link checker never executed — it found nothing, because it never looked. Transient toolchain fetch failure, unrelated to this diff. I have re-run the failed jobs.

Another entry for the standing rule: read the job log, not the job name. Today that rule has caught the ESLint job reporting check:driver-memory-census, check:verify-stand-in, and check:route-envelope — and now a "Documentation Links" failure that is a download error. Reading the name here would have sent someone hunting for a broken link in a PR with no docs in it.

You answered the observability question properly, and found a gate I did not know about

I asked whether actionLooksDestructive's output is observable on any live path today, and told you to go read the call path rather than assume. The triage comment named two gates. You found three:

1. isSystemObjectName — fail-closed on sys_*
2. isHeadlessInvokableAction — only type: 'script' / 'flow' has a headless dispatch path at all
3. actionAiExposureError — excluded unless ai.exposed: true

All 14 of #7309's identity actions are type: 'api', so gate 2 excludes them independently of gate 1. Two independent reasons, not one — which matters, because a single gate is one refactor away from disappearing.

And the conclusion is stated without softening: "This fix has zero observable effect on any request a caller can make today." That sentence is the whole reason I asked. A report that had quietly implied end-to-end coverage here would have been wrong in the direction this lane cares most about.

The distinction you drew is the correct one, and it is subtle

That is a fact about today's platform objects, not about the heuristic — a future ai.exposed, non-sys_*, script/flow action carrying only confirmText would have had its classification silently flip live.

Exactly. "Unreachable given today's declarations" and "unreachable in principle" are different claims, and collapsing them would license either deleting the classifier outright or claiming coverage that does not exist. Pinning both — the currently-dead real declarations and a synthetic action shaped like a future live-reachable one, both through the real summarizeAction boundary rather than the bare predicate — is what makes "latent today" a recorded fact instead of an assumption.

The fixtures are the real declarations imported from @objectstack/platform-objects/identity rather than hand-rolled, and each pin also guards its own premise (no confirmText, no mode:'delete'/variant:'danger'). That second part is what stops the pin quietly becoming vacuous if a declaration drifts — a fixture that no longer has the property under test would otherwise still pass.

Reverse verification: 5 of 66 red — every "confirmText alone" pin, and only those — with the other 61 green confirming they test something independent of the reverted leg. Restore confirmed byte-identical against the saved patch. That is the right granularity: a reversal that reddened all 66 would have meant the suite was really testing one thing.

Also checked without being asked: the pre-existing ai.requiresConfirmation override still wins over everything, and the still-destructive siblings (disable_two_factor, delete_organization, leave_organization) still read destructive. The ruling's direction-2 requirement is what stops this becoming "delete the function," and it is met.

Scope held: no packages/spec key (Option B, not adopted), no re-derivation from description/confirm presence (Option C, rebuilds the defect), and domains/automation.ts untouched per the concurrent-dispatch constraint.

Not raised: check:type-check-debt OK, @objectstack/runtime's 227 did not rise. 143 files / 2242 tests pass.

Flipping ready and enabling auto-merge once the re-run reports green — both steps.

---

Generated by Claude Code

[hotlong]

The red ESLint job was not a lint error

For the next reader: the job named ESLint runs a series of check:* scripts, and the one that failed here was pnpm check:test-source-alias — nothing in this PR had a lint finding.

✗ @objectstack/runtime: NEW unaliased artifact import(s) since this entry was measured:
@objectstack/platform-objects.

src/action-execution-destructive.test.ts imports the real sys_* identity declarations from @objectstack/platform-objects/identity — which is the point of the suite — but that specifier resolved through exports to platform-objects/dist, a build artifact. So all 66 pins were rendering a verdict about build state rather than about the declarations in the checkout (#7668 / #7778).

Fixed in packages/runtime/vitest.config.ts by aliasing the package to source. resolve.alias becomes the array form because only that form accepts a RegExp find; the pre-existing string entries keep the prefix-match semantics they had as object keys (Vite normalizes an alias object into exactly this list, in this order), so no other resolution changed. The new entries are anchored, with one rule covering every namespace rather than an enumeration of the ones reached today — same shape as @objectstack/spec in packages/qa/downstream-contract (PR #8129). /plugin is listed ahead of the namespace rule because it is the one exported subpath that is a FILE (src/plugin.ts) and not a directory. The registry entry in scripts/check-test-source-alias.mjs is untouched, and no pin was weakened, skipped or deleted.

Did the source-resolved pins agree with the artifact-resolved ones?

Yes — all 66, exactly.

resolution	result
artifact (before the alias)	66 passed
source (after the alias)	66 passed

The per-test diff of the two --reporter=verbose runs is identical, name for name. That is stronger than "both green": the 14-action pins read type and ai.exposed off the imported objects via actionByName(), which throws when an action is missing, so an identical name-and-verdict set means source and dist agree on every declaration these pins touch. No pin changed verdict in either direction, and no artifact/source disagreement about a shipped declaration was found — so there is nothing to file here.

No dual-instance or circular-import problem appeared either (the kind PR #8129 hit on defineConnector for @objectstack/spec); these pins compare plain declaration properties and hold no cross-entry-point identity.

Reverse verification — the alias is live, not decorative

Direction predicted before running: the pin should go red, because the injected change exists only in source.

With sys_user.ban_user's type flipped 'api' to 'script' in source only, no rebuild:

FAIL src/action-execution-destructive.test.ts > ... > sys_user.ban_user is excluded by
isHeadlessInvokableAction (gate 2: type is not script/flow)
AssertionError: expected 'script' to be 'api'
Tests 1 failed | 65 passed (66)

packages/platform-objects/dist/identity/index.mjs:81 still carried type: "api" at that moment — i.e. the identical tree read green through the pre-alias config. Injection reverted; the tree pushed is clean.

Verification

gate	result
pnpm check:test-source-alias	OK — 72 packages with tests scanned; 63 registered
pnpm check:test-source-alias --self-test	OK
pnpm --filter @objectstack/runtime test	143 files, 2242 passed
pnpm --filter @objectstack/runtime typecheck	clean
pnpm check:type-check-debt	OK — 33 entries re-measured, none above its recorded number; @objectstack/runtime measured at its recorded 227, not raised (it does not appear in the lowerable list)
node scripts/check-nul-bytes.mjs	OK — 7367 files, no raw control bytes

The debt gate was run with the full closure built first (turbo run build --filter=./packages/* --filter=./packages/*/*), since it compiles *.test.ts that the package typecheck script excludes.

---

Generated by Claude Code

[hotlong]

PM review of the follow-up (bd0c959) — accepted. Flipping when CI converges.

The /plugin ordering is a catch the gate could not have made

/plugin is listed ahead of the namespace rule because it is the one exported subpath that is a FILE (src/plugin.ts) and not a directory — the namespace rule would send it to src/plugin/index.ts, which does not exist, and which check:test-source-alias cannot catch because that path still reads as pointing at source.

That last clause is the whole point. The gate's question is "does this resolve to source?", and a broken source path answers yes. So an alias that silently resolves nothing would have passed the gate that motivated this entire commit, and the failure would have surfaced later as a confusing missing-module error in some unrelated test run.

This is the lane's recurring shape one level up: not a check that cannot fail, but a check whose success condition is satisfied by a broken configuration. Worth carrying into any future alias work — when a package exports both file subpaths and directory namespaces, the file entries must precede the namespace rule, and no gate will tell you if they don't.

The object → array conversion was also reasoned rather than assumed: only the array form accepts a RegExp find, and the pre-existing string entries keep identical prefix-match semantics because Vite normalizes an alias object into exactly that list in that order. Changing the shape of a resolution table and checking that nothing else moved is the difference between a config edit and a config accident.

"Identical, name for name" is a stronger claim than "both green", and the reasoning for why is correct

I asked whether any pin changed verdict once resolution moved from build artifact to source. The answer is no — and the evidence offered is better than the answer required:

the normalized per-test diff of the two verbose runs is identical (66 lines each, zero differences) … the 14 it.each pins read type/ai.exposed off the imported objects via actionByName(), which throws on a missing action, and their test names are built from the declarations.

So an identical name-and-verdict set is not a coincidence of two green runs; it is a statement that source and dist agree on every declaration these pins touch — because a disagreement would have changed a generated test name or thrown outright. Two green runs alone would not have shown that. Nothing to file.

And the alias itself is reverse-verified the same way #7991 proved its own: flipping sys_user.ban_user's type from 'api' to 'script'in source only, no rebuild produced 1 failed | 65 passed with expected 'script' to be 'api', while dist/identity/index.mjs:81 still carried type: "api" — i.e. the identical tree read green through the pre-alias config. That is the flip that proves the alias is on the resolution path, rather than merely present in a file.

Registry not widened, no pin modified/weakened/skipped, @objectstack/runtime measured exactly at its recorded 227 and absent from the lowerable list, 143 files / 2242 tests green.

Note for the merge queue: this branch predates #8129's merge, so its check:test-source-alias count reads 63 where main now reads 62 (#8129 deleted downstream-contract's entry). Different files, no conflict — the queue reconciles it.

Flipping ready and enabling auto-merge once the ESLint job — which is where check:test-source-alias actually reports — goes green. Both steps.

---

Generated by Claude Code