Skip to content

feat(flow): gate flows on a requires block - #751

Open
j-piasecki wants to merge 1 commit into
mainfrom
jpiasecki/flow-requires
Open

feat(flow): gate flows on a requires block#751
j-piasecki wants to merge 1 commit into
mainfrom
jpiasecki/flow-requires

Conversation

@j-piasecki

Copy link
Copy Markdown
Member

A directory run executes every flow it finds, so a mixed suite has no way to say "this one is Apple TV only". This adds a third top-level key naming the targets a flow supports:

requires:
  platform: [ios, android]   # bare string or list; ios covers ios-remote
  runtimeKind: tv            # tv | mobile, ANDed with platform

No block means the flow runs anywhere, so every existing flow is unaffected.

Requirements narrow device auto-detection rather than only judging its winner: with a simulator and an emulator both booted, an ios-only flow now picks the simulator instead of failing as ambiguous. runtimeKind comes free off the list-devices payload resolveFlowDevice was already discarding; only an explicit --device pays a probe, and those are memoized.

On a target that does not satisfy them the runner raises FLOW_REQUIREMENTS_UNMET, which a directory run turns into a per-flow skip and carries on — that is the feature. A single-flow run stays an error (you asked for it by name), and so does a run: fragment the device cannot satisfy: a composed fragment silently not running would leave a green report for a scenario that only half happened, which is what when: is for. A batch that skipped everything exits 2 under a NONE RAN verdict, so a mistyped requires cannot read as a green suite.

Unsatisfiable combinations are rejected at parse — a tv requirement on chromium alone, or a launch step declaring no app id for a platform requires.platform claims (judged per when-guard scope, so an ios-only launch behind an ios guard is not a contradiction).

flow-finish-recording now returns a requiresPrompt when the finished flow declares no block, so the agent asks the user whether to restrict it. That is the first moment the whole flow exists, and an unasked question silently answers itself "anywhere". The prompt suggests the launch step's platforms when they already narrow the flow.

Open question, deliberately left for later

mobile is the existing internal complement of tv, so a chromium desktop window classifies as mobile. Reusing the one word keeps a single vocabulary with list-devices, but tv/desktop/mobile (or tv/desktop/handheld) as three values would be more honest. That is a wider change — it touches list-devices' reported runtimeKind and the Android/iOS probes — so it is not folded in here.

A directory run executes every flow it finds, so a mixed suite has no way
to say "this one is Apple TV only". Adds a third top-level key naming the
targets a flow supports:

  requires:
    platform: [ios, android]   # bare string or list; ios covers ios-remote
    runtimeKind: tv            # tv | mobile, ANDed with platform

No block means the flow runs anywhere, so every existing flow is
unaffected.

Requirements narrow device auto-detection rather than only judging its
winner: with a simulator and an emulator both booted, an ios-only flow
now picks the simulator instead of failing as ambiguous. runtimeKind
comes free off the list-devices payload resolveFlowDevice was already
discarding; only an explicit --device pays a probe, and those are
memoized.

On a target that does not satisfy them the runner raises
FLOW_REQUIREMENTS_UNMET, which a directory run turns into a per-flow skip
and carries on — that is the feature. A single-flow run stays an error
(you asked for it by name), and so does a run: fragment the device cannot
satisfy: a composed fragment silently not running would leave a green
report for a scenario that only half happened, which is what `when:` is
for. A batch that skipped everything exits 2 under a NONE RAN verdict, so
a mistyped requires cannot read as a green suite.

Unsatisfiable combinations are rejected at parse — a tv requirement on
chromium alone, or a launch step declaring no app id for a platform
requires.platform claims (judged per when-guard scope, so an ios-only
launch behind an ios guard is not a contradiction).

flow-finish-recording now returns a requiresPrompt when the finished flow
declares no block, so the agent asks the user whether to restrict it.
That is the first moment the whole flow exists, and an unasked question
silently answers itself "anywhere". The prompt suggests the launch step's
platforms when they already narrow the flow.

Open question, deliberately left for later: `mobile` is the existing
internal complement of `tv`, so a chromium desktop window classifies as
`mobile`. Reusing the one word keeps a single vocabulary with
list-devices, but tv/desktop/mobile (or tv/desktop/handheld) as three
values would be more honest. That is a wider change — it touches
list-devices' reported runtimeKind and the Android/iOS probes — so it is
not folded in here.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the requires: block end to end and exercised it against real targets with a build of this branch: two Android emulators, an iOS simulator, an Apple TV simulator, and the Chromium launch path. The feature works on the happy paths I drove — auto-detect narrowing picked the right emulator out of two, a runtimeKind: tv flow selected the booted Apple TV and passed, the chromium hoist refused before booting, and a run: fragment the device could not satisfy errored the step rather than skipping it.

One behavioural issue is worth resolving before merge, plus a set of smaller ones. Comments are inline.

function meetsRequires(d: RawDevice, requires: FlowRequires | undefined): boolean {
if (!requires) return true;
if (requires.platform && !platformMeets(d.platform, requires.platform)) return false;
if (requires.runtimeKind && listedRuntimeKind(d) !== requires.runtimeKind) return false;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the list-devices payload carries no runtimeKind for a booted device, this treats "could not be read" identically to "wrong kind": the device is filtered out, resolveFlowDevice raises FLOW_REQUIREMENTS_UNMET, and a directory run turns that into a per-flow skip. The explicit --device path routes the same condition through assertDeviceMeetsRequires, which raises FLOW_REQUIREMENTS_UNVERIFIABLE and fails loudly — and the comment on that code in failure-codes.ts states the rule this path breaks: "we never found out" must not be silently skipped by a directory run.

Reproduced against two real booted Android targets, where emulator-5554's form factor cannot be read (pm/getprop unavailable, so the listing reports no runtimeKind) and emulator-5556 reports mobile:

$ argent flow run ./suite --platform android
[1/2] needs-mobile.yaml
  PASS (started on emulator-5556) — 1 passed, 0 failed, 0 errored, 0 skipped
[2/2] needs-tv.yaml
  · skipped — No booted device satisfies this flow's requires: { runtimeKind: tv }. Available devices: emulator-5554 (android, device, kind unknown), emulator-5556 (android, device, mobile).

PASS — 2 flows: 1 passed, 0 failed, 1 skipped
$ echo $?
0

Same flow, same device, explicit id:

$ argent flow run ./suite/needs-tv.yaml --device emulator-5554
This flow declares requires: { runtimeKind: tv }. The runtime kind of device emulator-5554 could not be determined (android), so the requirement cannot be verified.
$ echo $?
1

The message already prints kind unknown, so the runner knows it could not classify the device and still emits the skip code. The fact is also recoverable rather than genuinely unknown: with the listing enrichment failing under host load I saw emulator-5556 come back with no runtimeKind at all (and null model/avdName/sdkLevel), while getAndroidRuntimeKind('emulator-5556') in a fresh process answered mobile. adb.ts documents both ways this happens — pm not yet answering mid-boot, and the ATV emulator specifically staying indeterminate.

Net effect in the use case this feature exists for: in a mixed suite where the other flows pass, the run prints PASS and exits 0 with a regression test that never executed.

// A launch declaring no id for the run's platform is a run-time error
// (flow-run's runLaunch), so a launch missing an id for a platform the flow
// claims to support is that same error, decidable here without a device.
for (const { app, allowed } of launchesInScope(flow.steps, platform)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seeds launchesInScope with the raw requires.platform list, so platforms that requires.runtimeKind has already excluded are still required to declare an app id — platformCanPresent is consulted only by the standalone contradiction check above.

requires: { platform: [ios, android, vega], runtimeKind: mobile } with launch: { ios: …, android: … } is rejected at parse with "a launch step declares no app id for vega", although vega is always tv and no run of that file can reach a vega device — assertDeviceMeetsRequires refuses one for that exact block, and the auto-detect filter and chromium hoist hard-code the same fact. { platform: [ios, chromium], runtimeKind: tv } with launch: { native: … } is rejected the same way for chromium.

That second block is the one flow-requires.test.ts:191 asserts is legal ("allows the combination when one named platform can present the kind"); it stays legal only because that case has steps: []. Adding any launch step turns the blessed block into a hard parse error.

// after booting would strand a live Electron process. Parse-time
// validation does not cover this (a `requires` naming only a runtimeKind
// constrains no launch), which is why the check belongs on this path.
assertPlatformMeetsRequires(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This judges flow.requires — the root file's — but leadingLaunch deliberately crosses run: boundaries and already parses each fragment on the way. When the leading launch is reached through a run: chain, the fragment's own requires is not consulted here, so the Electron boot is attempted before it is judged at the run: step.

Observed side by side: requires: { runtimeKind: tv } on the root with a chromium leading launch is refused with no boot ("chromium is always mobile"); moving the identical block to a fragment reached by run: produces Electron boot: path does not exist instead — execution entered bootChromiumForFlow.

The instance is torn down by the finally, so nothing is stranded, contrary to the hazard the comment names. What remains is a full boot/teardown cycle for a run that is already decided, and a boot-shaped error replacing the actionable requirements one.


/** Validate cross-field invariants that are checkable without other files. */
export function validateFlow(flow: FlowFile): void {
validateRequires(flow);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooking validateRequires into validateFlow also puts it on the recording path: appendStep re-parses the whole file on every recorded step, so this now runs on each flow-add-step. A flow that already declares requires: { platform: [ios, android] } and then records its first launch step for one platform fails the append:

APPEND THREW: This flow's requires block can never be satisfied: a launch step declares no app id for android, which requires.platform says the flow supports

The recording stops on an error about a block the author is not editing. The test helper added in this PR describes hand-editing the file mid-recording as the supported door for writing a requires block, which is exactly the sequence that lands here.

console.log(`\n${renderBatchSummary(counts)}`);
if (ranNothing) {
console.error(
`No flow ran: every flow in ${dir} was skipped. Check the requires: blocks against ` +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This explanation sits inside the non-JSON branch, so argent flow run <dir> --json on a fully skipped batch exits 2 with completely empty stderr — verified, zero bytes. The per-flow error print earlier in this loop goes to stderr regardless of --json, so the omission is inconsistent with the surrounding code, and a CI leg that captures stdout as an artifact and surfaces stderr in the log gets a red build with no message.

The single-flow counterpart is emptier still: argent flow run x.yaml --json with unmet requires exits 1 having written zero bytes to stdout, so piping to jq fails rather than yielding ok: false.

* answer the file already knows — worth offering rather than making the agent
* re-derive it.
*/
function launchPlatforms(flow: FlowFile): WhenPlatform[] | null {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads firstLaunch only, while the validator that grades the resulting block walks every launch in scope. A flow whose launches are split across when: { platform: … } blocks — ios first, android second — is told requires: { platform: [ios] } is the likely answer. Writing that parses and validates cleanly, and then makes every android run of the flow skip in a directory run, with the android half dead and nothing reporting it.

Worth noting alongside: for flows the recorder actually produces the hint never fires at all, since flow-add-step records a launch as a bare app-id string, which names all four platforms. The hint only reaches hand-edited launch maps — which is also where it is most likely to be wrong.

});
});

it("names the unreadable kind that ruled a device out, rather than listing it as eligible", async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case is the exact configuration where the runner silently skips — a requires-constrained auto-detect whose only candidate has an unreadable kind — but it asserts only the message. Swapping the error code at flow-device.ts:313 to FLOW_DEVICE_RESOLUTION leaves the whole file green, and that code is what the CLI keys the per-flow skip on.

The neighbouring case at :348, whose title and comment say the code distinction is load-bearing, also asserts only the message. FLOW_REQUIRES_UNSATISFIABLE is asserted nowhere in the repo — the whole parse-rejection block matches prose only. Other branches where a mutation survives the suite: the vega/chromium arms of both probeRuntimeKind and listedRuntimeKind (no test puts either platform through a requires-constrained run), firstLaunch's recursion into when: blocks, and the non-platform-guard branch of launchesInScope.

## Flow file format

The top-level is an object with `steps` (array) and — fragments only — `executionPrerequisite` (an e2e flow, one beginning with `launch:`, has none). Besides the directives above:
The top-level is an object with `steps` (array), an optional `requires` block, and — fragments only — `executionPrerequisite` (an e2e flow, one beginning with `launch:`, has none). Besides the directives above:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The top-level key list here was updated, but the "Standalone runner" paragraph at line 121 mirrors the CLI help and was not updated with it. Three claims in it are now wrong or incomplete: the runner no longer only "exits non-zero on any failure" (a fully skipped batch exits 2 with zero failures); the summary is no longer always passed/failed/skipped with a PASS/FAIL verdict (NONE RAN is new and appears in no document); and the closing enumeration of batch dispositions — invalid file fails alone, infra error stops the batch — is now missing the requires-unmet skip, which is the disposition this PR adds.


- `- echo: <message>` — a label printed during replay
- `- tool: <name>` with optional `args:` — a raw tool call. A tool step may also carry `delayMs: <ms>` to sleep that long before it runs. (`await-ui-element` is an ordinary tool step; see _flow-add-step arguments_ and _Making flows resilient_ for when to gate a transition with one.)
- **`requires:`** names the targets a flow supports — `platform: [ios, android]` (one platform or a list; `ios` covers a remote simulator) and `runtimeKind: tv | mobile` (`tv` is a focus-driven remote environment — Apple TV, Android TV, Fire TV; `mobile` is everything else, chromium included). The keys are ANDed, so `{ platform: [ios, android], runtimeKind: tv }` means Apple TV or Android TV and excludes Fire TV, which `{ runtimeKind: tv }` alone would admit. **No block means the flow runs anywhere**, so every existing flow is unaffected. Requirements also narrow device auto-detection: an ios-only flow picks the booted simulator instead of failing as ambiguous when an emulator is up beside it. On a target that does not satisfy them, `argent flow run <dir>` **skips** the flow (that's the point — one command over a mixed suite), while running that flow on its own is an **error**, and so is a `run:` fragment the run device cannot satisfy (a composed fragment silently not running would leave a green report for a scenario that only half happened — use `when:` when you mean "optionally"). Combinations nothing could satisfy are rejected at parse: `runtimeKind: tv` with `platform: [chromium]`, or a `launch` step declaring no app id for a platform `requires.platform` claims (a `launch` inside a `when: { platform: … }` guard is judged against that guard's platform only).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ios covers a remote simulator" reads as applying to the auto-detection narrowing described two sentences later, but a booted remote simulator is never a candidate there: isBooted in flow-device.ts has no ios-remote case and falls through to return false, and deviceEntryId has no case for it either (such a row renders as ? in the error list). Verified — with a booted remote sim as the only device, an ios-only flow reports "No booted device found", not a requires outcome.

The fold is real, but only via --device remote:<udid>, which is also the only way the new getRemoteSimulatorRuntimeKind is reachable. The gap in isBooted predates this PR; what is new is documentation that reads as if it did not exist.

error stops the batch and counts the remaining flows skipped.

A flow may declare a \`requires:\` block (platform / runtimeKind) naming the
targets it supports. In a directory run a flow the target does not satisfy is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help says a flow the target does not satisfy is skipped in a directory run, without the carve-out that a requirement which could not be verified fails the flow instead (FLOW_REQUIREMENTS_UNVERIFIABLE, error_kind: validation, so the batch continues but the flow is red). Both outcomes are reachable from the same YAML and the same device, differing only in whether the runtime-kind probe answered — which is worth stating here since the two produce opposite exit codes.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two more findings from a second pass, both reproduced against a real Android emulator with a build of this branch.

// as easily a mistyped `requires` (or a target nobody meant to run against)
// as a deliberate no-op, and "PASS — 12 flows: 0 passed" reads as success
// either way, so the exit code has to disagree.
const ranNothing = counts.passed === 0 && counts.failed === 0 && counts.total > 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This guard counts flows, not executed steps, so a single flow that passes vacuously defeats it. A report with zero executed steps is ok: true (flow-run.ts:1489 — skips are not failures), the flow is classified pass at :1189, and ranNothing is then false no matter how many other flows were filtered out.

The shape that produces it is the pre-existing idiom for exactly the platform-specific flows requires is meant to replace: a flow whose steps all sit inside a when: { platform: … } guard. Reproduced with such a flow beside a requires: { runtimeKind: tv } flow, pinned to a mobile Android emulator:

$ argent flow run ./vacuous --device emulator-5556
[1/2] ios-guarded.yaml
  PASS (started on emulator-5556) — 0 passed, 0 failed, 0 errored, 2 skipped
[2/2] tv-required.yaml
  · skipped — This flow declares requires: { runtimeKind: tv }. Device emulator-5556 is mobile, not tv.

PASS — 2 flows: 1 passed, 0 failed, 1 skipped
$ echo $?
0

--json agrees: ok: true, passed: 1, failed: 0, skipped: 1. Not one step executed anywhere in that suite, and the run is green.

A fully-skipped flow reporting ok: true predates this PR. What is new is the guarantee this line is here to provide — that a mistyped requires cannot read as a green suite — and it does not hold in the suites this feature targets, since a migrating mixed suite is precisely where a when: { platform }-wrapped flow sits beside a requires-gated one.

);
}

if (!platform) return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning here when platform is absent means a runtimeKind-only block is never checked against the flow's launches, so a file no target anywhere can run parses clean and is then skipped silently forever rather than failing once.

requires: { runtimeKind: tv } on a flow whose leading launch is chromium-only is unsatisfiable by construction — chromium is always mobile, which the hoist at flow-run.ts:1164 and CONSTANT_RUNTIME_KIND both already know statically. Reproduced in a directory run:

$ argent flow run ./unrunnable --device emulator-5556
[1/2] normal.yaml
  PASS (started on emulator-5556) — 1 passed, 0 failed, 0 errored, 0 skipped
[2/2] tv-chromium.yaml
  · skipped — This flow declares requires: { runtimeKind: tv }. Device emulator-5556 is mobile, not tv.

PASS — 2 flows: 1 passed, 0 failed, 1 skipped
$ echo $?
0

The broken file is indistinguishable from one that legitimately does not apply here, on every target, in every run. The SKILL text added in this PR states that "combinations nothing could satisfy are rejected at parse", which is the behaviour this early return prevents for the runtimeKind-only case.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Third pass. Three more findings, the first two reproduced against real targets / the real tool with a build of this branch.

const device = await resolveFlowDevice(registry, ctx, {
device: params.device,
platform: params.platform as FlowPlatform | undefined,
requires: flow.requires,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the root file's block reaches auto-detection, so a requires living on a composed fragment never narrows device selection — it can only reject afterwards, at the run: step. A reusable fragment is a natural home for a platform constraint, and a root that merely composes it carries no block of its own.

Same effective constraint, moved between the two files:

# root carries requires: { platform: [android] }
3 booted devices matched — pass --device or --platform to disambiguate.
Available devices: emulator-5554 (android, device), emulator-5556 (android, device), emulator-5584 (android, device).

# identical constraint on the fragment instead, root bare
8 booted devices matched — pass --device or --platform to disambiguate.
Available devices: <4 ios sims>, <3 android emulators>, chromium-cdp-9222 (chromium, Running).

The narrowing this feature advertises simply does not apply. Worse for the mixed-suite case, the outcome is a failure rather than a skip — FLOW_DEVICE_RESOLUTION is error_kind: validation, so a directory run records it red:

$ argent flow run ./composedir --platform ios
[1/3] android-frag.yaml
  · skipped — This flow declares requires: { platform: [android] }, which excludes the ios target this run was pointed at.
[2/3] composed-android.yaml
  3 booted devices matched — pass --device or --platform to disambiguate. …

FAIL — 3 flows: 0 passed, 2 failed, 1 skipped

The fragment run directly is correctly skipped; the flow that composes it goes red. On a host with a single iOS device it resolves that device instead and then errors at the run: step — either way a red flow where the suite should simply have filtered it out.

`the block to the YAML yourself (there is no tool for it):\n` +
` requires:\n` +
` platform: [ios, android] # one platform or a list; ios covers a remote simulator\n` +
` runtimeKind: tv # tv (Apple TV / Android TV / Fire TV), or mobile for everything else\n` +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prompt tells the agent to hand-write the block into the YAML because no tool can set it — which is accurate, and is also why re-recording silently destroys it. flow-start-recording takes name, project_root and executionPrerequisite only, and writes a fresh two-key flow over whatever is on disk without reading it. requires is the one FlowFile field with no way back in.

Run against the real tool:

BEFORE:
steps:
  - echo: existing step
requires:
  platform:
    - ios
  runtimeKind: tv

tool said: {"message":"Started recording \"restricted\" flow", "flowFile":"steps: []\n", …}

AFTER:
steps: []

No warning in the result. The trigger is a documented workflow — SKILL.md:277 offers "Call flow-start-recording with the same flow name (overwrites)" as the repair path, and Strategy 4 is "record from scratch with the same flow name" — and neither the repair section nor the tool result mentions the block. A flow the user had fenced to a TV silently becomes a run-anywhere flow, so in a directory suite it stops being skipped and starts executing against phones.

flow-finish-recording re-emitting requiresPrompt afterwards softens this only if the agent asks again, and the suggestion it offers is derived from launch app ids rather than from the answer that was just deleted.

FLOW_REQUIRES_UNSATISFIABLE: "FLOW_REQUIRES_UNSATISFIABLE",
// A fine file on the wrong target. Distinct from the above because a
// directory run turns exactly this code into a per-flow skip.
FLOW_REQUIREMENTS_UNMET: "FLOW_REQUIREMENTS_UNMET",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth considering how these land in telemetry and the event log, since the skip is an expected outcome rather than a fault. Because the skip is implemented as a FailureError escaping flow-execute's execute(), the registry emits tool:fail with error_code: FLOW_REQUIREMENTS_UNMET and the event log writes an ERROR-level tool.failed entry reading Failed to run flow <name>: FLOW_REQUIREMENTS_UNMET. A 12-flow mixed suite emits one of those per filtered flow on every run, so routine filtering is indistinguishable from breakage in both the failure-rate metric and the server log.

The mirror image holds for the fragment path: execRunStep catches and converts to a step failure, so flow-execute returns normally and the call telemeters as tool:complete. The same condition is an error event at the root and invisible mid-run, and the UNMET / UNVERIFIABLE distinction these comments argue for is dropped there — only the message text survives.

@hubgan hubgan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last two, from a final sweep of the diff. Both are code-level rather than device-reproduced — I could not stand up a sim-remote orchestrator to drive the first one end to end, so it is reported on the strength of the three code facts it rests on.

* satisfy an iOS requirement.
*/
function platformMeets(platform: Platform, required: readonly WhenPlatform[]): boolean {
const folded = platform === "ios-remote" ? "ios" : platform;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fold admits a remote simulator for requires: { platform: [ios] }, but the launch lookup at run time does not fold, so validateRequires certifies a file for a target its own launch step cannot serve.

Three facts in tension:

  • platformMeets folds ios-remoteios here.
  • validateRequires checks appIdForPlatform(app, "ios") and finds the id, so the file parses clean — and the stated reason for that check is that a launch missing an id for a claimed platform is a run-time error made "decidable here without a device".
  • runLaunch passes device.platform raw: appIdForPlatform(app, "ios-remote"). LAUNCH_MAP_KEYS has no ios-remote spelling (declaring one is parse-rejected), so the lookup falls through to launch.native ?? null.

So requires: { platform: [ios] } with launch: { ios: com.example.app }, run against --device remote:<udid>, passes the parse check and assertDeviceMeetsRequires, then errors at step 0 with no app id declared for platform "ios-remote". The step failure itself predates this PR — the same flow with no requires: block fails identically — but the gate now advertises that target and the parse-time check certifies the file for it, so the one class of failure validateRequires exists to hoist to parse time survives on exactly the platform the fold was added to admit. In a directory run it lands as a fail rather than a requires-skip.

The mixed-map case is quieter and worse: appIdForPlatform({ ios: "com.a", native: "com.b" }, "ios-remote") returns com.b, so a flow certified for ios launches a different bundle on a remote sim. In practice only native: and bare-string launches behave there, which nothing currently documents.

try {
await assertDeviceMeetsRequires(state.device, fragment.requires);
} catch (err) {
return fail(`fragment "${target}" cannot run on this device: ${errMsg(err)}`);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertDeviceMeetsRequires throws either FLOW_REQUIREMENTS_UNMET or FLOW_REQUIREMENTS_UNVERIFIABLE, and both are rendered with this one fixed prefix. When the probe could not answer, the resulting step reason reads:

fragment "tv-bit.yaml" cannot run on this device: This flow declares requires: { runtimeKind: tv }.
The runtime kind of device emulator-5554 could not be determined (android), so the requirement
cannot be verified.

The prefix asserts as fact what the rest of the sentence says was never established. Everywhere else this PR spends real effort keeping the two apart — two failure codes with explanatory comments, a dedicated CLI branch, and a test named "reports an unreadable runtime kind under its own code, not the skip code" — but a report consumer reading this step cannot tell a wrong-target fragment from a broken probe. The remedy text carried along ("Pass a device whose kind is readable, or drop runtimeKind") is also addressed to a pre-flight caller rather than to someone reading a mid-run step. Both outcomes correctly error the step, so the impact is diagnostic only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants