Repository navigation
Harden agent workflow bootstrap and tool setup - #396
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe change removes ChangesCargo metadata tool resolution
Versioned Copilot CLI caching
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: ⚪ Minimal · up to The workflow and tooling changes introduce no identified merge-blocking risk at the current head; the PR is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant AgentReview
participant ActionsCache
participant RunnerToolcache
participant CopilotCLI
AgentReview->>ActionsCache: Restore versioned CLI cache
AgentReview->>CopilotCLI: Install Copilot CLI 1.0.79
AgentReview->>RunnerToolcache: Seed architecture-specific executable
AgentReview->>ActionsCache: Save the CLI toolcache
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/agent-review.md:
- Around line 30-41: Update the restore steps in
.github/workflows/agent-review.md (lines 30-41) and
.github/workflows/agent-review.lock.yml (lines 457-462 and 1314-1318) to use
continue-on-error: true, then add an explicit actions/cache/save step after the
Copilot CLI tool cache is seeded so saving does not depend on job success.
Regenerate .github/workflows/agent-review.lock.yml from the updated workflow.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6887a02-b0d7-4fdd-b0e0-247eeb785e02
📒 Files selected for processing (9)
.github/actions/tools/action.yml.github/workflows/agent-review.lock.yml.github/workflows/agent-review.md.github/workflows/ci.yml.pre-commit-config.yamlCargo.tomlcrates/amiss/tests/documentation_contracts/tools.rsdocs/src/development.mdscripts/tools.sh
💤 Files with no reviewable changes (1)
- scripts/tools.sh
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
Tool bootstrap is right and the copilot cache works under the current pin, but the version literal is written six times per workflow and a partial bump would silently disable the cache.
Question: the copilot version literal 1.0.79 is hardcoded in six places in agent-review.md (engine.version line 19, cache key/path 31-32, detection restore key/path 40-41, seed target line 58, safe-outputs engine line 72) and re-emitted in the compiled lock; a bump that edits only engine.version silently disables the new cache — should the key and path derive the version from it?
Because the cache key and the seed target also hardcode the version, a partial bump makes the seed step copy the new binary under the old version's toolcache directory, while the installer's exact-version lookup (install_copilot_cli.sh v0.86.2) can never match the new version: the restore keeps hitting the stale key and the installer downloads on every run — the CDN-503 failure mode this PR removes — leaving a wrong-version binary under the old label.
confirmation: simulated the seed step with a version mismatch — the new binary landed at copilot-cli/1.0.79/x64/bin/copilot, and the installer's own lookup shape (find -maxdepth 4 -path '*/bin/copilot' -path '*/1.0.80/*') returned zero results.
Nit: the seed step reimplements the uname -m → x64|arm64 mapping the installer already owns and exit 1s on unknown arches; both maps agree today but can drift — agent-review.md:52 vs install_copilot_cli.sh lines 105-113. Reading the arch from ${{ runner.arch }} would keep a single source.
confirmation: diffed both maps — identical four-name mapping today.
Session details
run — cargo nextest run --workspace --locked (1364 passed, 3 skipped), amiss check --repo . --object-format sha1 --profile enforce (exit 0), the six documentation_contracts tools tests, all three cargo metadata jq queries, taiki-e manifest version checks at both pinned SHAs, gh-aw-actions v0.86.2 installer reading, and a sandbox simulation of the seed step.
There was a problem hiding this comment.
Tip
Everything holds — the copilot pin, the toolcache seeding, and the cargo-metadata queries all verify against the tree and the pinned upstream installer.
No findings were confirmed. The load-bearing claims were checked against the tree, the pinned action sources, and live release metadata.
- The seeded layout
${RUNNER_TOOL_CACHE}/copilot-cli/1.0.79/${arch}/bin/copilotmatches what gh-aw's installer (install_copilot_cli.sh at the pinned sha) scans:find -maxdepth 4 -path '*/bin/copilot', exact-version match, arch namesx64/arm64, and the cached-copy activation prepends the bin dir to PATH, so the seed guard short-circuits on a hit. Thevprefix the installer adds to the version makes the download URL point at the real tag. - The
v1.0.79release ofgithub/copilot-clipublishescopilot-linux-x64.tar.gz; the exact pinned artifact exists. - The three jq projections (the tools composite, the platform-tests nextest pin, and both ratchet hooks) emit exactly what the deleted
scripts/tools.shproduced;hashFiles('Cargo.toml')covers every pin edit since the pins live in the root manifest. fallback: noneis a documented input of the pinned taiki-e/install-action revision.- The toolcache restore in the detection leg lands after the agent job's save step (save runs before the agent, detection
needs: agent), so even the first run's detection leg hits the cache.
Session details
Ran: cargo nextest run --locked -p amiss --test documentation_contracts (32/32 passed, including all seven tools tests), a simulation of the installer's toolcache scan against the seeded layout (exact match, no download), the three jq expressions against the live manifest, and a check of the v1.0.79 release assets and both pinned installers' sources.
The lane installers die when GitHub's release CDN 503s, and replay proved the outage sticks around (#392, #393). This makes the review lane's copilot install survive it.
Both engine blocks pin copilot 1.0.79, so the installer asks for one fixed artifact. A cache block keeps the toolcache copy of that version across runs, a post-install step seeds the toolcache from wherever the installer put the binary, and the detection job restores the same cache before its own installer runs. The installer's toolcache lookup finds the seeded copy and skips the download, and since the agent job's cache save lands before detection starts, even the first run's detection leg is covered.
The awf bundle still comes off the release CDN every run, since its installer has no toolcache lookup to feed. That half needs an upstream change. The mention and triage lanes keep downloading too; the same pattern extends to them once this proves out.
On the tool bench, scripts/tools.sh is gone. Every consumer queries cargo metadata with jq directly, the shared tool cache keys off hashFiles on the manifest, and taiki-e installs carry fallback none so a bad pin fails loudly instead of quietly building from source. The contract tests check the cargo form in the composite, the platform lane, and both ratchet hooks, and refuse the shim coming back.
Summary by cubic
Hardens the agent review lane against GitHub Releases CDN 503s and makes tool setup deterministic. The review lane now pins Copilot CLI 1.0.79, restores it from the runner toolcache, warms the build cache, and extends the timeout from 20 to 30 minutes.
Details
COPILOT_CLI_VERSIONandGH_AW_ENGINE_VERSIONto keep one version across the lane and lock.${{ runner.tool_cache }}viaactions/cache/restore, seeds${{ runner.tool_cache }}/copilot-cli/1.0.79/<arch>/bin/copilot, and saves withactions/cache/savebefore the agent runs; detection restores the same cache and skips downloads.shared-key: gatestoSwatinem/rust-cacheto warm the lane build cache from the gates job (applied to review, mention, triage, and CI).scripts/tools.shshim with directcargo metadata --locked --format-version 1 --no-deps --offlinequeries; the tools composite keys its cache withhashFiles('Cargo.toml')+runner.archand setsfallback: noneintaiki-e/install-action.cargo-nextestpin from cargo metadata and installs withfallback: none; pre-commit hooks read thesimilarity-rspin from thesourcetable.Migration
scripts/tools.shusage withcargo metadata --locked --format-version 1 --no-deps --offline | jq ...as shown in the tools composite and hooks.Written for commit cc55370. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Documentation