Emit JSON output schemas#50
Conversation
`version_matches` stripped range operators and prefix-matched, so `>=22.22.2` behaved as `=22.22.2` and warned on Node 22.22.3/25.9.0. Normalize node-semver grammar (space-AND, `||` unions, hyphen ranges, detached operators, `v` prefixes) into the comma grammar the existing `semver` dependency parses, and delegate evaluation to it. Bare versions keep prefix-at-segment-boundary semantics (a `.nvmrc` `20.11` means 20.11.x, narrower than the crate's caret default); unevaluable input falls back to the historical prefix match.
Unknown --pm/RUNNER_PM (and --runner/RUNNER_RUNNER) values used to be
Debug-dumped verbatim: a PowerShell unquoted `$env:RUNNER_PM=deno`
captures deno's multi-line REPL banner into the variable, and the error
rendered it as \u{1b}-escape soup with no indication which source
supplied it. Now the error names the carrying source, escapes control
chars, truncates past 60 chars, and hints at the captured-command-output
footgun (with the quoted spelling) when the value contains line breaks.
`install_pms` received the overrides but only used them for the GHA log group, so `RUNNER_PM=bun runner install` still ran every detected PM — in a bun+deno repo, `deno install` always ran and wrote a deno.lock nobody asked for. The override now selects the install set: a detected PM installs alone; an undetected one refuses with the new `ResolveError::PmOverrideNotDetected` (exit 2), naming the carrying source and the detected set. The chain path (`runner install <tasks>`) inherits the behavior. The "via <source>" provenance fragment moved to `OverrideOrigin::describe_pm_source` so `--explain` and the install error share one wording.
An invalid env override (e.g. a REPL banner captured by PowerShell unquoted assignment) killed every command at override parsing — before dispatch — including `runner doctor`, the one command whose job is to diagnose a broken environment. Dispatch now retries doctor through a lenient constructor that pre-validates env-sourced values (RUNNER_PM, RUNNER_RUNNER, RUNNER_FALLBACK, RUNNER_ON_MISMATCH), blanking invalid ones into `DetectionWarning::InvalidEnvOverride` rendered on the report. Strict behavior is untouched for all other commands and for CLI flag values; CLI-shadowed env garbage stays invisible, mirroring strict precedence. Env reads are deduped into EnvSnapshot so the two constructors cannot drift.
The PATH probe reported Volta shims (e.g. C:\Program Files\Volta\ npm.EXE) as if they were the tools themselves. New tool::volta module classifies a probe hit as a shim by canonicalized parent-dir equality against the located volta binary dir and $VOLTA_HOME/bin, then asks `volta which <tool>` (cwd = project root, so project pinning applies) for the real binary — classified by exit status and stdout only, never error-text parsing. Doctor renders `npm=<shim> -> <real> (volta)` or `(volta shim, not provisioned)`; JSON gains the additive signals.node.volta_shims map behind a builder flag (doctor/info: on, list and unit tests: off, so no hot path spawns volta). Probe-fallback filtering of unprovisioned phantom shims is deliberately deferred.
The loose prefix fallback never stripped a bare `=` operator and stripped `v` before trimming inner whitespace, so `=20.11` and `>= v18` mis-cleaned when an unparseable `current` forced the fallback path. Trim first, strip operators incl. `=`, then strip the `v` prefix. Also scrub every inherited `RUNNER_*` var (case-insensitive, for Windows) from the doctor_env child processes — a dev box exporting `RUNNER_NO_WARNINGS` or `RUNNER_FALLBACK` could flip assertions.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
runner | 4b81e62 | Commit Preview URL Branch Preview URL |
Jun 12 2026, 10:19 AM |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)📄 CodeRabbit inference engine (Custom checks)
Files:
.github/**/*.{yml,yaml}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (12)📚 Learning: 2026-03-26T16:14:15.754ZApplied to files:
📚 Learning: 2026-03-26T20:08:23.153ZApplied to files:
📚 Learning: 2026-05-04T22:42:28.879ZApplied to files:
📚 Learning: 2026-05-14T15:04:39.496ZApplied to files:
📚 Learning: 2026-05-04T15:23:38.296ZApplied to files:
📚 Learning: 2026-03-26T20:05:44.851ZApplied to files:
📚 Learning: 2026-04-21T15:16:40.277ZApplied to files:
📚 Learning: 2026-05-04T23:28:17.947ZApplied to files:
📚 Learning: 2026-06-01T17:42:48.461ZApplied to files:
📚 Learning: 2026-06-11T18:52:28.233ZApplied to files:
📚 Learning: 2026-05-15T01:31:48.037ZApplied to files:
📚 Learning: 2026-05-14T15:35:39.922ZApplied to files:
🪛 zizmor (1.25.2).github/workflows/pages.yml[warning] 10-10: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false (artipacked) [error] 4-4: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level (excessive-permissions) [error] 4-4: overly broad permissions (excessive-permissions): pages: write is overly broad at the workflow level (excessive-permissions) [error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [error] 11-11: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy) (unpinned-uses) [warning] 4-4: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment (undocumented-permissions) [info] 6-6: workflow or action definition without a name (anonymous-definition): this job (anonymous-definition) [warning] 3-3: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting (concurrency-limits) 🔍 Remote MCP GitHub GrepBased on my search results, I can now provide a summary of the relevant context found: Additional Context FoundCode Implementation PatternsSchema Generation and Writing Patterns:
Schema Versioning and Post-ProcessingThe approach of:
These are consistent with established practices seen in projects tracking schema evolution across versions (e.g., Q00/ouroboros maintains separate v0.2, v0.3, v0.4 schemas with documented changes). JSON Formatting and Deterministic OutputThe use of $schema Field HandlingThe implementation of adding an optional 🛑 Comments failed to post (14)
🔇 Additional comments (26)
📝 WalkthroughAdd JSON output schema generation and emit schemas for all versioned JSON surfaces. This change introduces comprehensive JSON Schema support for the runner's JSON output formats, enabling schema validation and IDE autocomplete for users of the JSON APIs. Key additions:
Tooling and configuration updates:
All schemas follow JSON Schema draft 2020-12 (except v1 variants) and include complete examples and validation constraints for each surface version. WalkthroughThis PR establishes a comprehensive, versioned JSON schema system for the runner's CLI outputs. It replaces a single schema file generation mechanism with multi-version support across Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Dispatch
participant WriteSchema
participant SchemaGen
participant PostProcess
participant IO
User->>CLI: cargo schema --all --output schemas/
CLI->>Dispatch: route Schema subcommand
Dispatch->>WriteSchema: all=true, output path
WriteSchema->>SchemaGen: for each (doctor,list,why) v1/v2
SchemaGen->>SchemaGen: schemars::to_json
SchemaGen->>PostProcess: inject version const<br/>inject TaskSourceLabel
PostProcess->>PostProcess: sort schema deterministically
PostProcess->>IO: write *.schema.json to directory
IO-->>User: schemas/ directory populated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces substantial new schema infrastructure with 14 schema files, significant schema generation logic with JSON mutation helpers, typed model refactoring, and coordinated changes across CLI routing, build tooling, and deployment automation. The schema generation post-processing (version const injection, TaskSourceLabel rewiring) requires careful validation, and the typed JSON model changes impact the Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
✨ Simplify code
|
`DetectionWarning::InvalidEnvOverride::detail()` renders the parse-error `message`, but `lenient_env_field` only sanitized the (unused-by-detail) `raw` field — so a long or secret-looking env value leaked verbatim and untruncated into the warning text. Route the message through a new `sanitize_error_message`, which rewrites the raw value (and its `escape_debug` form) to the truncated `sanitize_raw_label` rendering. Fixes the `lenient_policy_env_garbage_does_not_leak_full_raw_value` test, whose fix was previously stranded one PR up the stack in #50.
… doctor resilience, Volta shims (#45) * fix(types): evaluate node version ranges with real semver semantics `version_matches` stripped range operators and prefix-matched, so `>=22.22.2` behaved as `=22.22.2` and warned on Node 22.22.3/25.9.0. Normalize node-semver grammar (space-AND, `||` unions, hyphen ranges, detached operators, `v` prefixes) into the comma grammar the existing `semver` dependency parses, and delegate evaluation to it. Bare versions keep prefix-at-segment-boundary semantics (a `.nvmrc` `20.11` means 20.11.x, narrower than the crate's caret default); unevaluable input falls back to the historical prefix match. * feat(resolver): self-diagnosing override parse errors Unknown --pm/RUNNER_PM (and --runner/RUNNER_RUNNER) values used to be Debug-dumped verbatim: a PowerShell unquoted `$env:RUNNER_PM=deno` captures deno's multi-line REPL banner into the variable, and the error rendered it as \u{1b}-escape soup with no indication which source supplied it. Now the error names the carrying source, escapes control chars, truncates past 60 chars, and hints at the captured-command-output footgun (with the quoted spelling) when the value contains line breaks. * feat(install): honor --pm/RUNNER_PM override for the install set `install_pms` received the overrides but only used them for the GHA log group, so `RUNNER_PM=bun runner install` still ran every detected PM — in a bun+deno repo, `deno install` always ran and wrote a deno.lock nobody asked for. The override now selects the install set: a detected PM installs alone; an undetected one refuses with the new `ResolveError::PmOverrideNotDetected` (exit 2), naming the carrying source and the detected set. The chain path (`runner install <tasks>`) inherits the behavior. The "via <source>" provenance fragment moved to `OverrideOrigin::describe_pm_source` so `--explain` and the install error share one wording. * feat(doctor): survive unparseable RUNNER_* env overrides An invalid env override (e.g. a REPL banner captured by PowerShell unquoted assignment) killed every command at override parsing — before dispatch — including `runner doctor`, the one command whose job is to diagnose a broken environment. Dispatch now retries doctor through a lenient constructor that pre-validates env-sourced values (RUNNER_PM, RUNNER_RUNNER, RUNNER_FALLBACK, RUNNER_ON_MISMATCH), blanking invalid ones into `DetectionWarning::InvalidEnvOverride` rendered on the report. Strict behavior is untouched for all other commands and for CLI flag values; CLI-shadowed env garbage stays invisible, mirroring strict precedence. Env reads are deduped into EnvSnapshot so the two constructors cannot drift. * feat(doctor): resolve Volta shims to the real provisioned binary The PATH probe reported Volta shims (e.g. C:\Program Files\Volta\ npm.EXE) as if they were the tools themselves. New tool::volta module classifies a probe hit as a shim by canonicalized parent-dir equality against the located volta binary dir and $VOLTA_HOME/bin, then asks `volta which <tool>` (cwd = project root, so project pinning applies) for the real binary — classified by exit status and stdout only, never error-text parsing. Doctor renders `npm=<shim> -> <real> (volta)` or `(volta shim, not provisioned)`; JSON gains the additive signals.node.volta_shims map behind a builder flag (doctor/info: on, list and unit tests: off, so no hot path spawns volta). Probe-fallback filtering of unprovisioned phantom shims is deliberately deferred. * fix(types): strip `=` in version prefix fallback The loose prefix fallback never stripped a bare `=` operator and stripped `v` before trimming inner whitespace, so `=20.11` and `>= v18` mis-cleaned when an unparseable `current` forced the fallback path. Trim first, strip operators incl. `=`, then strip the `v` prefix. Also scrub every inherited `RUNNER_*` var (case-insensitive, for Windows) from the doctor_env child processes — a dev box exporting `RUNNER_NO_WARNINGS` or `RUNNER_FALLBACK` could flip assertions. * test(resolver): cover lenient env warning truncation * fix(resolver): scrub raw env value from warning detail `DetectionWarning::InvalidEnvOverride::detail()` renders the parse-error `message`, but `lenient_env_field` only sanitized the (unused-by-detail) `raw` field — so a long or secret-looking env value leaked verbatim and untruncated into the warning text. Route the message through a new `sanitize_error_message`, which rewrites the raw value (and its `escape_debug` form) to the truncated `sanitize_raw_label` rendering. Fixes the `lenient_policy_env_garbage_does_not_leak_full_raw_value` test, whose fix was previously stranded one PR up the stack in #50.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
3-3:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAhoy! Ye be sailing with a broken compass on this one! 🏴☠️
⚠️ The coding guidelines be crystal clear, matey:
"If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo"
This PR modifies multiple source files (
src/cli.rs,src/lib.rs,src/cmd/why.rs,src/schema/project.rs,src/cmd/schema.rs), and worse yet, it introduces a breaking change to the publicCommandenum by altering theSchemavariant's signature from{ output: Option<PathBuf> }to{ all: bool, output: Option<PathBuf> }.Per SemVer semantics in the guideline: breaking changes to public APIs require a MAJOR version bump. The current version
0.12.2should be bumped to at minimum0.13.0(or1.0.0if ye be ready to declare stability).Batten down the hatches and update that version field before this ship sails!
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Cargo.toml` at line 3, The PR introduces a breaking change to the public API by altering the Command enum's Schema variant signature (Command::Schema from { output: Option<PathBuf> } to { all: bool, output: Option<PathBuf> }) across src/cli.rs, src/lib.rs, src/cmd/why.rs, src/schema/project.rs and src/cmd/schema.rs; update the version field in Cargo.toml from "0.12.2" to a new semver-major release (e.g., "0.13.0" or "1.0.0") to reflect the breaking change and ensure package metadata is consistent with SemVer.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pages.yml:
- Line 10: The checkout step using actions/checkout@v6 currently omits the
persist-credentials setting; update the checkout action invocation (the line
that uses actions/checkout@v6) to include persist-credentials: false to avoid
storing Git credentials in the workspace (and thus leaking them into the Pages
artifact), while keeping the existing sparse-checkout configuration intact.
- Line 3: Add a top-level "concurrency" stanza to this GitHub Actions workflow
so only the latest run touching schemas/** executes; specifically add
concurrency: group: 'pages-schemas-${{ github.ref }}' cancel-in-progress: true
at the root of the workflow (alongside the existing on: definition) so runs
triggered by the on: { push: { branches: [master], paths: [schemas/**] } } event
will cancel any in-progress deploy and ensure only the newest commit's schemas
are deployed.
- Around line 10-12: The workflow is doing a sparse checkout of only
"schemas/*.schema.json" via the actions/checkout step (sparse-checkout) but
later uploads the whole workspace (path: .) while the workflow trigger uses
paths: [schemas/**]; update the checkout/upload to be consistent: either change
the sparse-checkout pattern to "schemas/**" so example files (e.g.,
schemas/doctor.v1.example.json) are included, or keep the narrow sparse-checkout
and change the upload to only publish the schema artifacts (e.g., upload
"schemas/*.schema.json"); alternatively remove sparse-checkout entirely if you
want the full repo—modify the actions/checkout and actions/upload-pages-artifact
usages accordingly.
In @.zed/settings.json:
- Around line 9-23: Add the missing why.v3-draft JSON schema entry to the
settings.json json.schemas array to mirror the doctor.v3-draft mapping: create a
schema mapping with fileMatch entries for "schemas/why.v3-draft.example.json"
and "why.v3-draft.example.json" and set its url to
"./schemas/why.v3-draft.schema.json" so it matches the existing pattern used by
"doctor.v3-draft" and the other v3-v2 mappings in the "schemas" list.
In `@bin/runner`:
- Line 1: The crate version in Cargo.toml wasn't bumped for the new schema
feature; update the package version field in Cargo.toml (the `version =
"0.12.2"` entry) to increment the MINOR version (e.g., `0.13.0`) so the release
reflects backward-compatible new functionality, then commit the change and
include a short commit message referencing the schema feature bump.
In `@rust-toolchain.toml`:
- Line 2: Restore reproducible pinning by setting rust-toolchain.toml to the
MSRV matching Cargo.toml (e.g., change channel = "stable" back to the exact
toolchain that matches rust-version = "1.95", or pin to "1.95.0") so CI and
local tooling use the same fixed toolchain, and update the outdated test comment
in src/tool/node.rs that currently claims rust-toolchain.toml “pins to 1.95” to
reflect the chosen pin (or document the intentional use of latest stable if you
prefer that route); ensure references to Cargo.toml's rust-version and the
comment in src/tool/node.rs are consistent with the chosen policy.
In `@schemas/why.v1.example.json`:
- Around line 4-15: Replace the hard-coded developer path in the example JSON by
using a neutral placeholder or null for the source_dir in the candidates array;
locate the "candidates" object entry that contains "source_dir":
"/home/kjanat/..." and change it to a generic path such as
"/path/to/cargo/config.toml" or set "source_dir" to null (and update the same
pattern at the other occurrence referenced in the comment), ensuring the example
no longer exposes a personal filesystem path.
- Around line 4-27: The example fixtures embed machine-specific absolute paths
in the JSON fields named "source_dir" (seen inside the "candidates" array and
the "selected" object); update those values in schemas/why.v1.example.json (and
the corresponding v2/v3 example files) to use repo-relative or neutral
placeholders (e.g. a repo-relative path like ".cargo/config.toml" or a token
such as "<repo>/.cargo/config.toml") instead of "/home/kjanat/…", ensuring both
the candidate entry and the selected entry are updated consistently.
In `@schemas/why.v1.schema.json`:
- Around line 4-15: The schema/example currently hard-codes a developer path in
the "source_dir" field (e.g., "/home/kjanat/...") inside the PmResolution
example; replace that literal with a neutral placeholder (for example
"<PROJECT_ROOT>/src" or "./path/to/source") so the example does not leak a
machine path and remains stable across regenerations; locate the "source_dir"
property within the PmResolution anyOf/object example and update its
example/default value accordingly.
In `@schemas/why.v2.example.json`:
- Around line 4-15: The example JSON leaks a developer machine path in the
"source_dir" field inside the "candidates" entry; update that value to a neutral
placeholder (e.g., a generic path like "/path/to/config.toml" or null) and do
the same for the other occurrences of "source_dir" in this file (lines
referenced in the review: the later candidate entries) so the example contains
no hard-coded user-specific paths.
In `@schemas/why.v3-draft.example.json`:
- Around line 4-5: The example JSON contains machine-specific absolute paths
(keys like "root", "source", and "cwd") that leak local layout; replace those
values with neutral, platform-independent placeholders or relative paths (e.g.,
"/workspace/project", "./", or "<project_root>") throughout the file
(occurrences around lines 9-23 and 35-49 as well) so the fixture is reusable and
non-machine-specific, ensuring the "query" and other non-path fields remain
unchanged.
In `@src/cmd/schema.rs`:
- Around line 173-202: The hardcoded arrays in source_labels are brittle; add a
compile-time/unit test that asserts the number of entries returned by
source_labels(version) equals the number of TaskSource variants (use
TaskSource::all() or equivalent) for each schema version, and update
source_labels when adding variants; specifically, implement a test referencing
source_labels and TaskSource::all() that fails if counts differ (so CI catches
missing labels), and add a short doc comment by the source_labels function
explaining the maintenance requirement.
- Around line 21-44: Add unit tests covering write_all_schemas and the schema
patching helpers: create tests that (1) assert write_all_schemas returns an
error when given a path that exists but is a file (use a temp file and call
write_all_schemas to trigger the bail!), (2) verify patch_schema_version_const
mutates the schemars output by constructing a schema Value with the expected
const location and asserting the const value is set, (3) verify
patch_source_schema rewrites TaskInfo/WhyCandidate source refs by constructing
representative schema Value and checking the rewritten refs, and (4) implement a
round-trip test that generates the schema via schema_documents()/schema_value(),
writes or patches it, then deserializes the resulting JSON back into the
expected types to assert structural validity; use tempdir/tempfile and the
public functions write_schema, write_all_schemas, patch_schema_version_const,
patch_source_schema, and schema_documents/schema_value to locate code under
test.
- Around line 123-147: Both patch_schema_version_const and patch_source_schema
currently return silently when expected JSON structure is missing, so update
these functions (patch_schema_version_const and patch_source_schema) to emit a
clear warning or debug log when the early-return branches are taken (use
log::warn! or debug! depending on build) including which key was missing (e.g.,
"properties" or "$defs" or "schema_version") and the top-level schema snippet if
helpful; then add unit/integration tests that call these functions with
altered/malformed schemars output to assert the warning is emitted and that the
schema is patched when structure is correct, and add a small note to the project
README or docs specifying the assumed schemars output shape and supported
schemars version so future changes are caught.
---
Outside diff comments:
In `@Cargo.toml`:
- Line 3: The PR introduces a breaking change to the public API by altering the
Command enum's Schema variant signature (Command::Schema from { output:
Option<PathBuf> } to { all: bool, output: Option<PathBuf> }) across src/cli.rs,
src/lib.rs, src/cmd/why.rs, src/schema/project.rs and src/cmd/schema.rs; update
the version field in Cargo.toml from "0.12.2" to a new semver-major release
(e.g., "0.13.0" or "1.0.0") to reflect the breaking change and ensure package
metadata is consistent with SemVer.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cde7a7d4-6211-4dfd-8af3-0d74448ac792
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.dprint.jsonc.github/workflows/pages.yml.zed/settings.jsonCargo.tomlbin/.runner-devbin/runbin/runbin/runnerbin/runnerjustfilenpm/targets.schema.jsonpackage.jsonrust-toolchain.tomlrustfmt.tomlschemas/doctor.v1.example.jsonschemas/doctor.v1.schema.jsonschemas/doctor.v2.example.jsonschemas/doctor.v2.schema.jsonschemas/doctor.v3-draft.schema.jsonschemas/list.v1.example.jsonschemas/list.v1.schema.jsonschemas/list.v2.example.jsonschemas/list.v2.schema.jsonschemas/runner.toml.schema.jsonschemas/why.v1.example.jsonschemas/why.v1.schema.jsonschemas/why.v2.example.jsonschemas/why.v2.schema.jsonschemas/why.v3-draft.example.jsonsrc/cli.rssrc/cmd/schema.rssrc/cmd/why.rssrc/lib.rssrc/schema/project.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION)
📄 CodeRabbit inference engine (Custom checks)
@(package.json|pyproject.toml|setup.py|Cargo.toml|go.mod|pom.xml|build.gradle|VERSION): If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo: package.json, pyproject.toml, setup.py, Cargo.toml, go.mod, pom.xml, build.gradle, or a VERSION file.
The new version MUST follow SemVer (MAJOR.MINOR.PATCH). If the PR introduces breaking changes (removal or renaming of public APIs, changes to function signatures, deleted exported symbols, or incompatible config changes), MAJOR must increment. If it adds backward-compatible functionality, MINOR must increment. If it only fixes bugs without changing public APIs, PATCH must increment.
Files:
Cargo.tomlpackage.json
.github/**/*.{yml,yaml}
⚙️ CodeRabbit configuration file
Do not ever warn about stylistic yamllint shit. Do warn about security related shit. Insecure shell handling of user supplied / defined strings: think of branch names, inputs, pr content, anything needs to be string interpolation and permission safe.
Files:
.github/workflows/pages.yml
🧠 Learnings (12)
📚 Learning: 2026-03-26T16:14:15.754Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/tool/go_task.rs:64-81
Timestamp: 2026-03-26T16:14:15.754Z
Learning: If code uses let-chains (e.g., `if let Some(x) = foo && ... && let Some(y) = bar`), ensure the crate’s `Cargo.toml` sets `package.edition = "2024"`. Rust 2021/earlier should not be used with let-chains; require 2024 specifically for compilation.
Applied to files:
Cargo.toml
📚 Learning: 2026-03-26T20:08:23.153Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/detect.rs:22-22
Timestamp: 2026-03-26T20:08:23.153Z
Learning: In `src/detect.rs` within the `kjanat/runner` codebase, `detect()` is intentionally infallible and should return `ProjectContext` directly. Parsing/I/O failures from task extractors (which return `anyhow::Result<Vec<String>>`) must be recorded by `push_extracted_tasks` into `ProjectContext.warnings` as `DetectionWarning` entries and then emitted to stderr via `cmd::print_warnings`. When reviewing, do not flag `detect()` for “swallowing errors” as long as failures are routed into the warnings-list design (i.e., they’re preserved in `ProjectContext.warnings`, not dropped silently).
Applied to files:
.dprint.jsonc
📚 Learning: 2026-05-04T22:42:28.879Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: CHANGELOG.md:8-27
Timestamp: 2026-05-04T22:42:28.879Z
Learning: In `CHANGELOG.md`, do not treat a `## [Unreleased]` section as an error/mismatch during development PRs even if `Cargo.toml` includes an in-progress version bump. The versioned header format `## [X.Y.Z] - YYYY-MM-DD` should only be added at release/tag time; this repo’s `CHANGELOG.md` includes a “Post-release checklist” confirming that behavior. Only flag changelog/version mismatches if they contradict the intended workflow described in `CHANGELOG.md`.
Applied to files:
.dprint.jsonc
📚 Learning: 2026-05-14T15:04:39.496Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/cli.rs:780-788
Timestamp: 2026-05-14T15:04:39.496Z
Learning: In `src/cli.rs`, do not recommend removing `trailing_var_arg = true` or `allow_hyphen_values = true` from `Command::Run.args` and `RunAliasCli.args`. They are intentionally required for the documented pass-through contract: hyphen-prefixed args (e.g., `--watch`) that appear after a task name must be forwarded to the underlying task. As a result, in chain mode, chain-failure flags like `-k`/`--kill-on-fail` that are placed after task names will be silently consumed as task args rather than parsed as flags; when proposing fixes or usage changes, use the documented workaround of placing all chain flags before task names (e.g., `runner run -s -k build test`). This behavior is verified in `src/lib.rs` (`run_alias_forwards_trailing_args`) and described in the README, so keep this design trade-off intact.
Applied to files:
.dprint.jsonc
📚 Learning: 2026-05-04T15:23:38.296Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: .github/workflows/release.yml:73-73
Timestamp: 2026-05-04T15:23:38.296Z
Learning: For GitHub Actions workflows in this repository (kjanat/runner), maintain the maintainer’s preference for “floating” action tags (e.g., `v6`, `v7`) instead of pinned full commit SHAs. During code review, do not flag action usages in `.github/workflows/**/*.yml` for not being pinned to full commit SHAs or suggest switching to SHA pinning.
Applied to files:
.github/workflows/pages.yml
📚 Learning: 2026-03-26T20:05:44.851Z
Learnt from: kjanat
Repo: kjanat/runner PR: 1
File: src/cmd/mod.rs:67-75
Timestamp: 2026-03-26T20:05:44.851Z
Learning: In Rust, `std::process::Command` does not provide getters for stdio configuration, so whether `Stdio::inherit()` (or other `Stdio::*` settings) was applied cannot be asserted via pure unit tests without spawning a process and inspecting OS-level fds. When reviewing Rust code, do not flag “missing unit test coverage” for stdio configuration on `std::process::Command` as a code issue—treat the explicit `Command::stdin/stdout/stderr` setter calls in the source as the meaningful guarantee.
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-04-21T15:16:40.277Z
Learnt from: kjanat
Repo: kjanat/runner PR: 4
File: src/tool/just.rs:132-133
Timestamp: 2026-04-21T15:16:40.277Z
Learning: In Rust, if a method like `ExtractedTask::name() -> &str` returns a borrowed `&str` tied to `&self`, then using `sort_unstable_by_key(|t| t.name())` should be avoided because `by_key` requires the key to not borrow from the element being sorted (it will fail to compile due to the key’s lifetime). Do not recommend `sort_unstable_by_key` as a simplification in this situation. If you need an allocation-free and idiomatic comparison, use `sort_unstable_by(|a, b| a.name().cmp(b.name()))` instead. Using `.to_owned()` inside `by_key` is an alternative but allocates a `String` per element.
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-05-04T23:28:17.947Z
Learnt from: kjanat
Repo: kjanat/runner PR: 5
File: src/lib.rs:222-223
Timestamp: 2026-05-04T23:28:17.947Z
Learning: When reviewing Rust `rustdoc` comments (`/// ...`), treat a trailing backslash (`\`) at the end of a comment line as valid CommonMark syntax for a hard line break (rendered as `<br>`). Do not flag such trailing backslashes as papercuts or recommend removing them unless there is clear evidence they are unintended (e.g., they are not in the `rustdoc` comment context or the surrounding formatting contradicts the intended CommonMark hard-break usage).
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-06-01T17:42:48.461Z
Learnt from: kjanat
Repo: kjanat/runner PR: 34
File: Cargo.toml:61-61
Timestamp: 2026-06-01T17:42:48.461Z
Learning: In this repo, do not flag `actions_rs::log::GroupGuard` or `actions_rs::log::group_guard` as missing/non-existent when they are referenced in Rust code, because the `actions-rs` crate (dependency `actions-rs = "0.1"` / published v0.1.x) exports `pub struct GroupGuard` and `pub fn group_guard` from `actions_rs::log` (and also exports `actions_rs::env::is_github_actions`). This avoids false positives from outdated/incorrect web search results. If the repo does not depend on `actions-rs = "0.1"` in `Cargo.toml`, then normal missing-import/item checks can apply.
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-06-11T18:52:28.233Z
Learnt from: kjanat
Repo: kjanat/runner PR: 45
File: src/lib.rs:659-661
Timestamp: 2026-06-11T18:52:28.233Z
Learning: In Rust, it’s acceptable to use `matches!(cli.command, Some(cli::Command::Doctor { .. }))` (or similar) when the scrutinee comes from a field accessed through a shared reference (e.g., `cli: &Cli`). If the match pattern uses `..` and binds zero fields (so only the enum discriminant is matched), Rust match ergonomics will avoid moving out of the referenced value. Reviewers should not flag such code as a compile-blocking move; confirm with `cargo check` rather than forcing changes like `.as_ref()` when compilation succeeds.
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-05-15T01:31:48.037Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/types.rs:584-585
Timestamp: 2026-05-15T01:31:48.037Z
Learning: In the `kjanat/runner` Rust codebase, remember that `just` treats its `justfile` name as officially case-insensitive and allows the hidden variant: `justfile`, `Justfile`, `JUSTFILE`, etc., and `.justfile`. `TaskSource::Justfile` detection should recognize these filename variants. Also note that `from_label` in `src/types.rs` is for parsing user-supplied qualifier prefixes (e.g. `justfile:build`), not for detecting the on-disk justfile. If you add/adjust qualifier label emission for `TaskSource::Justfile`, it’s not strictly required for backward compatibility because `TaskSource::label()` previously only emitted lowercase `"justfile"`, but adding additional casing/hidden variants as a defensive UX improvement is consistent with `just`’s official support.
Applied to files:
src/lib.rssrc/cli.rssrc/cmd/why.rssrc/schema/project.rssrc/cmd/schema.rs
📚 Learning: 2026-05-14T15:35:39.922Z
Learnt from: kjanat
Repo: kjanat/runner PR: 27
File: src/cmd/run.rs:206-212
Timestamp: 2026-05-14T15:35:39.922Z
Learning: For Go projects using the canonical `cmd/` layout, when `runner` dispatches a local Go tool by its bare name (e.g., `some-cli`) under `PackageManager::Go`, resolve it relative to `ctx.root` by probing `ctx.root/cmd/<task_name>/`. If it exists, run the tool with the filesystem-path form: `go run ./cmd/<task_name>` (note the leading `./`). Do NOT generate `go run <task_name>` because bare names are treated as Go import paths, not local directories. While `go run <module>/cmd/<task_name>` can work, it’s unnecessary for local development—prefer `./cmd/<task_name>`.
Applied to files:
src/cmd/why.rssrc/cmd/schema.rs
🪛 zizmor (1.25.2)
.github/workflows/pages.yml
[warning] 10-10: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 4-4: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
[error] 4-4: overly broad permissions (excessive-permissions): pages: write is overly broad at the workflow level
(excessive-permissions)
[error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 11-11: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 13-13: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[warning] 4-4: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[info] 6-6: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
[warning] 3-3: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🔍 Remote MCP GitHub Grep
Based on my search results, I can now provide a summary of the relevant context found:
Additional Context Found
Code Implementation Patterns
Schema Generation and Writing Patterns:
The PR follows well-established patterns found in other Rust projects for JSON schema generation and output:
-
Schema Writing Implementation: The approach of using
serde_json::to_string_pretty()for pretty-printing and writing schemas to files is consistent with patterns across multiple projects (Zed, Tauri, Turborepo, Pixi). -
Feature-Gated Schema Derivation: The use of
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]is a standard pattern for conditionally including schemars in Rust projects, as seen in Rome Tools, Polars, OpenTelemetry, and other production codebases. This allows the schema generation code to be optional and not impact runtime binaries when the feature is disabled. -
Directory vs. File Output Handling: The pattern of checking whether the output path is a directory (when
--allis set) versus treating it as a file path is a conventional approach for flexible CLI schema output, similar to how Turborepo and Pixi handle schema generation.
Schema Versioning and Post-Processing
The approach of:
- Using
schema_versionas aconstfield in the JSON schema ensures version integrity - Post-processing generated schemas to patch version-specific enums like
TaskSourceLabel - Maintaining separate example files per version (v1, v2, v3-draft)
These are consistent with established practices seen in projects tracking schema evolution across versions (e.g., Q00/ouroboros maintains separate v0.2, v0.3, v0.4 schemas with documented changes).
JSON Formatting and Deterministic Output
The use of json-schema-sort dependency (version 0.1) to ensure deterministic schema output before writing is a pragmatic approach to avoid spurious diffs when schemas are regenerated—a pattern similar to how other schema-generating projects handle output consistency.
$schema Field Handling
The implementation of adding an optional $schema field that is "omitted when empty" (using #[serde(skip_serializing_if)]) is a clean approach to providing schema metadata without introducing required fields for backward compatibility.
🔇 Additional comments (26)
bin/runner (1)
1-1: 🏴☠️ bin/runner ain’t busted—it's a symlink to a real executable; semver bump still to check
- Good news, matey:
bin/runneris a symlink to.runner-dev, and.runner-devis an executable Bash script (has#!/usr/bin/env bash);./bin/runner --helpruns fine—so scrap the “plain-text file won’t execute” panic.- Ya still gotta confirm the semver MINOR bump for the new schema generation feature: this workspace shows no diffs to
Cargo.toml/lockfiles, so check the actual PR version updates across all relevant manifests (Cargo.toml,package.json,pyproject.toml, etc.).src/cli.rs (1)
994-1009: ⚡ Quick winArrr, smooth sailin' on the CLI expansion, matey! ⚓
The
--allflag and PATH semantics be shipshape. The help text clearly signals the broadened scope from single-file to multi-schema emission. No quarrel with the implementation itself.src/lib.rs (2)
768-768: LGTM!
792-794: LGTM!src/cmd/why.rs (3)
89-142: ⚡ Quick winNyaa~! This typed refactoring is chef's kiss ✨
Replacing those messy untyped
json!macros with properSerialize-derived structs? Absolutely blessed. The#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]pattern is chef's tier feature-gating. And that#[serde(untagged)]onPmResolutionfor discriminated union vibes? Mwah!The type safety here prevents SO many runtime serialisation whoopsies. Love love love! 💖
163-234: LGTM!
452-452: LGTM!src/schema/project.rs (2)
32-38: ⚡ Quick winArrr, the
$schemafield addition be textbook perfect! 🏴☠️The
skip_serializing_if = "str::is_empty"pattern means ye won't pollute output with empty strings when the field ain't needed, and therenameattribute keeps the JSON clean with that proper$schemakey. Initialised to empty string at lines 135 and 184, which be correct since it'll be stripped during serialisation.Smart as a whip, this implementation!
Also applies to: 197-203
29-29: LGTM!Also applies to: 194-194, 219-219, 250-250, 261-261, 314-314, 324-324, 334-334, 344-344, 361-361, 371-371, 386-386, 397-397, 416-416, 439-439
src/cmd/schema.rs (1)
104-115: ⚡ Quick winThe deterministic sorting be a smart touch! 🎯
Using
json_schema_sort::sort_schemabefore pretty-printing ensures the committed schema files don't get spurious diffs every time they're regenerated. That's the kind of attention to detail that keeps git history clean.And that trailing newline on line 111? Perfection. Keeps the POSIX text file police happy.
justfile (1)
11-11: LGTM!Also applies to: 30-35
schemas/doctor.v1.schema.json (1)
1-472: ⚡ Quick winGenerated schema artifact—trust the generator! 🤖
This be a generated file, so hand-reviewing every line would be a fool's errand. The structure looks ship-shape:
$idpoints to the GitHub Pages URL (line 2)schema_versionhasconst: 1enforced (line 448)TaskSourceLabelenum has v1-style filename labels (lines 354-367)The real validation happens via the drift guard mentioned in
justfile:just gen-schema && git diff --exit-code schemas/If the generator changes, that guard'll catch it. Approve this artifact on faith in the generator code.
Cargo.toml (1)
72-75: Arr, matey—your Cargo wiring be shipshape:schemabuilds fine 📦
cargo check --features schemapulls injson-schema-sort v0.1.0successfully, so theoptional = true/default-features = falsesetup and feature hookup are doing the right thing. The crates.iomax_versionquery spat outnull(field mismatch), so don’t use that exact JSON path for version evidence.schemas/doctor.v1.example.json (1)
1-234: LGTM!schemas/doctor.v2.schema.json (1)
1-471: LGTM!schemas/doctor.v2.example.json (1)
1-234: LGTM!schemas/doctor.v3-draft.schema.json (1)
1-880: LGTM!schemas/list.v1.schema.json (1)
1-92: LGTM!schemas/list.v1.example.json (1)
1-173: LGTM!schemas/list.v2.schema.json (1)
1-92: LGTM!schemas/list.v2.example.json (1)
1-173: LGTM!bin/.runner-dev (1)
1-51: LGTM!bin/run (1)
1-1: LGTM!.dprint.jsonc (1)
1-52: LGTM!package.json (1)
11-11: LGTM!rustfmt.toml (1)
1-1: Setrustfmt.tomltoedition = 2024—don’t trust it blindly, ye doofus pirate.
Cargo.tomlalready usesedition = "2024", and the toolchain here (rustc 1.96.0) supports it—yetrustfmtisn’t available to runrustfmt --edition 2024 --checkin this environment, so you still need to run your realcargo fmt/CI formatting check to ensure no unexpected diffs.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
3-3:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAhoy! Ye be sailing with a broken compass on this one! 🏴☠️
⚠️ The coding guidelines be crystal clear, matey:
"If any source code files (excluding tests, docs, CI, markdown, or comments-only changes) are modified, then a version field MUST be updated in one of the following files if present in the repo"
This PR modifies multiple source files (
src/cli.rs,src/lib.rs,src/cmd/why.rs,src/schema/project.rs,src/cmd/schema.rs), and worse yet, it introduces a breaking change to the publicCommandenum by altering theSchemavariant's signature from{ output: Option<PathBuf> }to{ all: bool, output: Option<PathBuf> }.Per SemVer semantics in the guideline: breaking changes to public APIs require a MAJOR version bump. The current version
0.12.2should be bumped to at minimum0.13.0(or1.0.0if ye be ready to declare stability).Batten down the hatches and update that version field before this ship sails!
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Cargo.toml` at line 3, The PR introduces a breaking change to the public API by altering the Command enum's Schema variant signature (Command::Schema from { output: Option<PathBuf> } to { all: bool, output: Option<PathBuf> }) across src/cli.rs, src/lib.rs, src/cmd/why.rs, src/schema/project.rs and src/cmd/schema.rs; update the version field in Cargo.toml from "0.12.2" to a new semver-major release (e.g., "0.13.0" or "1.0.0") to reflect the breaking change and ensure package metadata is consistent with SemVer.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pages.yml:
- Line 10: The checkout step using actions/checkout@v6 currently omits the
persist-credentials setting; update the checkout action invocation (the line
that uses actions/checkout@v6) to include persist-credentials: false to avoid
storing Git credentials in the workspace (and thus leaking them into the Pages
artifact), while keeping the existing sparse-checkout configuration intact.
- Line 3: Add a top-level "concurrency" stanza to this GitHub Actions workflow
so only the latest run touching schemas/** executes; specifically add
concurrency: group: 'pages-schemas-${{ github.ref }}' cancel-in-progress: true
at the root of the workflow (alongside the existing on: definition) so runs
triggered by the on: { push: { branches: [master], paths: [schemas/**] } } event
will cancel any in-progress deploy and ensure only the newest commit's schemas
are deployed.
- Around line 10-12: The workflow is doing a sparse checkout of only
"schemas/*.schema.json" via the actions/checkout step (sparse-checkout) but
later uploads the whole workspace (path: .) while the workflow trigger uses
paths: [schemas/**]; update the checkout/upload to be consistent: either change
the sparse-checkout pattern to "schemas/**" so example files (e.g.,
schemas/doctor.v1.example.json) are included, or keep the narrow sparse-checkout
and change the upload to only publish the schema artifacts (e.g., upload
"schemas/*.schema.json"); alternatively remove sparse-checkout entirely if you
want the full repo—modify the actions/checkout and actions/upload-pages-artifact
usages accordingly.
In @.zed/settings.json:
- Around line 9-23: Add the missing why.v3-draft JSON schema entry to the
settings.json json.schemas array to mirror the doctor.v3-draft mapping: create a
schema mapping with fileMatch entries for "schemas/why.v3-draft.example.json"
and "why.v3-draft.example.json" and set its url to
"./schemas/why.v3-draft.schema.json" so it matches the existing pattern used by
"doctor.v3-draft" and the other v3-v2 mappings in the "schemas" list.
In `@bin/runner`:
- Line 1: The crate version in Cargo.toml wasn't bumped for the new schema
feature; update the package version field in Cargo.toml (the `version =
"0.12.2"` entry) to increment the MINOR version (e.g., `0.13.0`) so the release
reflects backward-compatible new functionality, then commit the change and
include a short commit message referencing the schema feature bump.
In `@rust-toolchain.toml`:
- Line 2: Restore reproducible pinning by setting rust-toolchain.toml to the
MSRV matching Cargo.toml (e.g., change channel = "stable" back to the exact
toolchain that matches rust-version = "1.95", or pin to "1.95.0") so CI and
local tooling use the same fixed toolchain, and update the outdated test comment
in src/tool/node.rs that currently claims rust-toolchain.toml “pins to 1.95” to
reflect the chosen pin (or document the intentional use of latest stable if you
prefer that route); ensure references to Cargo.toml's rust-version and the
comment in src/tool/node.rs are consistent with the chosen policy.
In `@schemas/why.v1.example.json`:
- Around line 4-15: Replace the hard-coded developer path in the example JSON by
using a neutral placeholder or null for the source_dir in the candidates array;
locate the "candidates" object entry that contains "source_dir":
"/home/kjanat/..." and change it to a generic path such as
"/path/to/cargo/config.toml" or set "source_dir" to null (and update the same
pattern at the other occurrence referenced in the comment), ensuring the example
no longer exposes a personal filesystem path.
- Around line 4-27: The example fixtures embed machine-specific absolute paths
in the JSON fields named "source_dir" (seen inside the "candidates" array and
the "selected" object); update those values in schemas/why.v1.example.json (and
the corresponding v2/v3 example files) to use repo-relative or neutral
placeholders (e.g. a repo-relative path like ".cargo/config.toml" or a token
such as "<repo>/.cargo/config.toml") instead of "/home/kjanat/…", ensuring both
the candidate entry and the selected entry are updated consistently.
In `@schemas/why.v1.schema.json`:
- Around line 4-15: The schema/example currently hard-codes a developer path in
the "source_dir" field (e.g., "/home/kjanat/...") inside the PmResolution
example; replace that literal with a neutral placeholder (for example
"<PROJECT_ROOT>/src" or "./path/to/source") so the example does not leak a
machine path and remains stable across regenerations; locate the "source_dir"
property within the PmResolution anyOf/object example and update its
example/default value accordingly.
In `@schemas/why.v2.example.json`:
- Around line 4-15: The example JSON leaks a developer machine path in the
"source_dir" field inside the "candidates" entry; update that value to a neutral
placeholder (e.g., a generic path like "/path/to/config.toml" or null) and do
the same for the other occurrences of "source_dir" in this file (lines
referenced in the review: the later candidate entries) so the example contains
no hard-coded user-specific paths.
In `@schemas/why.v3-draft.example.json`:
- Around line 4-5: The example JSON contains machine-specific absolute paths
(keys like "root", "source", and "cwd") that leak local layout; replace those
values with neutral, platform-independent placeholders or relative paths (e.g.,
"/workspace/project", "./", or "<project_root>") throughout the file
(occurrences around lines 9-23 and 35-49 as well) so the fixture is reusable and
non-machine-specific, ensuring the "query" and other non-path fields remain
unchanged.
In `@src/cmd/schema.rs`:
- Around line 173-202: The hardcoded arrays in source_labels are brittle; add a
compile-time/unit test that asserts the number of entries returned by
source_labels(version) equals the number of TaskSource variants (use
TaskSource::all() or equivalent) for each schema version, and update
source_labels when adding variants; specifically, implement a test referencing
source_labels and TaskSource::all() that fails if counts differ (so CI catches
missing labels), and add a short doc comment by the source_labels function
explaining the maintenance requirement.
- Around line 21-44: Add unit tests covering write_all_schemas and the schema
patching helpers: create tests that (1) assert write_all_schemas returns an
error when given a path that exists but is a file (use a temp file and call
write_all_schemas to trigger the bail!), (2) verify patch_schema_version_const
mutates the schemars output by constructing a schema Value with the expected
const location and asserting the const value is set, (3) verify
patch_source_schema rewrites TaskInfo/WhyCandidate source refs by constructing
representative schema Value and checking the rewritten refs, and (4) implement a
round-trip test that generates the schema via schema_documents()/schema_value(),
writes or patches it, then deserializes the resulting JSON back into the
expected types to assert structural validity; use tempdir/tempfile and the
public functions write_schema, write_all_schemas, patch_schema_version_const,
patch_source_schema, and schema_documents/schema_value to locate code under
test.
- Around line 123-147: Both patch_schema_version_const and patch_source_schema
currently return silently when expected JSON structure is missing, so update
these functions (patch_schema_version_const and patch_source_schema) to emit a
clear warning or debug log when the early-return branches are taken (use
log::warn! or debug! depending on build) including which key was missing (e.g.,
"properties" or "$defs" or "schema_version") and the top-level schema snippet if
helpful; then add unit/integration tests that call these functions with
altered/malformed schemars output to assert the warning is emitted and that the
schema is patched when structure is correct, and add a small note to the project
README or docs specifying the assumed schemars output shape and supported
schemars version so future changes are caught.
---
Outside diff comments:
In `@Cargo.toml`:
- Line 3: The PR introduces a breaking change to the public API by altering the
Command enum's Schema variant signature (Command::Schema from { output:
Option<PathBuf> } to { all: bool, output: Option<PathBuf> }) across src/cli.rs,
src/lib.rs, src/cmd/why.rs, src/schema/project.rs and src/cmd/schema.rs; update
the version field in Cargo.toml from "0.12.2" to a new semver-major release
(e.g., "0.13.0" or "1.0.0") to reflect the breaking change and ensure package
metadata is consistent with SemVer.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cde7a7d4-6211-4dfd-8af3-0d74448ac792
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
.dprint.jsonc.github/workflows/pages.yml.zed/settings.jsonCargo.tomlbin/.runner-devbin/runbin/runbin/runnerbin/runnerjustfilenpm/targets.schema.jsonpackage.jsonrust-toolchain.tomlrustfmt.tomlschemas/doctor.v1.example.jsonschemas/doctor.v1.schema.jsonschemas/doctor.v2.example.jsonschemas/doctor.v2.schema.jsonschemas/doctor.v3-draft.schema.jsonschemas/list.v1.example.jsonschemas/list.v1.schema.jsonschemas/list.v2.example.jsonschemas/list.v2.schema.jsonschemas/runner.toml.schema.jsonschemas/why.v1.example.jsonschemas/why.v1.schema.jsonschemas/why.v2.example.jsonschemas/why.v2.schema.jsonschemas/why.v3-draft.example.jsonsrc/cli.rssrc/cmd/schema.rssrc/cmd/why.rssrc/lib.rssrc/schema/project.rs
📜 Review details
🔇 Additional comments (26)
bin/runner (1)
1-1: 🏴☠️ bin/runner ain’t busted—it's a symlink to a real executable; semver bump still to check
- Good news, matey:
bin/runneris a symlink to.runner-dev, and.runner-devis an executable Bash script (has#!/usr/bin/env bash);./bin/runner --helpruns fine—so scrap the “plain-text file won’t execute” panic.- Ya still gotta confirm the semver MINOR bump for the new schema generation feature: this workspace shows no diffs to
Cargo.toml/lockfiles, so check the actual PR version updates across all relevant manifests (Cargo.toml,package.json,pyproject.toml, etc.).src/cli.rs (1)
994-1009: ⚡ Quick winArrr, smooth sailin' on the CLI expansion, matey! ⚓
The
--allflag and PATH semantics be shipshape. The help text clearly signals the broadened scope from single-file to multi-schema emission. No quarrel with the implementation itself.src/lib.rs (2)
768-768: LGTM!
792-794: LGTM!src/cmd/why.rs (3)
89-142: ⚡ Quick winNyaa~! This typed refactoring is chef's kiss ✨
Replacing those messy untyped
json!macros with properSerialize-derived structs? Absolutely blessed. The#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]pattern is chef's tier feature-gating. And that#[serde(untagged)]onPmResolutionfor discriminated union vibes? Mwah!The type safety here prevents SO many runtime serialisation whoopsies. Love love love! 💖
163-234: LGTM!
452-452: LGTM!src/schema/project.rs (2)
32-38: ⚡ Quick winArrr, the
$schemafield addition be textbook perfect! 🏴☠️The
skip_serializing_if = "str::is_empty"pattern means ye won't pollute output with empty strings when the field ain't needed, and therenameattribute keeps the JSON clean with that proper$schemakey. Initialised to empty string at lines 135 and 184, which be correct since it'll be stripped during serialisation.Smart as a whip, this implementation!
Also applies to: 197-203
29-29: LGTM!Also applies to: 194-194, 219-219, 250-250, 261-261, 314-314, 324-324, 334-334, 344-344, 361-361, 371-371, 386-386, 397-397, 416-416, 439-439
src/cmd/schema.rs (1)
104-115: ⚡ Quick winThe deterministic sorting be a smart touch! 🎯
Using
json_schema_sort::sort_schemabefore pretty-printing ensures the committed schema files don't get spurious diffs every time they're regenerated. That's the kind of attention to detail that keeps git history clean.And that trailing newline on line 111? Perfection. Keeps the POSIX text file police happy.
justfile (1)
11-11: LGTM!Also applies to: 30-35
schemas/doctor.v1.schema.json (1)
1-472: ⚡ Quick winGenerated schema artifact—trust the generator! 🤖
This be a generated file, so hand-reviewing every line would be a fool's errand. The structure looks ship-shape:
$idpoints to the GitHub Pages URL (line 2)schema_versionhasconst: 1enforced (line 448)TaskSourceLabelenum has v1-style filename labels (lines 354-367)The real validation happens via the drift guard mentioned in
justfile:just gen-schema && git diff --exit-code schemas/If the generator changes, that guard'll catch it. Approve this artifact on faith in the generator code.
Cargo.toml (1)
72-75: Arr, matey—your Cargo wiring be shipshape:schemabuilds fine 📦
cargo check --features schemapulls injson-schema-sort v0.1.0successfully, so theoptional = true/default-features = falsesetup and feature hookup are doing the right thing. The crates.iomax_versionquery spat outnull(field mismatch), so don’t use that exact JSON path for version evidence.schemas/doctor.v1.example.json (1)
1-234: LGTM!schemas/doctor.v2.schema.json (1)
1-471: LGTM!schemas/doctor.v2.example.json (1)
1-234: LGTM!schemas/doctor.v3-draft.schema.json (1)
1-880: LGTM!schemas/list.v1.schema.json (1)
1-92: LGTM!schemas/list.v1.example.json (1)
1-173: LGTM!schemas/list.v2.schema.json (1)
1-92: LGTM!schemas/list.v2.example.json (1)
1-173: LGTM!bin/.runner-dev (1)
1-51: LGTM!bin/run (1)
1-1: LGTM!.dprint.jsonc (1)
1-52: LGTM!package.json (1)
11-11: LGTM!rustfmt.toml (1)
1-1: Setrustfmt.tomltoedition = 2024—don’t trust it blindly, ye doofus pirate.
Cargo.tomlalready usesedition = "2024", and the toolchain here (rustc 1.96.0) supports it—yetrustfmtisn’t available to runrustfmt --edition 2024 --checkin this environment, so you still need to run your realcargo fmt/CI formatting check to ensure no unexpected diffs.
🛑 Comments failed to post (14)
.github/workflows/pages.yml (3)
3-3:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winAhoy! Ye be missin' concurrency controls, ye scallywag! ⚓
If two commits be pushed to
masterin quick succession, both touchingschemas/**, ye'll have two workflow runs racin' to deploy. The second might finish before the first, then the first overwrites it with stale schemas! Yer users be gettin' old schemas served from Pages!Add a concurrency group to ensure only the latest commit's schemas get deployed, cancelling any in-progress runs.
🏴☠️ Proposed fix to add concurrency control
--- name: Upload schemas to GitHub Pages on: { push: { branches: [master], paths: [schemas/**] } } +concurrency: { group: "pages", cancel-in-progress: true } permissions: { contents: read, id-token: write, pages: write }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.--- name: Upload schemas to GitHub Pages on: { push: { branches: [master], paths: [schemas/**] } } concurrency: { group: "pages", cancel-in-progress: true } permissions: { contents: read, id-token: write, pages: write }🧰 Tools
🪛 zizmor (1.25.2)
[warning] 3-3: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pages.yml at line 3, Add a top-level "concurrency" stanza to this GitHub Actions workflow so only the latest run touching schemas/** executes; specifically add concurrency: group: 'pages-schemas-${{ github.ref }}' cancel-in-progress: true at the root of the workflow (alongside the existing on: definition) so runs triggered by the on: { push: { branches: [master], paths: [schemas/**] } } event will cancel any in-progress deploy and ensure only the newest commit's schemas are deployed.
10-10:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winKyaa~! ✨ Senpai, you forgot something important! (๑•́ ω •̀๑)
The checkout action doesn't set
persist-credentials: false, which means Git credentials might leak into the Pages artifact through the.git/configfile! That's... that's really dangerous, baka! (⊙_⊙;)Even though we're only publishing public schema files, we should follow security best practices, ne~?
💝 Proposed fix to prevent credential leakage
- { uses: actions/checkout@v6, with: { sparse-checkout: schemas/*.schema.json } }, + { uses: actions/checkout@v6, with: { sparse-checkout: schemas/*.schema.json, persist-credentials: false } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{ uses: actions/checkout@v6, with: { sparse-checkout: schemas/*.schema.json, persist-credentials: false } },🧰 Tools
🪛 zizmor (1.25.2)
[warning] 10-10: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pages.yml at line 10, The checkout step using actions/checkout@v6 currently omits the persist-credentials setting; update the checkout action invocation (the line that uses actions/checkout@v6) to include persist-credentials: false to avoid storing Git credentials in the workspace (and thus leaking them into the Pages artifact), while keeping the existing sparse-checkout configuration intact.
10-12:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winArrr, ye've got a scurvy path mismatch here, matey! 🏴☠️
Line 10 be doin' a sparse checkout of only
schemas/*.schema.json, but then line 12 be uploadin' the entire current directory (path: .). That means after the sparse checkout, ye working directory only contains the schema files, so the upload will only contain those files.But here be the real problem: the workflow triggers on
paths: [schemas/**](line 3), which fires even when example files change, yet yer sparse checkout pattern don't include 'em! If someone pushes a change toschemas/doctor.v1.example.json, this workflow will trigger but won't check out that file, then upload an incomplete artifact.🏴☠️ Proposed fix to align the checkout and upload
- { uses: actions/checkout@v6, with: { sparse-checkout: schemas/*.schema.json } }, + { uses: actions/checkout@v6, with: { sparse-checkout: "schemas/\n" } },This checks out the entire
schemas/directory, matchin' the trigger pattern.Alternatively, if ye only want to publish schema files (not examples), update the upload path:
- { uses: actions/upload-pages-artifact@v4, with: { path: . } }, + { uses: actions/upload-pages-artifact@v4, with: { path: schemas } },But then reconsider whether the sparse checkout be needed at all, savvy?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{ uses: actions/checkout@v6, with: { sparse-checkout: "schemas/\n" } }, { uses: actions/configure-pages@v5, with: { enablement: true }, id: conf }, { uses: actions/upload-pages-artifact@v4, with: { path: . } },🧰 Tools
🪛 zizmor (1.25.2)
[warning] 10-10: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 10-10: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 11-11: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 12-12: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pages.yml around lines 10 - 12, The workflow is doing a sparse checkout of only "schemas/*.schema.json" via the actions/checkout step (sparse-checkout) but later uploads the whole workspace (path: .) while the workflow trigger uses paths: [schemas/**]; update the checkout/upload to be consistent: either change the sparse-checkout pattern to "schemas/**" so example files (e.g., schemas/doctor.v1.example.json) are included, or keep the narrow sparse-checkout and change the upload to only publish the schema artifacts (e.g., upload "schemas/*.schema.json"); alternatively remove sparse-checkout entirely if you want the full repo—modify the actions/checkout and actions/upload-pages-artifact usages accordingly..zed/settings.json (1)
9-23:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winArr, ye've mapped the
doctor.v3-draftschema (line 20) but forgot thewhy.v3-draftone, ye barnacle! 🦜The commit messages mention
why.v3-draft.example.jsonin the schema generation layer, and the review stack context lists it in layer 4's committed schema files. For consistency with the other v3-draft schema (doctor), ye should add a mapping for why.v3-draft too, savvy?🏴☠️ Proposed fix to add the missing schema mapping
{ "fileMatch": ["schemas/why.v2.example.json", "why.v2.example.json"], "url": "./schemas/why.v2.schema.json" }, - { "fileMatch": ["schemas/doctor.v3-draft.example.json", "doctor.v3-draft.example.json"], "url": "./schemas/doctor.v3-draft.schema.json" } + { "fileMatch": ["schemas/doctor.v3-draft.example.json", "doctor.v3-draft.example.json"], "url": "./schemas/doctor.v3-draft.schema.json" }, + { "fileMatch": ["schemas/why.v3-draft.example.json", "why.v3-draft.example.json"], "url": "./schemas/why.v3-draft.schema.json" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."settings": { "json": { "schemas": [ { "fileMatch": ["targets.json"], "url": "./npm/targets.schema.json" }, { "fileMatch": ["schemas/*.schema.json", "*.schema.json"], "url": "https://json-schema.org/draft/2020-12/schema" }, { "fileMatch": ["schemas/doctor.v1.example.json", "doctor.v1.example.json"], "url": "./schemas/doctor.v1.schema.json" }, { "fileMatch": ["schemas/doctor.v2.example.json", "doctor.v2.example.json"], "url": "./schemas/doctor.v2.schema.json" }, { "fileMatch": ["schemas/list.v1.example.json", "list.v1.example.json"], "url": "./schemas/list.v1.schema.json" }, { "fileMatch": ["schemas/list.v2.example.json", "list.v2.example.json"], "url": "./schemas/list.v2.schema.json" }, { "fileMatch": ["schemas/why.v1.example.json", "why.v1.example.json"], "url": "./schemas/why.v1.schema.json" }, { "fileMatch": ["schemas/why.v2.example.json", "why.v2.example.json"], "url": "./schemas/why.v2.schema.json" }, { "fileMatch": ["schemas/doctor.v3-draft.example.json", "doctor.v3-draft.example.json"], "url": "./schemas/doctor.v3-draft.schema.json" }, { "fileMatch": ["schemas/why.v3-draft.example.json", "why.v3-draft.example.json"], "url": "./schemas/why.v3-draft.schema.json" } ] } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.zed/settings.json around lines 9 - 23, Add the missing why.v3-draft JSON schema entry to the settings.json json.schemas array to mirror the doctor.v3-draft mapping: create a schema mapping with fileMatch entries for "schemas/why.v3-draft.example.json" and "why.v3-draft.example.json" and set its url to "./schemas/why.v3-draft.schema.json" so it matches the existing pattern used by "doctor.v3-draft" and the other v3-v2 mappings in the "schemas" list.bin/runner (1)
1-1:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Search for version bumps in version-bearing files echo "=== Checkin' Cargo.toml ===" git diff origin/master...HEAD -- Cargo.toml | grep -A2 -B2 'version.*=' echo -e "\n=== Checkin' package.json ===" git diff origin/master...HEAD -- package.json | grep -A2 -B2 '"version"' echo -e "\n=== Checkin' other version files ===" fd -t f '(pyproject.toml|setup.py|go.mod|pom.xml|build.gradle|VERSION)$' -x git diff origin/master...HEAD -- {} \; | grep -i versionRepository: kjanat/runner
Length of output: 258
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== Git remotes ===" git remote -v echo -e "\n=== Current branch ===" git rev-parse --abbrev-ref HEAD echo -e "\n=== Remote branches ===" git branch -r | sed 's/^/ /' # Detect a usable remote base (prefer main, then master) BASE_CANDIDATES=(origin/main origin/master origin/develop origin/trunk) BASE="" for c in "${BASE_CANDIDATES[@]}"; do if git show-ref --verify --quiet "refs/remotes/$c"; then if git merge-base --is-ancestor "$c" HEAD 2>/dev/null || true; then BASE="$c" break fi fi done if [ -z "$BASE" ]; then echo "Could not find a suitable remote base from candidates; falling back to first remote branch." BASE="$(git branch -r | head -n1 | sed 's/^[ *]*//' )" fi echo -e "\n=== Using base ref: $BASE ===" git rev-parse "$BASE" >/dev/null git merge-base "$BASE" HEAD echo -e "\n=== Check Cargo.toml version diffs ===" git diff "$BASE"...HEAD -- Cargo.toml 2>/dev/null | grep -E -A2 -B2 'version\s*=' || true echo -e "\n=== Check package.json version diffs ===" if git ls-files --error-unmatch package.json >/dev/null 2>&1; then git diff "$BASE"...HEAD -- package.json 2>/dev/null | grep -E -A2 -B2 '"version"\s*:' || true else echo "package.json not found in repo." fi echo -e "\n=== Check other version-like files diffs ===" fd -t f '(pyproject.toml|setup.py|go.mod|pom.xml|build.gradle|VERSION)$' \ -x sh -c 'git diff "'"$BASE"'...HEAD -- "{}" 2>/dev/null | grep -i -E -A2 -B2 "version" || true' \ | head -n 200Repository: kjanat/runner
Length of output: 336
🏁 Script executed:
#!/bin/bash set -euo pipefail BASE=origin/master echo "=== Merge-base availability (may fail) ===" git merge-base "$BASE" HEAD || true echo -e "\n=== Version fields in Cargo.toml (BASE vs HEAD) ===" if git ls-files --error-unmatch Cargo.toml >/dev/null 2>&1; then echo "-- BASE: $BASE:Cargo.toml" git show "$BASE":Cargo.toml | rg -n '^\s*version\s*=' || true echo "-- HEAD: Cargo.toml" rg -n '^\s*version\s*=' Cargo.toml || true echo -e "\n-- Diff ($BASE..HEAD):" git diff "$BASE"..HEAD -- Cargo.toml | rg -n 'version\s*=' -n || true else echo "Cargo.toml not present in repo." fi echo -e "\n=== Version fields in package.json (BASE vs HEAD) ===" if git ls-files --error-unmatch package.json >/dev/null 2>&1; then echo "-- BASE: $BASE:package.json" git show "$BASE":package.json | rg -n '"version"\s*:' || true echo "-- HEAD: package.json" rg -n '"version"\s*:' package.json || true echo -e "\n-- Diff ($BASE..HEAD):" git diff "$BASE"..HEAD -- package.json | rg -n '"version"\s*:' || true else echo "package.json not present in repo." fi echo -e "\n=== Other version-like files (present) ===" VERS_FILES="$(git ls-files | rg -E '(^|/)(pyproject\.toml|setup\.py|go\.mod|pom\.xml|build\.gradle|VERSION)$' || true)" echo "$VERS_FILES" if [ -n "${VERS_FILES}" ]; then echo -e "\n=== Diff hits for 'version' in those files ($BASE..HEAD) ===" # Show only lines containing 'version' (and their immediate context) to keep output tight for f in $VERS_FILES; do echo -e "\n-- $f" if git diff "$BASE"..HEAD -- "$f" >/dev/null 2>&1; then git diff "$BASE"..HEAD -- "$f" | rg -n -C1 -i 'version' || true fi done fiRepository: kjanat/runner
Length of output: 1332
🏁 Script executed:
#!/bin/bash set -euo pipefail BASE=origin/master echo "=== Cargo.toml(s) version fields changed? ===" CARGO_FILES="$(git ls-files | rg 'Cargo\.toml$' || true)" echo "Cargo files:" echo "$CARGO_FILES" if [ -n "${CARGO_FILES}" ]; then for f in $CARGO_FILES; do echo -e "\n-- $f" echo "-- BASE version lines" git show "$BASE:$f" 2>/dev/null | rg -n '^\s*version\s*=\s*"' || true echo "-- HEAD version lines" rg -n '^\s*version\s*=\s*"' "$f" || true echo "-- Diff version lines" git diff "$BASE"..HEAD -- "$f" 2>/dev/null | rg -n -C1 'version\s*=\s*"' || true done fi echo -e "\n=== Version-like files present (tracked) ===" VERSION_FILES="$(git ls-files | rg -i '(^|/)(pyproject\.toml|setup\.py|go\.mod|pom\.xml|build\.gradle|VERSION)$' || true)" echo "$VERSION_FILES" if [ -n "${VERSION_FILES}" ]; then for f in $VERSION_FILES; do echo -e "\n-- $f" git diff "$BASE"..HEAD -- "$f" 2>/dev/null | rg -n -i -C1 'version' || true done fi echo -e "\n=== package.json ===" if git ls-files --error-unmatch package.json >/dev/null 2>&1; then git diff "$BASE"..HEAD -- package.json | rg -n -C1 '"version"' || true else echo "package.json not tracked in this repo." fiRepository: kjanat/runner
Length of output: 955
Bump MINOR version for the new schema feature (Cargo.toml wasn’t bumped)
Cargo.toml’s crate
version = "0.12.2"is unchanged betweenorigin/masterandHEAD, so the required MINOR increment for backward-compatible functionality didn’t happen. No trackedpackage.jsonor other common version files were found to bump.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bin/runner` at line 1, The crate version in Cargo.toml wasn't bumped for the new schema feature; update the package version field in Cargo.toml (the `version = "0.12.2"` entry) to increment the MINOR version (e.g., `0.13.0`) so the release reflects backward-compatible new functionality, then commit the change and include a short commit message referencing the schema feature bump.Source: Coding guidelines
rust-toolchain.toml (1)
2-2:
⚠️ Potential issue | 🟡 Minor🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Check for Rust version-specific features or syntax that might be affected # Check for edition 2024 features that might require specific Rust versions rg -n "edition.*2024" --type toml # Check for potential nightly/unstable features rg -n "#!\[feature\(" --type rust # Check CI configuration for Rust version expectations rg -n "rust.*version|toolchain" .github/workflows/Repository: kjanat/runner
Length of output: 1884
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== rust-toolchain.toml ===" if [ -f rust-toolchain.toml ]; then nl -ba rust-toolchain.toml | sed -n '1,120p' else echo "missing rust-toolchain.toml" fi echo echo "=== workflows/toolchain pins (release.yml, crates-release.yml) ===" for f in .github/workflows/release.yml .github/workflows/crates-release.yml; do if [ -f "$f" ]; then echo "--- $f (toolchain setup blocks) ---" # show lines around dtolnay/rust-toolchain and any 'stable'/'nightly' usage rg -n "dtolnay/rust-toolchain|rust-toolchain|toolchain|nightly|stable" "$f" -n --context 2 || true fi done echo echo "=== search for explicit pinned versions (1.95 etc.) ===" rg -n "1\.95|1\.9[0-9]|toolchain.*[0-9]+\.[0-9]+|channel *= *\"[0-9]+\.[0-9]+\"" .github/workflows Cargo.toml rust-toolchain.toml rust-toolchain . || true echo echo "=== search for nightly features (#![feature( ... ) ) ===" rg -n "#!\[feature\(" --type rust . || trueRepository: kjanat/runner
Length of output: 146
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== rust-toolchain.toml (current) ===" if [ -f rust-toolchain.toml ]; then cat -n rust-toolchain.toml | sed -n '1,120p' else echo "missing rust-toolchain.toml" fi echo echo "=== search for pinned Rust versions (esp 1.95) ===" rg -n "1\.95|channel\s*=\s*\"[0-9]+\.[0-9]+\"|toolchain\s*=\s*\"[0-9]+\.[0-9]+\"" . || true echo echo "=== search for explicit toolchain/channel pins in workflows ===" rg -n "dtolnay/rust-toolchain|channel\s*:\s*(stable|nightly|1\.)|toolchain" .github/workflows --hidden || trueRepository: kjanat/runner
Length of output: 2371
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "=== src/tool/node.rs excerpt around 991 ===" sed -n '960,1030p' src/tool/node.rs echo echo "=== Cargo.toml excerpt (top) ===" sed -n '1,40p' Cargo.tomlRepository: kjanat/runner
Length of output: 4243
Arrr—why’s
rust-toolchain.tomlswitched tochannel = "stable"?Your crate still pins MSRV via
Cargo.toml(rust-version = "1.95") and there’s no nightly-only#![feature(...)]usage, so this isn’t about unstable features blowing up. The real risk is reproducibility: CI/workflows also install the lateststable, so future Rust/rustfmt/clippychanges could subtly alter behaviour or output.Is that intentional for schema generation? If so, document the rationale (or consider pinning for reproducible toolchain/formatter behaviour).
Also:
src/tool/node.rscontains a test comment claimingrust-toolchain.toml“pins to 1.95” — that’s now untrue, so clean that up.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust-toolchain.toml` at line 2, Restore reproducible pinning by setting rust-toolchain.toml to the MSRV matching Cargo.toml (e.g., change channel = "stable" back to the exact toolchain that matches rust-version = "1.95", or pin to "1.95.0") so CI and local tooling use the same fixed toolchain, and update the outdated test comment in src/tool/node.rs that currently claims rust-toolchain.toml “pins to 1.95” to reflect the chosen pin (or document the intentional use of latest stable if you prefer that route); ensure references to Cargo.toml's rust-version and the comment in src/tool/node.rs are consistent with the chosen policy.schemas/why.v1.example.json (2)
4-15:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a placeholder path here.
Arrr,
source_diris hard-coded to/home/kjanat/..., which leaks a developer-machine path into a committed example and makes the artefact brittle across regenerations.Also applies to: 17-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/why.v1.example.json` around lines 4 - 15, Replace the hard-coded developer path in the example JSON by using a neutral placeholder or null for the source_dir in the candidates array; locate the "candidates" object entry that contains "source_dir": "/home/kjanat/..." and change it to a generic path such as "/path/to/cargo/config.toml" or set "source_dir" to null (and update the same pattern at the other occurrence referenced in the comment), ensuring the example no longer exposes a personal filesystem path.
4-27:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the machine-specific absolute paths from the example fixtures.
The shared root cause is the same across
schemas/why.v1.example.json,schemas/why.v2.example.json, andschemas/why.v3-draft.example.json: they embed/home/kjanat/...in path-bearing fields. Please switch these to repo-relative or neutral placeholder values so the committed examples do not leak local filesystem layout and stay stable when regenerated elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/why.v1.example.json` around lines 4 - 27, The example fixtures embed machine-specific absolute paths in the JSON fields named "source_dir" (seen inside the "candidates" array and the "selected" object); update those values in schemas/why.v1.example.json (and the corresponding v2/v3 example files) to use repo-relative or neutral placeholders (e.g. a repo-relative path like ".cargo/config.toml" or a token such as "<repo>/.cargo/config.toml") instead of "/home/kjanat/…", ensuring both the candidate entry and the selected entry are updated consistently.schemas/why.v1.schema.json (1)
4-15:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a placeholder path here.
Arrr,
source_diris hard-coded to/home/kjanat/..., which leaks a developer-machine path into a committed example and makes the artefact brittle across regenerations.Also applies to: 17-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/why.v1.schema.json` around lines 4 - 15, The schema/example currently hard-codes a developer path in the "source_dir" field (e.g., "/home/kjanat/...") inside the PmResolution example; replace that literal with a neutral placeholder (for example "<PROJECT_ROOT>/src" or "./path/to/source") so the example does not leak a machine path and remains stable across regenerations; locate the "source_dir" property within the PmResolution anyOf/object example and update its example/default value accordingly.schemas/why.v2.example.json (1)
4-15:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a placeholder path here.
Arrr,
source_diris hard-coded to/home/kjanat/..., which leaks a developer-machine path into a committed example and makes the artefact brittle across regenerations.Also applies to: 17-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/why.v2.example.json` around lines 4 - 15, The example JSON leaks a developer machine path in the "source_dir" field inside the "candidates" entry; update that value to a neutral placeholder (e.g., a generic path like "/path/to/config.toml" or null) and do the same for the other occurrences of "source_dir" in this file (lines referenced in the review: the later candidate entries) so the example contains no hard-coded user-specific paths.schemas/why.v3-draft.example.json (1)
4-5:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the local paths in this example.
Arrr,
root,source, andcwdall point at/home/kjanat/..., which leaks local layout and makes the fixture machine-specific.Also applies to: 9-23, 35-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/why.v3-draft.example.json` around lines 4 - 5, The example JSON contains machine-specific absolute paths (keys like "root", "source", and "cwd") that leak local layout; replace those values with neutral, platform-independent placeholders or relative paths (e.g., "/workspace/project", "./", or "<project_root>") throughout the file (occurrences around lines 9-23 and 35-49 as well) so the fixture is reusable and non-machine-specific, ensuring the "query" and other non-path fields remain unchanged.src/cmd/schema.rs (3)
21-44:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftNyaa~! The branching logic is clean but… where are the TESTS?! 😤
The
write_schemadispatch andwrite_all_schemasdirectory validation look solid. Thatbail!on line 35 when the output isn't a directory? Chef's kiss. But this whole file is doing complex JSON manipulation with zero test coverage.For code that's patching generated schemas with hardcoded assumptions about schemars output structure (lines 123-147), that's a maintenance timebomb waiting to go off when schemars changes its internal format.
At minimum, add tests for:
write_all_schemasdirectory validationpatch_schema_version_constactually sets the const fieldpatch_source_schemacorrectly rewrites TaskInfo/WhyCandidate source refs- Round-trip: generate schema → deserialise → validate structure
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/schema.rs` around lines 21 - 44, Add unit tests covering write_all_schemas and the schema patching helpers: create tests that (1) assert write_all_schemas returns an error when given a path that exists but is a file (use a temp file and call write_all_schemas to trigger the bail!), (2) verify patch_schema_version_const mutates the schemars output by constructing a schema Value with the expected const location and asserting the const value is set, (3) verify patch_source_schema rewrites TaskInfo/WhyCandidate source refs by constructing representative schema Value and checking the rewritten refs, and (4) implement a round-trip test that generates the schema via schema_documents()/schema_value(), writes or patches it, then deserializes the resulting JSON back into the expected types to assert structural validity; use tempdir/tempfile and the public functions write_schema, write_all_schemas, patch_schema_version_const, patch_source_schema, and schema_documents/schema_value to locate code under test.
123-147:
⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffArrr, this be fragile patchin' in shark-infested waters! 🦈
The
patch_schema_version_constandpatch_source_schemafunctions silently bail with early returns when the expected JSON structure ain't found. Ifschemarschanges how it emits schemas, these patches could silently fail and ye'd never know until a consumer tries to validate against a broken schema.Aye, the external context notes this be a "standard pattern" across other projects, but that don't make it less brittle. Consider:
- Add logging/warnings when patches bail early (at least in debug builds)
- Add schema validation tests that would catch missing patches
- Document the schemars version dependency and structure assumptions
Right now, if
properties.schema_versionmoves or gets renamed, theconstfield won't be set and v1 schemas could masquerade as v2. That's a silent corruption risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/schema.rs` around lines 123 - 147, Both patch_schema_version_const and patch_source_schema currently return silently when expected JSON structure is missing, so update these functions (patch_schema_version_const and patch_source_schema) to emit a clear warning or debug log when the early-return branches are taken (use log::warn! or debug! depending on build) including which key was missing (e.g., "properties" or "$defs" or "schema_version") and the top-level schema snippet if helpful; then add unit/integration tests that call these functions with altered/malformed schemars output to assert the warning is emitted and that the schema is patched when structure is correct, and add a small note to the project README or docs specifying the assumed schemars output shape and supported schemars version so future changes are caught.
173-202: 🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy lift
Hardcoded label maintenance burden ahead! 📋
The
source_labelsfunction has hardcoded arrays for v1 and v2 labels. If a newTaskSourcevariant lands (say,TaskSource::NixFlake), these arrays need manual updates or the generated schemas will be incomplete.This is probably unavoidable given the version-specific label mapping requirement, but it's a footgun for future maintainers. Consider:
- Add a compile-time check (e.g., a test that compares
TaskSource::all()count to schema enum count)- Document this maintenance requirement prominently
- Consider a procedural macro to generate these from the
TaskSourceenum itselfRight now, forgetting to update this after adding a
TaskSourcewill cause schema/runtime mismatch that only manifests when users hit the new task source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cmd/schema.rs` around lines 173 - 202, The hardcoded arrays in source_labels are brittle; add a compile-time/unit test that asserts the number of entries returned by source_labels(version) equals the number of TaskSource variants (use TaskSource::all() or equivalent) for each schema version, and update source_labels when adding variants; specifically, implement a test referencing source_labels and TaskSource::all() that fails if counts differ (so CI catches missing labels), and add a short doc comment by the source_labels function explaining the maintenance requirement.
1 participant
Stacked on #base: fix/version-ranges-pm-override
Adds generated schemas/examples for JSON output surfaces and schema generation support.
Verify:
run gen-schemajq empty schemas/*.schema.json schemas/*.example.jsoncargo check --features schema --bin runner