[codex] Flatten app config and prelaunch configured app#48
Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces run-scoped primary app context resolution and bootstrapping for the CLI. It adds app configuration support to Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / CLI
participant Check as Check Runner
participant Test as Test Runner
participant Session as Goal Session
participant Device as Device
participant Executor as Goal Executor
participant Planner as AI Planner
User->>Check: finalrun check --app <path>
Check->>Check: Validate & resolve app config
Check->>Check: Log resolved app summary
User->>Test: finalrun test --app <path>
Test->>Test: Resolve app config from workspace/env/override
Test->>Session: Create goal session with app config
Session->>Device: ensureAppReady(app)
Device->>Device: Query installed apps
Device->>Device: Launch primary app
Device-->>Session: Return launch summary
Session->>Executor: Execute goal with app context
Executor->>Planner: plan(preContext: launchSummary, appIdentifier, appKnowledge)
Planner->>Planner: Generate actions (e.g., LaunchAppAction)
Planner-->>Executor: Action with shouldUninstallBeforeLaunch=false for primary app
Executor->>Device: Execute action
Device-->>Executor: Action result
Executor-->>Session: Execution complete
Session->>Test: Record app context in artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/appConfig.ts`:
- Around line 177-193: The current resolveSelectedPlatform flow lets a preferred
--platform override an inferred platform from --app and skip validation,
allowing mismatched pairs like `--platform ios --app app.apk`; update
resolveSelectedPlatform to detect when both a requestedPlatform
(normalizePlatform of params.requestedPlatform) and an inferredPlatform
(normalizePlatform of params.inferredPlatform or from params.app) exist and
conflict, and in that case throw or return a validation error instead of
returning the requestedPlatform; ensure the check happens early so callers
(bootstrap/prelaunch) never receive a mismatched platform, referencing the
normalizePlatform calls and resolveSelectedPlatform function to locate where to
add the conflict-detection and error path.
- Around line 347-371: The current resolveAndroidToolCandidates function uses
process.env['ANDROID_HOME'] ?? process.env['ANDROID_SDK_ROOT'] which
short-circuits and ignores ANDROID_SDK_ROOT if ANDROID_HOME is set; change the
logic to collect both env values (ANDROID_HOME and ANDROID_SDK_ROOT) when
present, iterate each root to add candidates for 'aapt' (build-tools/*/toolName)
and 'apkanalyzer' (cmdline-tools/latest/bin and tools/bin), then still append
resolveOnPath(toolName) and pass the combined list through dedupeExistingPaths
to collapse duplicates; update references in resolveAndroidToolCandidates and
keep the existing dedupeExistingPaths and resolveOnPath behavior.
In `@packages/goal-executor/src/HeadlessActionExecutor.ts`:
- Around line 566-568: The runtime uses unsafe TypeScript casts on
output['shouldUninstallBeforeLaunch'] (and similarly allowAllPermissions,
clearState, stopAppBeforeLaunch) which don't validate that the value is actually
a boolean; update the parsing logic around shouldUninstallBeforeLaunch (and the
other three flags) to perform a runtime boolean check (e.g., test for === true
or check typeof === 'boolean') and only use the output value when it is a real
boolean, otherwise fall back to the existing defaults (for
shouldUninstallBeforeLaunch keep the packageName === this._primaryAppIdentifier
? false : true behavior). Ensure you reference the same properties on the output
object and preserve existing default semantics for
packageName/_primaryAppIdentifier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 55a26643-bd10-42fa-8819-0e428819a2a3
📒 Files selected for processing (25)
openspec/changes/align-cli-app-precontext-with-dart/.openspec.yamlopenspec/changes/align-cli-app-precontext-with-dart/design.mdopenspec/changes/align-cli-app-precontext-with-dart/proposal.mdopenspec/changes/align-cli-app-precontext-with-dart/specs/cli-primary-app-context/spec.mdopenspec/changes/align-cli-app-precontext-with-dart/tasks.mdpackages/cli/bin/finalrun.tspackages/cli/src/appConfig.tspackages/cli/src/checkRunner.tspackages/cli/src/finalrun.test.tspackages/cli/src/goalRunner.test.tspackages/cli/src/goalRunner.tspackages/cli/src/reportWriter.tspackages/cli/src/specLoader.tspackages/cli/src/testRunner.test.tspackages/cli/src/testRunner.tspackages/cli/src/workspace.test.tspackages/cli/src/workspace.tspackages/common/src/index.tspackages/common/src/models/RepoEnvironment.tspackages/common/src/models/RunArtifacts.tspackages/goal-executor/src/HeadlessActionExecutor.test.tspackages/goal-executor/src/HeadlessActionExecutor.tspackages/goal-executor/src/HeadlessGoalExecutor.test.tspackages/goal-executor/src/HeadlessGoalExecutor.tsskills/generate-finalrun-test/SKILL.md
Contributor
Author
|
Launch-defaults summary for this PR: Shared
What this PR changes on top of that:
Android behavior:
iOS simulator behavior:
So the practical default for this PR’s bootstrap path is: prelaunch the configured app, do not reinstall it, do not clear data, do not force-stop first, and grant permissions using the platform-default “allow all” path. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/cli/src/appConfig.test.ts (1)
1-94: Consider expanding test coverage.The existing tests cover important edge cases (platform conflict, SDK fallback), but there's room for additional tests covering:
readRepoAppConfigvalidation (invalid keys, missing platforms)resolveIOSBundleIdwith plutil fallbacknormalizePlatformedge cases- Error scenarios in
resolvePrimaryAppConfigThis can be addressed in a follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/appConfig.test.ts` around lines 1 - 94, Add follow-up unit tests to expand coverage: add tests for readRepoAppConfig that supply invalid config objects (unknown keys, missing platform entries) and assert it throws/returns validation errors; add tests for resolveIOSBundleId that simulate missing Info.plist but available plutil output (mock filesystem/child process) and assert correct bundle id is returned; add edge-case tests for normalizePlatform with varied inputs (case variants, unexpected strings, null/undefined) and assert normalized outputs or thrown errors; and add additional resolvePrimaryAppConfig error-path tests (e.g., missing workspaceApp entry, ambiguous requestedPlatform/inferredPlatform combos) to assert proper rejection messages. Ensure tests reference the functions resolveIOSBundleId, readRepoAppConfig, normalizePlatform, and resolvePrimaryAppConfig so they’re easy to locate.packages/cli/src/appConfig.ts (1)
399-410:whichis Unix-specific; consider cross-platform note.The
whichcommand works on macOS/Linux but not on Windows. This is likely acceptable for mobile development workflows, but worth documenting if Windows CI runners might be used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/appConfig.ts` around lines 399 - 410, The resolveOnPath function currently calls the Unix-only 'which' via execFileAsync, which fails on Windows CI; update resolveOnPath to be cross-platform by detecting process.platform === 'win32' and using 'where' on Windows or, preferably, replace the execFileAsync call with a cross-platform utility (e.g., the npm 'which' package or implement PATH lookup using process.env.PATH and fs.access) so execFileAsync('which', ...) is no longer relied on; adjust the function and any callers (resolveOnPath, execFileAsync usage) and add a brief comment noting cross-platform behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/src/appConfig.test.ts`:
- Around line 1-94: Add follow-up unit tests to expand coverage: add tests for
readRepoAppConfig that supply invalid config objects (unknown keys, missing
platform entries) and assert it throws/returns validation errors; add tests for
resolveIOSBundleId that simulate missing Info.plist but available plutil output
(mock filesystem/child process) and assert correct bundle id is returned; add
edge-case tests for normalizePlatform with varied inputs (case variants,
unexpected strings, null/undefined) and assert normalized outputs or thrown
errors; and add additional resolvePrimaryAppConfig error-path tests (e.g.,
missing workspaceApp entry, ambiguous requestedPlatform/inferredPlatform combos)
to assert proper rejection messages. Ensure tests reference the functions
resolveIOSBundleId, readRepoAppConfig, normalizePlatform, and
resolvePrimaryAppConfig so they’re easy to locate.
In `@packages/cli/src/appConfig.ts`:
- Around line 399-410: The resolveOnPath function currently calls the Unix-only
'which' via execFileAsync, which fails on Windows CI; update resolveOnPath to be
cross-platform by detecting process.platform === 'win32' and using 'where' on
Windows or, preferably, replace the execFileAsync call with a cross-platform
utility (e.g., the npm 'which' package or implement PATH lookup using
process.env.PATH and fs.access) so execFileAsync('which', ...) is no longer
relied on; adjust the function and any callers (resolveOnPath, execFileAsync
usage) and add a brief comment noting cross-platform behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b99d12e-90cf-4983-8cc7-459bce2d84fc
📒 Files selected for processing (4)
packages/cli/src/appConfig.test.tspackages/cli/src/appConfig.tspackages/goal-executor/src/HeadlessActionExecutor.test.tspackages/goal-executor/src/HeadlessActionExecutor.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/goal-executor/src/HeadlessActionExecutor.ts
- packages/goal-executor/src/HeadlessActionExecutor.test.ts
arnoldlaishram
requested changes
Apr 3, 2026
|
|
||
| **App configuration (required):** | ||
| - FinalRun runs require `.finalrun/config.yaml` to define the default app identity for the repo. | ||
| - Use `app.android.packageName` for Android and `app.ios.bundleId` for iOS. |
Contributor
There was a problem hiding this comment.
We can read the codebase if they have different package name for different environments and do the setup.
| const session = await prepareGoalSession( | ||
| { | ||
| platform: PLATFORM_ANDROID, | ||
| primaryApp: { |
Contributor
There was a problem hiding this comment.
we can simply call it app
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/goalRunner.ts (1)
301-309:⚠️ Potential issue | 🟠 MajorForward
appKnowledgeinto the executor.This call wires
preContextandprimaryAppIdentifier, but it drops the optional app knowledge carried on the resolved primary app. As written, the planner never receivesappKnowledge, so part of the new precontext feature is silently inactive.Suggested fix
const executor = dependencies.createExecutor({ goal: config.goal, platform: session.platform, maxIterations: config.maxIterations, agent: session.device, aiAgent, preContext: session.launchSummary, + appKnowledge: session.primaryApp?.appKnowledge, primaryAppIdentifier: session.primaryApp?.identifier, runtimeBindings: config.runtimeBindings, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/goalRunner.ts` around lines 301 - 309, The executor is created without forwarding the optional app knowledge from the resolved primary app, so include the appKnowledge when calling dependencies.createExecutor: pass the property (e.g. appKnowledge: session.primaryApp?.appKnowledge) alongside preContext and primaryAppIdentifier so the planner receives the new precontext data; update the createExecutor call site (where executor is built) to add the appKnowledge field and ensure any downstream executor usage expects it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/align-cli-app-precontext/design.md`:
- Around line 29-35: Update the "Non-Goals" and the related design/migration
text so they reflect that repo app identity is required (not `--app`-first) and
that config-backed rollout mandates a mandatory `.finalrun/config.yaml` with
environment-specific overrides in `.finalrun/env/<env>.yaml`; specifically,
replace any language saying primary-app resolution is `--app`-first or that
`.finalrun/config.yaml` support is out of scope with statements that the system
requires a repo-level app identity, documents how primary app is resolved from
that config, and describes migration guidance for converting existing `--app`
usage to the new mandatory `.finalrun/config.yaml` plus
`.finalrun/env/<env>.yaml` override pattern (also update the matching guidance
in the other referenced section around lines 113-126).
In
`@openspec/changes/align-cli-app-precontext/specs/cli-primary-app-context/spec.md`:
- Around line 3-18: Update the normative requirements so primary app context is
set not only when an explicit `--app` override is provided but also when a
repo-config app identity exists; revise the Scenario sections under "CLI run
resolves a primary app context for explicit app inputs" to (1) add a new
scenario describing that when `.finalrun/config.yaml` defines the app identity
(with optional environment-specific overrides in `.finalrun/env/<env>.yaml`),
the run SHALL resolve and store a primary app context containing platform,
user-facing label, source, and a launchable app identifier, and (2) modify the
"No explicit app input" scenario to clarify it applies only when neither `--app`
nor repo-config/app env overrides are present; ensure language uses normative
"SHALL" for repo-config behavior and references `.finalrun/config.yaml` and
`.finalrun/env/<env>.yaml` as valid sources of the primary app identity.
---
Outside diff comments:
In `@packages/cli/src/goalRunner.ts`:
- Around line 301-309: The executor is created without forwarding the optional
app knowledge from the resolved primary app, so include the appKnowledge when
calling dependencies.createExecutor: pass the property (e.g. appKnowledge:
session.primaryApp?.appKnowledge) alongside preContext and primaryAppIdentifier
so the planner receives the new precontext data; update the createExecutor call
site (where executor is built) to add the appKnowledge field and ensure any
downstream executor usage expects it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dff1663a-44a5-42ab-a1c3-2a4a39d285f8
📒 Files selected for processing (9)
openspec/changes/align-cli-app-precontext/.openspec.yamlopenspec/changes/align-cli-app-precontext/design.mdopenspec/changes/align-cli-app-precontext/proposal.mdopenspec/changes/align-cli-app-precontext/specs/cli-primary-app-context/spec.mdopenspec/changes/align-cli-app-precontext/tasks.mdpackages/cli/bin/finalrun.tspackages/cli/src/goalRunner.tspackages/goal-executor/src/HeadlessActionExecutor.tspackages/goal-executor/src/HeadlessGoalExecutor.ts
✅ Files skipped from review due to trivial changes (3)
- openspec/changes/align-cli-app-precontext/.openspec.yaml
- packages/cli/bin/finalrun.ts
- openspec/changes/align-cli-app-precontext/tasks.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/goal-executor/src/HeadlessGoalExecutor.ts
- packages/goal-executor/src/HeadlessActionExecutor.ts
Comment on lines
+29
to
+35
| **Non-Goals:** | ||
|
|
||
| - Introduce a full multi-app orchestration system for YAML tests. | ||
| - Change YAML `setup:` into a separately executed setup phase. | ||
| - Add a UI/editor app registry. | ||
| - Make primary app context mandatory for all runs. Runs without a resolvable primary app should continue to work as they do now. | ||
|
|
There was a problem hiding this comment.
Update the design scope to match the config-backed rollout.
This still describes primary-app resolution as --app-first and treats mandatory .finalrun/config.yaml support as out of scope. The PR objective is the opposite: repo app identity is required, with .finalrun/env/<env>.yaml overrides. Please rewrite these sections so the design and migration guidance match the behavior being shipped.
Also applies to: 113-126
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/align-cli-app-precontext/design.md` around lines 29 - 35,
Update the "Non-Goals" and the related design/migration text so they reflect
that repo app identity is required (not `--app`-first) and that config-backed
rollout mandates a mandatory `.finalrun/config.yaml` with environment-specific
overrides in `.finalrun/env/<env>.yaml`; specifically, replace any language
saying primary-app resolution is `--app`-first or that `.finalrun/config.yaml`
support is out of scope with statements that the system requires a repo-level
app identity, documents how primary app is resolved from that config, and
describes migration guidance for converting existing `--app` usage to the new
mandatory `.finalrun/config.yaml` plus `.finalrun/env/<env>.yaml` override
pattern (also update the matching guidance in the other referenced section
around lines 113-126).
Comment on lines
+3
to
+18
| ### Requirement: CLI run resolves a primary app context for explicit app inputs | ||
|
|
||
| When a CLI run includes an explicit primary app input, the system SHALL resolve a run-scoped primary app context that includes the platform, user-facing label, source, and a launchable app identifier such as package name or bundle identifier. | ||
|
|
||
| #### Scenario: App override resolves primary app identity | ||
|
|
||
| - **WHEN** a user runs `finalrun test` or `finalrun suite` with `--app <path>` | ||
| - **AND** the CLI successfully validates and installs that override | ||
| - **THEN** the run SHALL store a primary app context for the shared goal session | ||
| - **AND** that context SHALL include the resolved app identifier needed to launch the installed app again without relying on the override file path | ||
|
|
||
| #### Scenario: No explicit app input leaves primary app context unset | ||
|
|
||
| - **WHEN** a CLI run starts without an explicit primary app input | ||
| - **THEN** the system SHALL leave primary app context unset | ||
| - **AND** it SHALL preserve existing goal-execution behavior without bootstrap app launch |
There was a problem hiding this comment.
The requirements still exclude the mandatory repo-config flow.
These scenarios make primary app context depend on explicit --app input and say runs without it keep the context unset. That no longer matches the PR: .finalrun/config.yaml app identity is required, and .finalrun/env/<env>.yaml can provide environment-specific overrides. Please update the normative requirements so acceptance coverage includes the repo-config path, not just override-backed runs.
Also applies to: 67-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/align-cli-app-precontext/specs/cli-primary-app-context/spec.md`
around lines 3 - 18, Update the normative requirements so primary app context is
set not only when an explicit `--app` override is provided but also when a
repo-config app identity exists; revise the Scenario sections under "CLI run
resolves a primary app context for explicit app inputs" to (1) add a new
scenario describing that when `.finalrun/config.yaml` defines the app identity
(with optional environment-specific overrides in `.finalrun/env/<env>.yaml`),
the run SHALL resolve and store a primary app context containing platform,
user-facing label, source, and a launchable app identifier, and (2) modify the
"No explicit app input" scenario to clarify it applies only when neither `--app`
nor repo-config/app env overrides are present; ensure language uses normative
"SHALL" for repo-config behavior and references `.finalrun/config.yaml` and
`.finalrun/env/<env>.yaml` as valid sources of the primary app identity.
droid-ash
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/finalrun.test.ts (1)
68-82: Tightenapp:detection to avoid accidental false positives in test fixtures.Line 77 uses
/^app:/m, which can match unintendedapp:lines in multi-line scalar content. A stricter top-level key check (or lightweight YAML parse) would make fixture generation more deterministic.Suggested tweak
- if (/^app:/m.test(configYaml)) { + if (/^app:\s*$/m.test(configYaml)) { return configYaml; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/finalrun.test.ts` around lines 68 - 82, In buildWorkspaceConfigYaml the regexp /^app:/m can match “app:” inside multiline scalar content; replace that loose check with a strict top-level key detection by parsing only the first non-empty, non-comment line (or do a lightweight YAML parse) to verify it equals "app:" (e.g., inspect the first left-aligned key before any indentation) and only then return configYaml; otherwise append the defaultAppConfig as before. Ensure changes reference buildWorkspaceConfigYaml and preserve the existing null/undefined behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/src/finalrun.test.ts`:
- Around line 68-82: In buildWorkspaceConfigYaml the regexp /^app:/m can match
“app:” inside multiline scalar content; replace that loose check with a strict
top-level key detection by parsing only the first non-empty, non-comment line
(or do a lightweight YAML parse) to verify it equals "app:" (e.g., inspect the
first left-aligned key before any indentation) and only then return configYaml;
otherwise append the defaultAppConfig as before. Ensure changes reference
buildWorkspaceConfigYaml and preserve the existing null/undefined behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b100b30-2cc4-4c8e-b1a4-16c8468d56bd
📒 Files selected for processing (8)
packages/cli/src/appConfig.test.tspackages/cli/src/appConfig.tspackages/cli/src/finalrun.test.tspackages/cli/src/testRunner.test.tspackages/cli/src/workspace.test.tspackages/common/src/index.tspackages/common/src/models/RepoEnvironment.tsskills/generate-finalrun-test/SKILL.md
✅ Files skipped from review due to trivial changes (3)
- packages/common/src/models/RepoEnvironment.ts
- packages/cli/src/appConfig.test.ts
- packages/cli/src/appConfig.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/common/src/index.ts
- skills/generate-finalrun-test/SKILL.md
- packages/cli/src/workspace.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
openspec/changes/archive/2026-04-03-align-cli-app-precontext/specs/cli-primary-app-context/spec.md (2)
67-75:⚠️ Potential issue | 🟠 MajorRun-artifact requirements are incomplete for config/env-backed runs.
Line 71-Line 75 only specify override-backed artifact recording. The spec should also require recording resolved primary app identity when source is config/env, including source classification and resolved identifier fields, so artifacts match the new mandatory config flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-04-03-align-cli-app-precontext/specs/cli-primary-app-context/spec.md` around lines 67 - 75, The spec currently only mandates recording resolved primary app identity for override-backed runs (Scenario: Override-backed run records resolved app identity); extend the requirement to also cover config/env-backed runs by adding language that the run inputs artifact SHALL record the app source classification (e.g., "config" or "env") and the resolved app identifier used for primary-app launch behavior when the primary app is determined from configuration or environment, either by adding a new scenario (e.g., "Config/env-backed run records resolved app identity") or by generalizing the existing requirement to enumerate all possible sources (override, config, env) and the two required fields (source and resolved identifier).
3-18:⚠️ Potential issue | 🟠 MajorSpec contradicts mandatory repo-config primary app resolution.
Line 3 and Line 14 still define primary app context as explicit-
--app-only behavior, and Line 16-Line 18 require context to remain unset without explicit input. That conflicts with the PR contract where.finalrun/config.yamlis mandatory and should resolve primary app context (with optional.finalrun/env/<env>.yamlreplacement). Please add a normative config/env-backed scenario and narrow the “unset” case to only when neither CLI input nor repo/env config provides app identity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-04-03-align-cli-app-precontext/specs/cli-primary-app-context/spec.md` around lines 3 - 18, Update the spec to add a normative scenario where the repo config resolves the primary app context: add a new "Scenario: Repo/env config resolves primary app identity" that describes when .finalrun/config.yaml (and optional .finalrun/env/<env>.yaml) exist the CLI SHALL derive the run-scoped primary app context (platform, label, source, launchable identifier) from that config without requiring --app, and ensure it is used for the shared goal session; then narrow the existing "No explicit app input" scenario so it states the primary app context is left unset only when neither an explicit --app nor a repo/env config provides app identity. Reference the existing scenario headings ("Scenario: App override resolves primary app identity" and "Scenario: No explicit app input leaves primary app context unset") when making these edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@openspec/changes/archive/2026-04-03-align-cli-app-precontext/specs/cli-primary-app-context/spec.md`:
- Around line 67-75: The spec currently only mandates recording resolved primary
app identity for override-backed runs (Scenario: Override-backed run records
resolved app identity); extend the requirement to also cover config/env-backed
runs by adding language that the run inputs artifact SHALL record the app source
classification (e.g., "config" or "env") and the resolved app identifier used
for primary-app launch behavior when the primary app is determined from
configuration or environment, either by adding a new scenario (e.g.,
"Config/env-backed run records resolved app identity") or by generalizing the
existing requirement to enumerate all possible sources (override, config, env)
and the two required fields (source and resolved identifier).
- Around line 3-18: Update the spec to add a normative scenario where the repo
config resolves the primary app context: add a new "Scenario: Repo/env config
resolves primary app identity" that describes when .finalrun/config.yaml (and
optional .finalrun/env/<env>.yaml) exist the CLI SHALL derive the run-scoped
primary app context (platform, label, source, launchable identifier) from that
config without requiring --app, and ensure it is used for the shared goal
session; then narrow the existing "No explicit app input" scenario so it states
the primary app context is left unset only when neither an explicit --app nor a
repo/env config provides app identity. Reference the existing scenario headings
("Scenario: App override resolves primary app identity" and "Scenario: No
explicit app input leaves primary app context unset") when making these edits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 517408a0-4eb7-432f-8bfd-6cef0c105ae1
📒 Files selected for processing (6)
openspec/changes/archive/2026-04-03-align-cli-app-precontext/.openspec.yamlopenspec/changes/archive/2026-04-03-align-cli-app-precontext/design.mdopenspec/changes/archive/2026-04-03-align-cli-app-precontext/proposal.mdopenspec/changes/archive/2026-04-03-align-cli-app-precontext/specs/cli-primary-app-context/spec.mdopenspec/changes/archive/2026-04-03-align-cli-app-precontext/tasks.mdopenspec/specs/cli-primary-app-context/spec.md
✅ Files skipped from review due to trivial changes (4)
- openspec/changes/archive/2026-04-03-align-cli-app-precontext/.openspec.yaml
- openspec/changes/archive/2026-04-03-align-cli-app-precontext/tasks.md
- openspec/changes/archive/2026-04-03-align-cli-app-precontext/proposal.md
- openspec/specs/cli-primary-app-context/spec.md
arnoldlaishram
approved these changes
Apr 3, 2026
This was referenced Apr 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.finalrun/config.yamlapp identity mandatory and treat it as the source of truth for repo-local runsappobject:name,packageName,bundleId.finalrun/env/<env>.yamlto replace the baseappobject for env-specific app identity--appas a binary override only, validate it against the configured app identity, and prelaunch the resolved app once before AI executionConfig Contract
This PR changes the app config shape.
Before:
After:
Env overrides use the same flat shape and replace the base
appobject for that env:Important behavior changes:
app.android/app.iosis no longer acceptedapp.finalrun/config.yamlmust define at least one ofapp.packageNameorapp.bundleId--platformis required unless--appalready disambiguates with.apkor.appRuntime Behavior
check,test, andsuitenow resolve the effective app identity from config first and print the chosen Android package or iOS bundle ID before execution--appremains an install artifact override, not the source of truth for app identitypre_contextlaunch_appactions keep working, but relaunching the known app no longer defaults to uninstall/reinstallWhy
The CLI previously carried app identity in a more complicated nested config shape and depended too heavily on
--appduring execution. This change makes the repo config the stable source of truth, simplifies authoring, keeps env-specific app identity in one obvious place, and starts the planner from a known app-under-test state.Validation
npm run build --workspace=@finalrun/common --workspace=@finalrun/goal-executor --workspace=@finalrun/finalrun-agentnpm test --workspace=@finalrun/finalrun-agentSummary by CodeRabbit
Release Notes
New Features
.finalrun/config.yamlwith environment-specific overrides for Android and iOS app identities.Improvements