Repository navigation
feat: replace arf r-home with arf r resolve - #290
Conversation
`arf r-home` reported a single path, but editors and agent extensions need more than that to start the same R as arf: the binary to spawn, the version, why that version was picked, and any warnings raised on the way. Replace it with `arf r resolve`, which always emits a JSON descriptor. The command groups under a new `arf r` noun, matching `arf config`, `arf history`, and `arf ipc`. Output is pretty-printed on a terminal and compact when piped, so the `--json` flag is gone along with the branching it forced on config loading. The descriptor separates why a version was chosen (`selected_by.kind` and `origin`) from where the installation was found (`provider`), and names the version `resolved_version` rather than `r_version` because it is a prediction made before R starts, unlike the versions the IPC layer reports from a running session. `target` is null exactly when `resolved` is false. Resolution mirrors normal startup rather than being stricter: a config file that fails to load still falls back to defaults and exits zero with a diagnostic, because reporting a different R than arf would actually use defeats the purpose of the command. A missing R installation is likewise a successful query with `resolved: false`, not an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diagnostic codes were derived by substring-matching the human-readable warning text, so rewording a message silently changed a published part of the descriptor. Carry the code on the diagnostic itself instead: `RSourceResolutionReport` now holds structured diagnostics with a code, severity, message, and path, assigned where each warning is raised. The console still prints the messages, and `arf r resolve` no longer inspects them. Invalid invocations now exit 2 rather than 4, matching the code `arf ipc` already documents, and errors are reported in the same shape the IPC client uses so a single parser covers both. Requesting a specific version that is not installed stays an error, because normal startup also refuses to continue in that case. A version that a project file merely suggests is different: startup falls back to `startup.r_source` with a warning, and resolve reports that fallback with `resolved: true` and the warnings attached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A version a project file suggests and a version the user asks for on the command line behave differently when that version is not installed: the first falls back to `startup.r_source` with a warning, the second is an error. Nothing pinned that distinction end to end, so add tests for the fallback path and for the two paths side by side, plus one confirming `ARF_R_VERSION` is treated as an explicit request like the flag is. The fixtures now use raw strings, which matters most for the fake `rig` script where the JSON quoting and the `printf` format were both escaped. They also ask for R 3.99.99 rather than 4.9.9: the 3.x series ended at 3.6.3, so that version can never become real and start passing for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`--with-r-version` was validated against the version grammar before rig was consulted, so a custom rig alias that normal startup accepts was rejected here. Consult rig for aliases, names, and exact versions first, and treat a value as invalid only when it matches none of those and is not a version specification either. Failures were also classified by whether an explicit flag was present, so a malformed `rig list --json` or an R binary that will not run came back as a caller error. Classify by the failure itself instead: bad paths, bad specifications, and uninstalled requested versions are invalid invocations, while process, parsing, and I/O failures are internal. A missing `--r-home` path now exits non-zero, which it did not before despite printing an error. The descriptor kept `selected_by.path` and `requested_version` for overrides only, leaving an explicit selection without the input that explains it, so both are now filled in per `kind`. The fallback tests assumed the host had rig and a discoverable R, and would have failed on a clean machine. They now run against an isolated fake rig and fallback source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`to_string()` on an `anyhow::Error` reports only the outermost context, so
an internal error told the caller that resolving the R version failed
without saying why — the malformed rig output or the R binary that would
not run was dropped. Format with `{error:#}` instead, as the rest of the
crate already does, and assert in the operational-failure tests that the
underlying cause survives into the reported message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the unreleased arf r-home command with a new machine-facing arf r resolve subcommand that always emits a JSON “resolve descriptor” (pretty on TTY, compact when piped), including selection metadata and structured diagnostics.
Changes:
- Introduces
arf r resolveunder a newrnoun, removesr-home, and updates CLI parsing, help, and shell completions accordingly. - Adds a new resolve descriptor format (schema v1) with
resolver,selected_by,provider, and structured diagnostics; updates config-load warnings and R-source override warnings to carry stable codes and paths. - Replaces/extends integration tests to cover resolve behavior, exit codes, and fallback/error scenarios.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/arf-console/tests/resolve_tests.rs | New end-to-end tests for arf r resolve JSON descriptor, diagnostics, and exit codes. |
| crates/arf-console/tests/r_home_tests.rs | Removes r-home tests now that the command is retired. |
| crates/arf-console/tests/cli_tests.rs | Renames a CLI-scope test to reflect updated flag semantics. |
| crates/arf-console/src/snapshots/arf__cli__tests__help_long.snap | Updates long help snapshot to show new r command group. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_zsh.snap | Updates zsh completion snapshot for arf r resolve and nested help. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_powershell.snap | Updates PowerShell completion snapshot for arf r resolve. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_fish.snap | Updates fish completion snapshot for arf r resolve. |
| crates/arf-console/src/snapshots/arf__cli__tests__completions_bash.snap | Updates bash completion snapshot for arf r resolve and nested help. |
| crates/arf-console/src/main.rs | Wires arf r resolve, adds structured JSON error printing and exit-code mapping for resolve failures. |
| crates/arf-console/src/cli/resolve.rs | Adds new r noun and resolve subcommand definitions. |
| crates/arf-console/src/cli/r_home.rs | Removes r-home CLI args type (and --json). |
| crates/arf-console/src/cli/mod.rs | Swaps RHome for R(resolve::RArgs) and updates CLI parsing tests. |
| crates/arf-console/src/cli/completions.rs | Ensures r_version completions are applied to headless and r resolve. |
| crates/arf-console/src/app/setup/rig.rs | Improves rig resolution error wrapping with anyhow::Context. |
| crates/arf-console/src/app/setup/r_source.rs | Introduces RSourceDiagnostic and switches resolution reports to structured diagnostics. |
| crates/arf-console/src/app/setup/overrides.rs | Converts override warnings into structured diagnostics with stable codes and optional paths. |
| crates/arf-console/src/app/setup/mod.rs | Re-exports RSourceDiagnostic alongside existing setup utilities. |
| crates/arf-console/src/app/resolve.rs | Implements arf r resolve descriptor generation, diagnostics mapping, and resolve-specific JSON errors. |
| crates/arf-console/src/app/r_home.rs | Removes old r-home implementation. |
| crates/arf-console/src/app/mod.rs | Replaces r_home module with resolve. |
| crates/arf-console/src/app/headless.rs | Adapts headless JSON warnings collection to the new diagnostic struct. |
| crates/arf-console/src/app/config_load.rs | Adds ConfigLoadWarning and a diagnostic-preserving config-load path for resolve output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`RLessEnvironment` is only compiled off Windows, but one of the two tests using it was missing the matching attribute, so the test binary failed to compile there. The other test already carries it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The descriptor normalized `selected_by.path` against the working directory but printed `target.r_home` as given, so a relative `--r-home` appeared twice in two different forms in the same document. Normalize through one helper so every path in the descriptor is absolute, and derive `r_binary` from the normalized value. Asking for a version with `--with-r-version` when rig is not installed also came back as an internal error, telling clients that arf had a bug when the message was already asking the user to install rig. Give the condition its own `RigError` variant and return it as an invalid invocation before resolution starts, so the exit code matches what the error actually says. Also gate the project-override fixture to unix, since both of its callers already were and it went unused on Windows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Probing rig with `--version` collapsed every failure into "not installed", so a rig that exists but cannot run — no execute permission, a non-zero exit — was reported as a caller error telling the user to install rig. Return the reason instead: only `NotFound` means absent, and anything else is a command failure, which surfaces as an internal error. Callers that genuinely only need a yes/no now discard the reason explicitly, so the next one has to decide rather than inherit the loss. Path normalization also resolved symlinks, which made the descriptor disagree with the path the caller passed on macOS, where the temporary directory sits under a symlinked `/var`. Joining against the working directory is enough to give every path one representation; resolving links was never asked for and changes what the user sees. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Making the rig probe return its reason only helped `arf r resolve`; the explicit rig source, override resolution, and `:switch` still folded every failure back into "rig is not installed". A rig that cannot execute now reports what went wrong instead, and the install hint appears only when rig is genuinely absent. The relative-path test compared against a path built outside the descriptor, which broke on macOS because `current_dir()` resolves the symlinked temporary directory while the test's expectation did not. Check what the test is actually about instead: that `target.r_home` and `selected_by.path` agree, that the path is absolute, and that it matches the descriptor's own `cwd`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`selected_by.kind` spelled out arf's own configuration layers — `r_source_override`, `startup_r_source` — so reading the descriptor meant knowing `arf.toml`, and renaming a config key would have been a breaking API change. Separate what was matched on from where that condition came from instead: `kind` is one of `r_home`, `version_request`, or `default`, and a nested `source` says whether it arrived from a command-line argument, an environment variable, a project file, a configuration file, or a built-in default. `source` carries the specific name — `--r-home`, `ARF_R_VERSION`, the file path and key — so a caller can point the user at the exact thing to change. That replaces `origin`, which called both `arf.toml` and `rproject.toml` "config" and lost the distinction that matters. A command line and an environment variable share a code path here, but they do not share a fix, so they stay apart. Every key is always present, null when it does not apply, matching the rest of the JSON this crate emits. Which arf rule was evaluated, and which candidates failed on the way, stay out of `selected_by` — those are already reported as diagnostics. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reporting whether `startup.r_source` came from a configuration file meant opening that file a second time after resolution had already finished. A file edited or removed in between would make the descriptor disagree with the configuration actually used — the exact divergence this command exists to prevent — and a FIFO would block the second read. Collect the provenance during the first read and carry it alongside the parsed config. Three tests compared descriptor paths against what `tempfile` returned, which fails on macOS because `getcwd` resolves the symlinked temporary directory. That was already fixed once in a single test and came back in new ones, so route every such comparison through one helper that derives the expectation from the descriptor's own `cwd` and explains why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Capturing provenance from a single read deserialized `Config` through `toml::Value`, which drops source spans, so a config with a wrong value type no longer told the user which line to look at. Deserialize `Config` from the file contents directly and parse the same string a second time for provenance: two parses of one string still means one read, so nothing can change underneath and a FIFO still cannot block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
crates/arf-console/src/app/resolve.rs:283
- With
provideralways computed fromresolution.status, unresolved descriptors (target: null) still get a non-null provider value. Ifprovideris meant to describe the found install, it should benullwhenresolvedis false (and only set whentargetis present).
version: env!("CARGO_PKG_VERSION"),
},
selected_by,
provider: provider(&resolution.status),
diagnostics,
crates/arf-console/src/app/resolve.rs:86
provideris documented (in the PR description) as “where the install was found”, but the descriptor currently always emits a non-null string even whenresolvedis false (targetis null). That makesprovidermisleading for unresolved queries and violates the “null when it does not apply” rule used elsewhere in the schema.
Make provider nullable (still always present as a key) so it can be null when unresolved.
This issue also appears on line 279 of the same file.
resolver: Resolver,
selected_by: SelectedBy,
provider: String,
diagnostics: Vec<Diagnostic>,
arf r-homeprinted a single path. Editors and agent extensions need more: the binary to spawn, the version, why that version was picked, and any warnings raised along the way.arf r resolvereplaces it, always emitting a JSON descriptor.It groups under a new
arf rnoun, matchingarf config,arf history, andarf ipc. Output is pretty-printed on a terminal and compact when piped, so--jsonis gone.{ "schema_version": 1, "resolved": true, "cwd": "/project", "target": { "r_home": "/opt/R/4.5.2/lib/R", "r_binary": "/opt/R/4.5.2/lib/R/bin/R", "resolved_version": "4.5.2" }, "resolver": { "name": "arf", "version": "0.4.4" }, "selected_by": { "kind": "version_request", "requested_r_home": null, "requested_version": "4.4", "source": { "kind": "project_file", "name": null, "path": "/project/rproject.toml", "format": "toml", "key": "project.r_version" } }, "provider": "rig", "diagnostics": [] }resolversays which tool produced the descriptor, so the resolution could later move behind a separate command without breaking readers.providersays where the install was found. The version is namedresolved_version, notr_version, because it is a prediction made before R starts — unlike the versions the IPC layer reports from a running session.targetis null exactly whenresolvedis false, and every key is always present, null when it does not apply.selected_byseparates what was matched on from where that condition came from.kindisr_home,version_request, ordefault;sourcesays whether it arrived from a command-line argument, an environment variable, a project file, a configuration file, or a built-in default, and carries the specific name —--r-home,ARF_R_VERSION, the file path and key — so a caller can point the user at the exact thing to change. Deliberately absent are arf's own configuration layer names, which would have made renaming a config key a breaking API change, and the evaluation trace of candidates that failed on the way, which is already reported as diagnostics.Resolution mirrors normal startup rather than being stricter: a config file that fails to load still falls back to defaults and exits zero with a diagnostic, and so does a version merely suggested by a project file. A version requested explicitly on the command line is an error, because startup refuses to continue in that case too — reporting a different R than arf would actually use would defeat the purpose of the command.
Diagnostics carry a stable code, severity, message, and path, assigned where the warning is raised rather than recovered from its text. Codes are an open enum: clients must not reject unknown codes, and
messageis for display only.Exit codes follow
arf ipc: 0 for success (including a query that finds no R at all), 2 for invalid invocation, 4 for internal failure. Errors are JSON on stderr in the shape the IPC client already uses.Probing for rig now reports why it failed, so a rig that exists but cannot run is no longer diagnosed as missing — in
:switchand override resolution as well as here.arf r-homewas never released, so there is nothing to migrate. Docs and changelog follow separately.Test plan
cargo fmt --check,cargo clippy --workspace --all-targets -- -D warnings,cargo test(1079 passed, 0 failed)kind×source.kindcombination checked against real command outputARF_R_VERSION--r-homeappears as one absolute path throughout the descriptor🤖 Generated with Claude Code