Skip to content

Commit 8a21532

Browse files
Lykhoydaclaude
andauthored
feat: command-surface gate — /health.commands enumeration, tiered artifact rebuild, typed UNSUPPORTED_COMMAND (#418) (#420)
* docs(specs): #418 command-surface gate design — typed UNSUPPORTED_COMMAND + /health.commands enumeration (B235) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(plans): #418 command-surface gate implementation plan + spec addendum (B235 true root cause) Amendments applied from the multi-LLM plan review (Codex + Claude Opus coordinator; Gemini CLI unavailable — IneligibleTierError): - BLOCKER: Android remediation rewritten — resolveAndroidInstallAction never Gradle-builds while APKs exist, so Task 8 now deletes the APKs (open-only allowArtifactRebuild flag, retry-once wrapper) and surfaces RUNNER_COMMANDS_STALE instead of RUNNER_PROTOCOL_MISMATCH. - BLOCKER: iOS DerivedData invalidation now reaps first and runs behind a checkout-scoped mkdir build lock; per-plugin-version rebuild budget stops cold-build loops on a broken checkout. - SHOULD-FIX: invalidate-before-first-spawn at open; raw-value-aware Swift sync regex; two extra Android fixture migrations; stale dismissKeyboard reference scrub. - Rejected on cross-check: exhaustive satisfies completeness (REQUIRED is a curated subset by design). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(device): iOS keyboard-dismiss wire verb is keyboardDismiss, not dismissKeyboard (B235 root cause, #418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(protocol): REQUIRED command lists + missing-commands classification + typed codes (#418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(rn-fast-runner): enumerate commands in /health + typed UNSUPPORTED_COMMAND (#418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(rn-android-runner): enumerate supported commands in /health + foreground alias verbs (#418) The foreground pre-flight now covers press/fill/scroll — the aliases reach the same handlers as tap/type/drag but skipped app activation (codex-pair). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(rn-fast-runner): validate client-supplied Content-Length — 400 on invalid, no unchecked range arithmetic (#418) Negative values made an invalid subdata range; huge values trapped on integer overflow — both crashed the XCTest runner (codex-pair HIGH). A complete header section with a missing/negative/oversized Content-Length can never become valid, so it now answers 400 immediately instead of buffering until the client times out (codex-pair MED follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(sync): tri-file command-surface drift guard (#418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ios-gate): classify runners missing required commands as stale (#418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(ios-gate): tiered missing-commands remediation — rebuild at open, refuse mid-flow (#418) Open-path artifact invalidation is reap-first, serialized behind a checkout-scoped mkdir build lock (DerivedData is plugin-scoped while the device lock is UDID-scoped), and budgeted to one cold rebuild per plugin version. Mid-flow device tools refuse fast with RUNNER_COMMANDS_STALE. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(android-gate): command-surface gate + real APK-invalidation rebuild tier (#418) resolveAndroidInstallAction only Gradle-builds when the APKs are ABSENT (review BLOCKER — forceReinstall alone re-installs the same stale APK), so missing-commands remediation deletes the APKs behind an open-only allowArtifactRebuild flag with a retry-once wrapper that reaps the device-side UiAutomation slot first (#237). Mid-flow surfaces RUNNER_COMMANDS_STALE, mirroring the iOS contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(status): surface missing runner commands in deviceSession.runnerProtocol (#418) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: #418 changeset + rebuilt dist (troubleshooting entry lives in local CLAUDE.md, per repo convention) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ios-gate): rebuild tier fires on first open when a dead runner spawns from a stale prebuilt (#418) First probe 'dead' meant the up-front short-circuit never ran and the after-probe missing-commands fell into the mid-flow refusal even at open — the first open after an upgrade errored and only the second healed (pre-device-verify trace). The after-probe branch now enters the same rebuild tier when allowArtifactRebuild is set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ios-gate): open cold-builds when nothing is prebuilt — the #210 refusal is mid-flow-only (#418) decideRunnerSpawn's not-prebuilt error text directs users to open, but open itself hit the same refusal (pre-existing since #383 routed open through ensureRunnerForCommand; device-verify finding). #418 makes it bite harder: the invalidation deletes DerivedData and a cold xcodebuild test leaves no .xctestrun, so a later runner death bricked every open until a manual build-for-testing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ios-gate): typed RUNNER_COMMANDS_STALE for stale+not-prebuilt mid-flow; budget message names the reset path (#418) Multi-review advisories: the not-prebuilt decision shadowed the typed refusal (callers surfaced RN_FAST_RUNNER_DOWN), and a transiently-failed cold rebuild spent the once-per-version budget with a misleading 'checkout may be broken' message — it now names the commands-rebuild.json reset. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(android-gate): retry-once wrapper is the single rebuild owner (#418) Multi-review advisory: the reuse-gate's inline invalidate+rebuild plus the wrapper retry meant a broken checkout paid Gradle twice on the error path. The reuse gate now always throws the typed error; the wrapper owns all rebuilds — one build on every path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: oxfmt gh-418 classify test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 3fdc856 commit 8a21532

35 files changed

Lines changed: 3365 additions & 59 deletions
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"rn-dev-agent-cdp": minor
3+
"rn-dev-agent-plugin": minor
4+
---
5+
6+
Command-surface gate (#418, B235): both native runners enumerate their supported
7+
commands in `/health.commands` (iOS derives it from `CommandType.allCases`, Android
8+
from a sync-tested `SUPPORTED_COMMANDS` list) and the liveness gate classifies a
9+
runner missing any bridge-required verb as stale (`missing-commands`). Remediation
10+
is tiered: `device_snapshot action=open` auto-invalidates the stale artifact and
11+
rebuilds — iOS deletes DerivedData and cold-builds (once per plugin version, behind a
12+
checkout-scoped build lock), Android deletes the runner APKs so self-install
13+
Gradle-rebuilds; mid-flow device tools refuse fast with `RUNNER_COMMANDS_STALE`
14+
instead of silently building. An unknown verb reaching the iOS runner now returns a
15+
typed `UNSUPPORTED_COMMAND` error instead of a raw Swift decode failure. Root cause
16+
of B235 fixed: the explicit iOS keyboard-dismiss path posted `dismissKeyboard`,
17+
which no Swift artifact ever accepted — the wire verb is now `keyboardDismiss`.
18+
`cdp_status` surfaces `deviceSession.runnerProtocol.missingCommands`. Hardening
19+
from per-edit review: the iOS runner validates client-supplied Content-Length
20+
(400 on invalid instead of crash/hang) and Android foregrounds alias verbs
21+
(press/fill/scroll) before dispatch.

docs/superpowers/plans/2026-07-03-418-command-surface-gate.md

Lines changed: 1655 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
# #418 — Typed UNSUPPORTED_COMMAND + command-surface gate (B235)
2+
3+
**Date:** 2026-07-03
4+
**Issue:** [#418](https://github.com/Lykhoyda/rn-dev-agent/issues/418) · Bug: workspace B235
5+
**Depends on:** #383 (protocol gate, shipped), #384/PR #406 (dynamic /health capabilities, shipped)
6+
7+
## Problem
8+
9+
A prebuilt runner artifact that predates a command-enum addition passes the #383
10+
health gate — protocol versions bump on wire-shape changes, not enum additions —
11+
and then rejects the newer verb at dispatch time. On iOS the failure is a raw
12+
Swift decode error deep inside a batch:
13+
14+
```
15+
dataCorrupted … Cannot initialize CommandType from invalid String value dismissKeyboard
16+
```
17+
18+
Root cause of the gate blind spot: **neither existing health signal fingerprints
19+
the build artifact's command surface.**
20+
21+
- `protocolVersion` tracks the wire shape (asserted by humans, bumped rarely).
22+
- `runnerVersion` is injected at **spawn time** (`TEST_RUNNER_RN_PLUGIN_VERSION`
23+
env from the spawning bridge), so version-skew catches a runner spawned by an
24+
old bridge — but a reap+respawn "fixes" the skew while the same stale binary
25+
keeps running. It never sees the artifact.
26+
27+
Three concrete ways a stale artifact is admitted today:
28+
29+
1. Artifact built earlier in the **same release cycle**, before a new verb landed
30+
— same version string, missing command (the B235 scenario).
31+
2. The documented manual pre-build (`xcodebuild build-for-testing`) doesn't
32+
inject `RN_PLUGIN_VERSION``runnerVersion` absent → skew check skipped
33+
(fail-open by design).
34+
3. Unreadable plugin manifest → `pluginVersion` null → skew check disabled.
35+
36+
Android already returns a typed `UNSUPPORTED_COMMAND` error
37+
(`CommandDispatcher.kt` `else` branch); its gate has the same blind spot
38+
(`/health.capabilities` is always `[]`). iOS has neither the typed error nor the
39+
enumeration.
40+
41+
## Design principle
42+
43+
Derive state from the artifact, don't assert it. `CommandType.allCases` is a
44+
fact extracted from the compiled binary; enumerating it in `/health` gives the
45+
gate a signal that cannot drift from the truth. (Same principle as #384's
46+
dynamic capabilities.)
47+
48+
## Decisions (brainstorm 2026-07-03)
49+
50+
- **Scope: both platforms** in one increment (iOS typed error + enumeration;
51+
Android enumeration; shared gate).
52+
- **Strict on absence + auto-invalidate at open**: a runner not advertising
53+
`commands` (every pre-fix artifact) is classified stale; remediation happens
54+
at `device_snapshot action=open` where cold builds are already accepted.
55+
Mid-flow tools never trigger a silent multi-minute build (#210 rule).
56+
- **Approach: `/health.commands` enumeration** over `cmd:*` capability entries
57+
(display pollution, semantic collision with feature flags) and over a
58+
human-bumped `commandSetVersion` int (reintroduces the #383 drift failure
59+
mode; no missing-verb diagnostics).
60+
61+
## Design
62+
63+
### 1. Wire change (additive — no protocol bump)
64+
65+
`/health` on both runners gains a `commands: string[]` field alongside
66+
`ok/protocolVersion/runnerVersion/capabilities`.
67+
68+
- **iOS** (`RnFastRunnerTests+Models.swift`, `+Transport.swift`):
69+
`CommandType` adopts `CaseIterable`; the health response includes
70+
`CommandType.allCases.map(\.rawValue)`. `Response` gains an optional
71+
`commands: [String]?` (nil-omitted, backwards-compatible).
72+
- **Android** (`CommandServer.kt`, `CommandDispatcher.kt`): a
73+
`SUPPORTED_COMMANDS` list declared adjacent to the dispatcher's `when`,
74+
emitted from `/health`.
75+
- **Sync test**: extend the `gh-383-protocol-sync.test.js` source-parsing
76+
pattern — one Node test asserts (a) Kotlin `SUPPORTED_COMMANDS` ==
77+
the dispatcher's `when` labels, (b) Swift `CommandType` cases ⊇
78+
`REQUIRED_IOS_COMMANDS`, (c) Kotlin list ⊇ `REQUIRED_ANDROID_COMMANDS`.
79+
80+
`capabilities` keeps its current meaning (feature flags like
81+
`QUIESCENCE_BYPASS`); commands do NOT go there.
82+
83+
### 2. Gate (TS — `scripts/cdp-bridge/src/runners/protocol.ts`)
84+
85+
- New per-platform constants: `REQUIRED_IOS_COMMANDS` /
86+
`REQUIRED_ANDROID_COMMANDS` — exactly the verbs each client dispatches.
87+
Tied to the existing client command union types via `satisfies` so adding a
88+
verb to the union without updating the list is a compile error.
89+
- `classifyRunnerCompatibility` gains an optional commands input and a new
90+
`RunnerIncompatibilityReason: 'missing-commands'`; the result carries
91+
`missing: string[]` (for absence, `missing` = the full required list).
92+
- **Strict on absence**: `commands === undefined``missing-commands`.
93+
- Check order: legacy → protocol-older/newer → version-skew →
94+
missing-commands (most fundamental mismatch wins).
95+
- Both platform probes (`defaultHttpProbe` in `rn-fast-runner-client.ts`,
96+
the Android info fetch in `rn-android-runner-client.ts`) parse `commands`;
97+
liveness detail gains `commands?: string[]` and the new stale reason flows
98+
through existing plumbing.
99+
100+
### 3. Remediation — two tiers
101+
102+
`missing-commands` joins `PROTOCOL_STALE_REASONS` (reap + respawn + re-verify),
103+
but because respawn reuses the same binary, artifact staleness needs its own
104+
tier:
105+
106+
- **At `device_snapshot action=open`**: when the probe (before or after
107+
respawn) classifies `missing-commands`, invalidate the artifact —
108+
iOS: stop the runner and delete the **plugin-owned** DerivedData
109+
(`derivedDataPathForRunner()` only; never a user path), then let the
110+
existing self-build-on-first-use path cold-build; Android: force the #309
111+
self-install rebuild/reinstall. Result carries
112+
`meta.note: "runner artifact stale (missing commands: <list>) — rebuilt"`.
113+
- **Mid-flow tools** (`device_find`/`press`/`fill` auto-spawn via
114+
`ensureRunnerForCommand`): if still `missing-commands` after the standard
115+
reap+respawn, return a structured error with new
116+
`ToolErrorCode: 'RUNNER_COMMANDS_STALE'` naming the missing verbs and
117+
pointing at `device_snapshot action=open` (which now self-heals). No silent
118+
cold build mid-flow, ever.
119+
120+
### 4. Typed error on iOS `/command` (defense-in-depth)
121+
122+
`handleRequestBody` pre-decodes `{command: String}`; if
123+
`CommandType(rawValue:)` is nil it returns
124+
`ok:false, error:{code:"UNSUPPORTED_COMMAND", message:"Unsupported iOS runner
125+
command: <verb> — runner artifact predates it; re-open the device session
126+
(device_snapshot action=open) to rebuild"}` — mirroring Android's existing
127+
shape. `ErrorPayload` gains an optional `code` field if absent. Any other
128+
decode failure keeps today's behavior.
129+
130+
TS `runIOS` maps `code === 'UNSUPPORTED_COMMAND'` to a clean `failResult`
131+
with the same hint. **No auto-retry** — the gate owns the proactive path;
132+
this branch only catches mid-session hot-swaps.
133+
134+
### 5. Observability
135+
136+
- `cdp_status.deviceSession.runnerProtocol` gains `missingCommands: string[]`
137+
when the runner is stale for this reason.
138+
- `runnerCapabilities` display rule from #384 (omit empty; feature flags only)
139+
is unchanged.
140+
141+
## Testing
142+
143+
- **TS unit (hermetic, deps-injected)**: classify matrix (present / absent /
144+
subset-missing / ordering vs protocol+skew reasons); probe parsing of
145+
`commands` on both clients; `ensureRunnerForCommand` respawn note + terminal
146+
`RUNNER_COMMANDS_STALE`; open-path artifact invalidation (fake fs/build
147+
deps); `runIOS` mapping of `UNSUPPORTED_COMMAND`.
148+
- **Tri-file sync test** extension (section 1).
149+
- **Swift unit**: `CommandType.allCases` contains every dispatched verb
150+
(e.g. `keyboardDismiss`); unknown-verb request → typed
151+
`UNSUPPORTED_COMMAND` response.
152+
- **Device verification**: temporarily add a fake verb to
153+
`REQUIRED_IOS_COMMANDS` in a local build → gate classifies live runner
154+
stale → open-path rebuild loop proven end-to-end on the simulator; Android
155+
equivalent with the emulator. Normal pass afterwards (no fake verb) proves
156+
zero-regression happy path.
157+
158+
## Non-goals
159+
160+
- No protocol version bump (the change is additive).
161+
- No auto-retry after an `UNSUPPORTED_COMMAND` reply.
162+
- No enumeration of `device_batch`-internal sub-verbs beyond what the clients
163+
already dispatch as `/command` verbs.
164+
- No change to `capabilities` semantics.
165+
166+
## Addendum (planning discovery, 2026-07-03): B235's true root cause
167+
168+
Plan-time code archaeology (`git log -S`) showed the Swift `CommandType` enum
169+
**never** had a `dismissKeyboard` case — it has been `keyboardDismiss` since
170+
the original import — while the TS bridge posts `dismissKeyboard`
171+
(`buildRunIOSArgs` `case 'keyboard'`). The B235 incident was therefore not a
172+
stale artifact at all: the explicit iOS keyboard-dismiss wire verb has NEVER
173+
decoded on any artifact. (The #356/#370 keyboard-guard dismissal is
174+
Swift-internal via the `guardKeyboard` flag on taps, which is why
175+
device-verified keyboard work still passed.)
176+
177+
Consequences:
178+
- The plan adds **Task 1: fix the wire verb TS-side** (`dismissKeyboard`
179+
`keyboardDismiss` in `buildRunIOSArgs` + the `RunIOSArgs` union) so old AND
180+
new artifacts accept it. Android keeps `dismissKeyboard` (its Kotlin
181+
when-label) — the platforms legitimately differ, which is itself an argument
182+
for per-platform REQUIRED lists.
183+
- The gate blind spot this spec addresses remains real and worth fixing —
184+
nothing fingerprints the artifact's command surface — but the device
185+
verification uses the genuine pre-#418 artifact (absent `commands` field)
186+
as the migration test instead of the fake-verb protocol from the Testing
187+
section, which is now optional.
188+
189+
## Risks / mitigations
190+
191+
- **Upgrade friction**: every existing DerivedData prebuild is classified
192+
stale once → one announced cold rebuild at next session open. Accepted in
193+
brainstorm (matches the #383 'legacy' precedent; the alternative leaves
194+
B235 unfixed for exactly those artifacts).
195+
- **REQUIRED list under-maintenance**: a future verb added to the enum but not
196+
the REQUIRED list is fail-soft — the gate won't protect it, but the typed
197+
`UNSUPPORTED_COMMAND` error still names it. The `satisfies` tie to the
198+
client command unions plus the sync test make the miss unlikely.
199+
- **DerivedData deletion safety**: deletion is scoped to
200+
`derivedDataPathForRunner()` (in-tree `scripts/rn-fast-runner/build/`),
201+
never a user-configurable path.

0 commit comments

Comments
 (0)