Skip to content

Agents: bind fleet closeout to generated pack - #4805

Merged
justin808 merged 2 commits into
mainfrom
jg-codex/faster-fleet-validation
Jul 27, 2026
Merged

Agents: bind fleet closeout to generated pack#4805
justin808 merged 2 commits into
mainfrom
jg-codex/faster-fleet-validation

Conversation

@justin808

@justin808 justin808 commented Jul 26, 2026

Copy link
Copy Markdown
Member

Why

Fleet closeout is the release-safety boundary that validates the durable result ledger and renders the tracker disposition. The generated INDEX.md previously invoked the validator from the repository root but passed bare result-ledger.json and tracker-closeout.md paths. That made the command depend on the operator's current directory and could validate or write an unrelated file instead of the selected fleet pack.

This branch was refreshed from current origin/main. Its four historical local commits were already merged byte-for-byte as squash commit 5121928ca in #4694, so they were not replayed as duplicate history; the current-main implementation and later lifecycle hardening were reviewed semantically instead.

What changed

  • Thread the generated output directory into the launch-index renderer.
  • Normalize the output directory at pack generation, then emit a shell-escaped absolute PACK_DIR and bind both --ledger and --render-tracker to that exact directory.
  • Run the validator through bundle exec ruby, matching the documented repository workflow.
  • Add regressions for an output path with spaces and for relative generation followed by closeout from another working directory.

Release-safety rationale

The independent checker still receives pack ID, selector, candidate/tag commit, policy commit, and tracker mode from external launch evidence. This change does not relax schema or semantic validation. It removes ambient-CWD ambiguity so a successful closeout necessarily reads the intended pack ledger and writes the tracker matrix beside it, reducing the risk of false promotion evidence.

Verification

  • bundle exec ruby .agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb — 166 runs, 670 assertions
  • bundle exec ruby .agents/skills/run-fleet-validation/scripts/replay_rc12_lifecycle.rb — pass
  • bundle exec ruby .agents/skills/run-fleet-validation/scripts/fleet_health_test.rb — 73 runs, 393 assertions
  • bundle exec ruby .agents/skills/run-fleet-validation/scripts/replay_rc12_fleet_health.rb — pass
  • bundle exec rubocop --config .agents/.rubocop.yml .agents/skills/run-fleet-validation/scripts/generate_prompts.rb .agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb — no offenses
  • CI-equivalent OSS RuboCop — 247 files, no offenses
  • skill quick validator — valid
  • .agents/bin/validate --changed — pass
  • pre-push branch lint — pass
  • independent Sol/xhigh exact-head checker — clean, no outstanding findings
  • current-head Claude review — clean overall; documentation suggestion verified against the existing skill and declined with evidence
  • current-head CodeRabbit review — approved, no actionable comments

Review-system coverage

Current head: 8fc13ee7693955223b95628757077910c654fe07.

  • Working: Claude review, CodeRabbit, and the independent Codex Sol/xhigh checker.
  • Degraded: Copilot was quota-limited; Greptile did not emit a fresh-head artifact after the fix. Greptile's old-head P1 finding was fixed, replied to, and resolved; both current-head working reviewers and the independent checker found no blocker.
  • Coverage floor: satisfied with at least two independent working systems. All review threads are resolved and exact-head required checks are complete.

Churn and scope

  • Two implementation commits: the original closeout hardening and one batched review-fix commit for relative output directories.
  • No changelog entry: this hardens internal contributor release tooling and does not change product behavior.

Summary by CodeRabbit

  • Bug Fixes
    • Improved generated validation/closeout instructions to reliably reference the correct pack output directory by normalizing and using an absolute path.
    • Updated closeout commands so ledger and tracker arguments point to pack-scoped files.
  • Tests
    • Added/expanded test coverage to verify generated closeout commands use the correct PACK_DIR value for both absolute and relative output locations.

Completed-batch audit

Status: Clean — no outstanding findings or follow-ups. Durable receipt.

Copilot AI review requested due to automatic review settings July 26, 2026 23:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The fleet validation prompt generator now normalizes pack output paths, passes them into index rendering, and generates pack-scoped ledger closeout commands with coverage for absolute and relative output directories.

Changes

Fleet pack closeout

Layer / File(s) Summary
Pass normalized output directory to index rendering
.agents/skills/run-fleet-validation/scripts/generate_prompts.rb
write_pack expands output_dir and passes it to the updated render_index method.
Generate and test pack-scoped closeout commands
.agents/skills/run-fleet-validation/scripts/generate_prompts.rb, .agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb
Generated closeout commands define pack and expected-value variables, use pack-scoped ledger and tracker paths, and test absolute path resolution for temporary and relative output directories.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested labels: ready-for-hosted-ci

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: binding fleet closeout to the generated pack directory.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/faster-fleet-validation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 @.agents/skills/run-fleet-validation/scripts/generate_prompts.rb:
- Line 89: Resolve and normalize output_dir to an absolute path before passing
it to render_index and embedding PACK_DIR, including the related result-ledger
and tracker path handling. Add a regression test covering a relative
--output-dir and verify generated paths remain anchored to that directory
regardless of the launch directory.
🪄 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: CHILL

Plan: Pro Plus

Run ID: 968fc38d-8219-433e-b01c-38fbb9a76ed2

📥 Commits

Reviewing files that changed from the base of the PR and between 37ddaf5 and a15fdc2.

📒 Files selected for processing (2)
  • .agents/skills/run-fleet-validation/scripts/generate_prompts.rb
  • .agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb

Comment thread .agents/skills/run-fleet-validation/scripts/generate_prompts.rb
@claude

claude Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review: Agents: bind fleet closeout to generated pack

Overview

Fixes ambient-CWD ambiguity in the generated INDEX.md closeout instructions: the validator command now binds --ledger and --render-tracker to the actual generated pack directory ($PACK_DIR) instead of bare relative filenames, and invokes the validator via bundle exec ruby to match the documented workflow elsewhere in this skill (SKILL.md).

Code quality

  • Minimal, surgical diff: render_index now threads output_dir through, and write_pack passes it at the one call site (generate_prompts.rb:89). No other callers needed updates.
  • Correct use of Shellwords.escape for PACK_DIR — verified it safely handles spaces, quotes, $, backticks, and even embedded newlines (Ruby's Shellwords.escape quotes newlines rather than leaving them raw), so there's no shell-injection or malformed-script risk from an unusual --output-dir value.
  • The unquoted PACK_DIR=#{Shellwords.escape(output_dir)} assignment is fine specifically because Shellwords.escape backslash-escapes metacharacters for exactly this unquoted-assignment context; all downstream uses are properly double-quoted ("$PACK_DIR/result-ledger.json").
  • bundle exec ruby prefix now matches every other documented invocation of these scripts in SKILL.md — good consistency.

Test coverage

  • New regression test (test_index_closeout_command_targets_the_generated_pack_directory) uses an output path containing a space, which is exactly the case that would break naive interpolation. It asserts the PACK_DIR= assignment, the bundle exec ruby prefix, and both quoted $PACK_DIR-relative flag values. This is a good, targeted test — no complaints.

Risk / correctness notes (non-blocking)

  • output_dir is taken verbatim from the CLI's --output-dir (no File.expand_path), so if it's a relative path, $PACK_DIR will only resolve correctly if the operator runs the closeout bundle exec ruby command from the same working directory used to invoke generate_prompts.rb. That matches the documented "run from repo root" workflow, but it's an implicit coupling — worth being aware of if --output-dir is ever invoked from a different cwd than the closeout step.
  • This only changes generated instructional text (Markdown consumed by a human operator), not executable logic, so blast radius is low even if something were off.

Verdict

Clean, well-tested, low-risk fix that does what the PR description claims. No blocking issues found.

@greptile-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR binds generated fleet closeout commands to a PACK_DIR and invokes the ledger validator through Bundler.

  • Threads the output directory into the INDEX renderer.
  • Quotes ledger and tracker paths beneath the generated pack directory.
  • Adds regression coverage for an absolute output path containing spaces.

Confidence Score: 3/5

The PR should not merge until generated closeout commands bind relative output directories to the pack’s actual absolute location.

The CLI permits relative output paths and carries them unchanged into PACK_DIR, so running the generated closeout command from a different directory can validate or render against the wrong location.

Files Needing Attention: .agents/skills/run-fleet-validation/scripts/generate_prompts.rb, .agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb

Important Files Changed

Filename Overview
.agents/skills/run-fleet-validation/scripts/generate_prompts.rb Adds pack-bound closeout paths, but preserves relative output directories and therefore does not fully eliminate working-directory-dependent resolution.
.agents/skills/run-fleet-validation/scripts/generate_prompts_test.rb Covers shell quoting for an absolute path containing spaces but does not cover relative output-directory resolution.

Reviews (1): Last reviewed commit: "Bind fleet closeout to its generated pac..." | Re-trigger Greptile

Comment thread .agents/skills/run-fleet-validation/scripts/generate_prompts.rb
Copilot AI review requested due to automatic review settings July 27, 2026 03:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: full PR history through 2026-07-27T03:16:10Z (no previous summary cutoff).

Mattered

  • CodeRabbit correctness blocker (thread): fixed in 8fc13ee7693955223b95628757077910c654fe07; the generation boundary now expands the output directory before embedding PACK_DIR, with relative-path and changed-launch-directory regression coverage. Replied and resolved.
  • Greptile P1 duplicate (thread): satisfied by the same fix and regression. Replied and resolved.

Skipped

  • CodeRabbit walkthrough/status text, the duplicate CodeRabbit review summary, Claude's clean review, Greptile's duplicate general summary, and the Copilot quota notice require no additional code changes. The two substantive claims are covered by the resolved threads above; the remaining items are status or reviewer-availability metadata.

Validation on the fixed head: generator 166 runs / 670 assertions; fleet health 73 runs / 393 assertions; both RC12 replays passed; focused and CI-equivalent RuboCop passed; skill validation passed; independent Sol/xhigh checker returned CLEAN with no outstanding findings.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review: Agents: bind fleet closeout to generated pack

Overview

Small, well-scoped fix to generate_prompts.rb: the generated INDEX.md closeout command now binds --ledger/--render-tracker to an absolute PACK_DIR computed at generation time (File.expand_path(output_dir)), runs the validator via bundle exec ruby, and a new regression test exercises a pack path containing spaces (quoting/anchoring). This closes the "operator's CWD determines which ledger gets validated/overwritten" hazard described in the PR body.

Code quality

  • The fix is minimal and localized: write_pack now normalizes output_dir once up front, so every downstream File.join (prompts, INDEX.md, @lifecycle.write_artifacts) consistently operates on the same absolute path — no behavior change to where files land, just how they're referenced in the generated instructions.
  • Reuses the existing Shellwords.escape pattern already used for @pack_id/@release_selector, so the new PACK_DIR=... assignment is consistent with the surrounding style and correctly handles spaces (bash treats a backslash-escaped space in a bare VAR=word assignment as part of the single token, so PACK_DIR ends up holding the unescaped literal path, and the later "$PACK_DIR/..." expansions are safe).
  • The two new tests cover this well: one asserts the exact generated command text, the other (test_index_closeout_command_anchors_a_relative_output_directory) specifically proves the bug scenario — generating with a relative output dir from one CWD, then confirming the embedded PACK_DIR still resolves correctly when the printed command is later run from an unrelated directory. That's the right regression test for this class of bug.

Potential issues

  1. Documentation drift (moderate): .agents/skills/run-fleet-validation/SKILL.md ("Validate and render closeout" section, ~line 159) still shows the old pattern this PR is fixing — plain ruby (not bundle exec ruby) and hardcoded relative paths (tmp/fleet-validation-prompts/result-ledger.json / tracker-closeout.md) instead of a PACK_DIR anchor. Since that doc is the human-facing skill instructions (likely read before a generated INDEX.md even exists), it still carries the exact ambient-CWD ambiguity this PR's rationale says it's removing. Worth a follow-up to sync the doc example with the generator output, otherwise the fix only protects operators who follow the generated pack and not those following the skill doc directly.

  2. Minor/informational — baked-in absolute path: PACK_DIR is now an absolute path resolved at generation time via File.expand_path. If a generated pack directory is later moved/copied/rsynced to a different location or machine before closeout runs, the embedded PACK_DIR would point at the stale original location. Probably fine given packs are expected to run in place, but worth being aware of as a tradeoff vs. e.g. resolving PACK_DIR relative to the script's own location at runtime.

Test coverage / verification

New tests are well targeted, and the space-containing-path case is a good choice to force quoting bugs to surface. The PR description lists a thorough manual verification list (full test suites, rubocop, lifecycle/health replay scripts, /simplify); nothing in the diff contradicts those claims.

Security

No concerns — this only changes generated documentation/instructions for a human-run validator command in internal release tooling; no new attack surface, and the shell-escaping is correct.

Overall: solid, low-risk fix. Recommend addressing the SKILL.md drift (finding 1) either in this PR or as an immediate follow-up so the documented workflow matches the generated one.

@justin808

Copy link
Copy Markdown
Member Author

Reviewed the documentation-drift observation in the current-head Claude review: #4805 (comment)

No code or documentation change is needed. The skill's closeout example already invokes bundle exec ruby and uses the complete default-pack paths tmp/fleet-validation-prompts/result-ledger.json and tmp/fleet-validation-prompts/tracker-closeout.md. The bug fixed by this PR was specific to generated INDEX.md output, which previously emitted bare filenames even when --output-dir selected another pack. The example remains correct for its documented default pack.

The noted absolute-path tradeoff is intentional: a fleet pack is machine/path-bound launch evidence. If it is moved to another machine or location, the operator must generate a fresh pack rather than reuse stale closeout instructions.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: current-head feedback since the previous summary at 2026-07-27T03:16:36Z.

Mattered

  • Claude current-head review (comment) returned an overall clean/low-risk assessment. Its documentation-drift suggestion was verified and declined: the skill already uses bundle exec ruby and full default-pack paths; only generated indexes had emitted bare filenames. The absolute-path portability note is an intentional regenerate-on-move constraint.

Skipped

  • CodeRabbit's current-head review produced no actionable comments and approved the fix. Its generic docstring-coverage warning does not identify a missing docstring in the changed Ruby code and conflicts with no repository lint rule; focused and CI-equivalent RuboCop are green.
  • Copilot remained quota-limited, and Greptile did not emit a fresh-head artifact. Their coverage state is recorded in the PR description before merge; the old Greptile blocker is fixed and its thread is resolved.

No open review threads remain, required checks are complete, and the exact-head independent checker is CLEAN.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

@justin808
justin808 added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit 1aa5d7e Jul 27, 2026
44 checks passed
@justin808
justin808 deleted the jg-codex/faster-fleet-validation branch July 27, 2026 03:23
@justin808

Copy link
Copy Markdown
Member Author

Completed-batch audit: replay evidence follows.

justin808 added a commit that referenced this pull request Jul 31, 2026
…t-policy

* origin/main: (33 commits)
  Fix ci-required base ref for PR merge-ref checkouts (#4819)
  Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817)
  [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821)
  Forward-port the 17.0.1 changelog section to main (#4814)
  Handle selector metacharacters in renderComponent DOM IDs (#4808)
  [Pro] Prevent caching RSC renders with errors (#4804)
  Agents: trust Copilot review identities (#4807)
  Agents: bind fleet closeout to generated pack (#4805)
  Docs: ADR 0002 — Skills-in-package over MCP for agent-native DX (#4735)
  Scope GitHub release commands to the origin repository (#4803)
  Forward-port OSS npm license metadata fix (#4794)
  Add golden-output gate for the serverWebpackConfig generator template (#4790)
  Cover the rspack CSS SSR generator fixes and de-duplicate the loader path (#4788)
  Configure agent workflow repo policy (#4785)
  Forward-port gh include mixed framing from #4684 (#4784)
  Release: enforce one-change forward-port closeout (#4783)
  Forward-port multi-URL rolling-deploy seeding to main (#4782)
  Docs: clarify React 18 streaming without RSC (#4780)
  Docs: forward-port v17 upgrade and generator gate guidance (#4781)
  Record the final React on Rails 17.0.0 changelog (#4742)
  ...

# Conflicts:
#	AGENTS.md
#	internal/contributor-info/release-train-runbook.md
justin808 added a commit that referenced this pull request Aug 8, 2026
…ential-broker

* origin/main: (75 commits)
  Silence routine startup diagnostics for Rails commands (#4849)
  Docs: move agent coordination to the HTTP backend (#4764)
  Detect unnoticed changes across generated webpack/Rspack configs (#4839)
  Fix durable ShakaPerf release evidence reuse (#4833)
  Docs: add missing content — release notes, upgrade guide, config, API references (#4843) (#4844)
  Fix incorrect docs: helper names, defaults, requirements, runtime refs (#4836)
  Scroll-priority streaming: candidate architecture evaluation (#4835) (#4841)
  Fix generated server config lint cleanup (#4840)
  Document serialized release backport policy (#4592)
  Package version-matched agent skills and docs (#4809)
  Fix ci-required base ref for PR merge-ref checkouts (#4819)
  Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817)
  [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821)
  Forward-port the 17.0.1 changelog section to main (#4814)
  Handle selector metacharacters in renderComponent DOM IDs (#4808)
  [Pro] Prevent caching RSC renders with errors (#4804)
  Agents: trust Copilot review identities (#4807)
  Agents: bind fleet closeout to generated pack (#4805)
  Docs: ADR 0002 — Skills-in-package over MCP for agent-native DX (#4735)
  Scope GitHub release commands to the origin repository (#4803)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants