Add downstream seam sync command and registry#22
Conversation
Add bin/push-downstream and downstream.yml so the managed "## Agent Workflow Configuration" seam can be rolled into consumer repos as one PR per repo, while each repo's seam values stay repo-owned. - reuse AgentWorkflowSeamDoctor::REQUIRED_KEYS and parsing as the single source of truth, so the command and the seam doctor cannot drift - reconcile is idempotent: insert a base-branch-seeded, n/a-filled seam when absent; preserve existing values (including multi-line) and extra optional keys when present - registry mode plans by default and clones/reconciles/validates/opens PRs under --apply; --root reconciles a local checkout with no network - seed downstream.yml with the 16 public consumer repos (react_on_rails is the reference seam; private and archived repos are out of scope) - wire bin/push-downstream-test.rb and a registry dry-run into bin/validate - document the workflow in docs/downstream-sync.md and the README Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 32 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughAdds ChangesDownstream Seam Sync Tool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
bin/push-downstream-test.rb (1)
237-257: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one CLI-level case for the remaining registry flags.
This suite proves
select_reposdirectly, but the subprocess path still only exercises the default registry flow. A small end-to-end case for--onlyand the disabled-repo inclusion flag added in this PR would catch option-parsing/wiring regressions that the helper test cannot.🤖 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/push-downstream-test.rb` around lines 237 - 257, Add a CLI-level subprocess test in push-downstream-test.rb to cover the remaining registry flags, since the current end-to-end coverage around run_cli only exercises the default registry flow. Extend the existing test helpers around run_cli/select_repos with one case that invokes the CLI using --only and another that includes disabled repos via the new flag added in this PR, then assert the expected repos are planned or included. Keep the test focused on the option-wiring path so regressions in argument parsing are caught even when select_repos unit tests still pass.
🤖 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 `@bin/push-downstream`:
- Around line 120-143: run_local currently reads and writes AGENTS.md with
locale-dependent File.read/File.write before AgentWorkflowSeamDoctor.check runs,
so it can break on non-ASCII content. Update this path to use the same
UTF-8-safe IO approach as AgentWorkflowSeamDoctor (including the reconciliation
flow around reconcile and the write after apply), so the CLI handles AGENTS.md
consistently without depending on the system locale.
In `@bin/push-downstream-test.rb`:
- Line 121: The subprocess invocation in push-downstream-test currently
hardcodes "ruby", which can use a different interpreter than the one running the
suite. Update the Open3.capture2e call in bin/push-downstream-test.rb to use
RbConfig.ruby, and make sure the other subprocess call site in the same script
uses the same interpreter source so both runs are consistent.
---
Nitpick comments:
In `@bin/push-downstream-test.rb`:
- Around line 237-257: Add a CLI-level subprocess test in
push-downstream-test.rb to cover the remaining registry flags, since the current
end-to-end coverage around run_cli only exercises the default registry flow.
Extend the existing test helpers around run_cli/select_repos with one case that
invokes the CLI using --only and another that includes disabled repos via the
new flag added in this PR, then assert the expected repos are planned or
included. Keep the test focused on the option-wiring path so regressions in
argument parsing are caught even when select_repos unit tests still pass.
🪄 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
Run ID: ed7bd92f-00d7-4065-a67d-96c0f1722d3f
📒 Files selected for processing (6)
README.mdbin/push-downstreambin/push-downstream-test.rbbin/validatedocs/downstream-sync.mddownstream.yml
Adopt repos that have no AGENTS.md yet by creating a minimal one (title plus the managed seam) instead of erroring. Both --root mode and registry --apply now create-or-update via a shared reconcile_agents helper; --root also fails cleanly when the target directory does not exist. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Real AGENTS.md files carry non-ASCII bytes (em dashes, arrows). Under a non-UTF-8
default external encoding (e.g. LANG=C, common in headless agents), File.read
returned a US-ASCII string and crashed match? on the first non-ASCII line. Read
via binread + force_encoding("UTF-8").scrub, matching agent-workflow-seam-doctor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seed seam values through defaults -> preset -> per-repo overrides so the fan-out renders complete, agent-optimized seams instead of n/a scaffolds. - seam-presets.yml: org-uniform defaults + archetype presets (ts-package, ruby-gem, ror-demo, site) - downstream.yml: per-repo `preset:` + optional `overrides:` (RSC carries the NODE_CONDITIONS override and is disabled pending its canary PR) - resolve_values layers the three sources and seeds the base branch; unknown presets fail before any writes - reconcile(..., seed:) fills unset keys from the seed while repo-owned values still win, so re-runs stay idempotent - registry plan shows the chosen preset; --presets selects the preset file Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bin/push-downstream (1)
203-215: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKey resolved seeds by full repo identity.
repo[:repo]can collide across owners, while the rest of the workflow treatsrepo[:nwo]as the unique target. Usenwofor the seed map to avoid applying another repo’s overrides if a future registry entry reuses a repo name.Proposed fix
- seeds = repos.to_h { |repo| [repo[:repo], resolve_values(repo, presets)] } + seeds = repos.to_h { |repo| [repo[:nwo], resolve_values(repo, presets)] } @@ - failures = repos.count { |repo| !sync_repo(repo, seeds[repo[:repo]]) } + failures = repos.count { |repo| !sync_repo(repo, seeds.fetch(repo[:nwo])) }🤖 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/push-downstream` around lines 203 - 215, The seed map is keyed by repo name only, which can collide across owners and cause the wrong resolved values to be applied. Update the seeds hash in push-downstream to use the unique repo identity already used elsewhere in the flow, specifically repo[:nwo], and make sure the later sync_repo lookup reads from the same key so each target repo gets its own resolved overrides.
🤖 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 `@bin/push-downstream`:
- Around line 161-163: The early return in the reconciliation flow is skipping
validation, so an existing current path can succeed without running
AgentWorkflowSeamDoctor.check. Update the logic around the existing/updated
checks in push-downstream so that AgentWorkflowSeamDoctor.check still runs
before returning success, even when the seam is already current, and make the
same adjustment in the other matching early-return block referenced in the
comment. Keep the return behavior, but ensure validation is always executed for
the existing agents_path/current state path.
---
Nitpick comments:
In `@bin/push-downstream`:
- Around line 203-215: The seed map is keyed by repo name only, which can
collide across owners and cause the wrong resolved values to be applied. Update
the seeds hash in push-downstream to use the unique repo identity already used
elsewhere in the flow, specifically repo[:nwo], and make sure the later
sync_repo lookup reads from the same key so each target repo gets its own
resolved overrides.
🪄 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
Run ID: 22f4c7b7-6afa-45c8-9c02-ddb115500cff
📒 Files selected for processing (6)
README.mdbin/push-downstreambin/push-downstream-test.rbdocs/downstream-sync.mddownstream.ymlseam-presets.yml
✅ Files skipped from review due to trivial changes (2)
- README.md
- docs/downstream-sync.md
🚧 Files skipped from review as they are similar to previous changes (1)
- downstream.yml
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 `@bin/push-downstream-test.rb`:
- Around line 440-481: The existing-sync-branch tests in PushDownstream do not
verify the remote branch is visible before exercising the rerun/update path, so
they can accidentally pass through the new-branch path. In
test_registry_apply_updates_existing_remote_sync_branch, and the similar
open_pull_request/sync_clone scenarios noted by the comment, add an explicit
precondition assertion using PushDownstream.fetch_remote_branch_head(clone,
branch) (or equivalent) before calling the method under test. Keep the assertion
close to the setup so the test clearly fails if branch visibility/regression
breaks.
🪄 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
Run ID: 9cf6f365-324f-40dd-b3d0-425c6667147c
📒 Files selected for processing (2)
bin/push-downstreambin/push-downstream-test.rb
🚧 Files skipped from review as they are similar to previous changes (1)
- bin/push-downstream
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/push-downstream (1)
447-459: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKey contracts by full repo identity.
contractsis keyed byrepo[:repo], so two entries likeowner-a/appandowner-b/appwill collide and the later contract can be pushed into the wrong downstream repo. Userepo[:nwo]here, or pass the resolved contract directly alongside each repo entry.Suggested fix
- contracts = repos.to_h { |repo| [repo[:repo], resolve_contract(repo, presets)] } + contracts = repos.to_h { |repo| [repo[:nwo], resolve_contract(repo, presets)] } @@ - failures = repos.count { |repo| !sync_repo(repo, contracts.fetch(repo[:repo])) } + failures = repos.count { |repo| !sync_repo(repo, contracts.fetch(repo[:nwo])) }🤖 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/push-downstream` around lines 447 - 459, The contract map in push-downstream is keyed by the short repo name, which can collide across different owners and send the wrong contract to sync_repo. Update the contracts hash to use the full repo identity from repo[:nwo], or otherwise carry the resolved contract on each repo entry so sync_repo gets the matching contract for that specific repository.
♻️ Duplicate comments (2)
bin/push-downstream (1)
478-489: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate the unchanged remote sync branch before reusing its PR.
When the branch already exists remotely and reconciliation is a no-op, this goes straight to
ensure_pull_requestwithout runningAgentWorkflowSeamDoctor.check. An invalid preserved script or malformed policy can therefore get a fresh/reused PR even though the seam is broken.Suggested fix
result = reconcile_scaffold(clone, contract) + issues = AgentWorkflowSeamDoctor.check(clone) unless result.changed? - return ensure_pull_request(repo, result.follow_ups) if branch_state == :existing_remote + if branch_state == :existing_remote + unless issues.empty? + warn "FAIL #{repo[:nwo]}: seam doctor: #{issues.join('; ')}" + return false + end + return ensure_pull_request(repo, result.follow_ups) + end puts "UP_TO_DATE #{repo[:nwo]}" return true end - - issues = AgentWorkflowSeamDoctor.check(clone)🤖 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/push-downstream` around lines 478 - 489, The no-op remote sync path in push-downstream bypasses AgentWorkflowSeamDoctor.check before calling ensure_pull_request, so validate the existing remote branch/script state first. Update the reconcile_scaffold flow so the :existing_remote branch runs the seam doctor check before reusing or creating the PR, and only call ensure_pull_request when issues is empty; otherwise warn and return false just like the changed branch path.bin/push-downstream-test.rb (1)
199-200: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse
RbConfig.rubyfor the seam-doctor subprocesses too.These call sites still invoke
"ruby"directly while the CLI helper already usesRbConfig.ruby. On machines with multiple Rubies onPATH, the suite can exercisebin/push-downstreamandbin/agent-workflow-seam-doctorunder different interpreters.Suggested fix
- out, status = Open3.capture2e("ruby", DOCTOR, "--root", root) + out, status = Open3.capture2e(RbConfig.ruby, DOCTOR, "--root", root) @@ - out, status = Open3.capture2e("ruby", DOCTOR, "--root", root) + out, status = Open3.capture2e(RbConfig.ruby, DOCTOR, "--root", root) @@ - out, status = Open3.capture2e("ruby", DOCTOR, "--root", root) + out, status = Open3.capture2e(RbConfig.ruby, DOCTOR, "--root", root)Also applies to: 276-277, 414-417
🤖 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/push-downstream-test.rb` around lines 199 - 200, The seam-doctor subprocess invocation still hardcodes "ruby", which can run a different interpreter than the rest of the test helper. Update the affected Open3.capture2e call sites in the push-downstream test to use RbConfig.ruby, matching the existing CLI helper approach, and apply the same change to the other seam-doctor-related invocations referenced in this test file so all subprocesses run under the same Ruby.
🧹 Nitpick comments (1)
bin/install-agent-workflows-test.bash (1)
254-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the zero-consumer regression case.
This new test still exercises the non-empty
--consumer-rootpath, but the behavior change inbin/upgrade-agent-workflowsis the empty-consumer case. A regression that runsupgrade-agent-workflowswithout any--consumer-rootwould lock in the fix and catch the old false failure if this guard is removed later.🤖 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/install-agent-workflows-test.bash` around lines 254 - 273, The current test in test_upgrade_validates_consumer_root_after_install only covers a non-empty consumer root, so it misses the regression in upgrade-agent-workflows when no consumers exist. Add a zero-consumer case in this test file by invoking upgrade-agent-workflows without --consumer-root (or with an empty consumer root setup) and assert the upgrade still succeeds with UPGRADE_COMPLETE, while keeping the existing positive path around write_consumer_agents for coverage.
🤖 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 `@bin/push-downstream`:
- Around line 239-247: The run_lines heuristic is too broad because it only
excludes a few shell operators, so complex commands like pipelines, redirects,
and boolean chains still get wrapped in exec and can break. Update run_lines to
treat exec as the fast path only for a plain single command in the
push-downstream script, using the existing run_lines method to detect and
preserve any command containing shell operators or other non-simple syntax, and
only prepend exec when the command is truly a simple standalone invocation.
In `@docs/seam-design.md`:
- Around line 30-42: Update the policy-source description in this section so it
no longer labels AGENTS.md as the canonical policy; instead, align the wording
with the rest of the doc by making .agents/agent-workflow.yml the workflow
policy source and AGENTS.md a pointer-only file. Use the existing references to
AGENTS.md, CLAUDE.md, and .agents/agent-workflow.yml to keep the repo map
consistent and remove the contradiction.
In `@skills/qa-stress/SKILL.md`:
- Around line 190-194: Update the QA stress contract input resolution to read
trusted inputs from the .agents/ sources instead of only AGENTS.md. In the
SKILL.md guidance that describes extracting inputs and running the trust gate,
reference the actual contract files under .agents/bin/ and
.agents/agent-workflow.yml so validation and execution use the same source of
truth, and make sure the step names tied to the trust gate and scope resolution
reflect that change.
In `@workflows/pr-processing.md`:
- Around line 587-588: The goal template now sources <base-branch> from
.agents/agent-workflow.yml, but the later instruction in pr-processing.md and
the mirrored text in skills/pr-batch/SKILL.md still tells workers to use
AGENTS.md, which conflicts with the new seam. Update the later sentence in both
places to reference the same base-branch source used earlier, and make sure the
wording consistently points to .agents/agent-workflow.yml rather than AGENTS.md.
---
Outside diff comments:
In `@bin/push-downstream`:
- Around line 447-459: The contract map in push-downstream is keyed by the short
repo name, which can collide across different owners and send the wrong contract
to sync_repo. Update the contracts hash to use the full repo identity from
repo[:nwo], or otherwise carry the resolved contract on each repo entry so
sync_repo gets the matching contract for that specific repository.
---
Duplicate comments:
In `@bin/push-downstream`:
- Around line 478-489: The no-op remote sync path in push-downstream bypasses
AgentWorkflowSeamDoctor.check before calling ensure_pull_request, so validate
the existing remote branch/script state first. Update the reconcile_scaffold
flow so the :existing_remote branch runs the seam doctor check before reusing or
creating the PR, and only call ensure_pull_request when issues is empty;
otherwise warn and return false just like the changed branch path.
In `@bin/push-downstream-test.rb`:
- Around line 199-200: The seam-doctor subprocess invocation still hardcodes
"ruby", which can run a different interpreter than the rest of the test helper.
Update the affected Open3.capture2e call sites in the push-downstream test to
use RbConfig.ruby, matching the existing CLI helper approach, and apply the same
change to the other seam-doctor-related invocations referenced in this test file
so all subprocesses run under the same Ruby.
---
Nitpick comments:
In `@bin/install-agent-workflows-test.bash`:
- Around line 254-273: The current test in
test_upgrade_validates_consumer_root_after_install only covers a non-empty
consumer root, so it misses the regression in upgrade-agent-workflows when no
consumers exist. Add a zero-consumer case in this test file by invoking
upgrade-agent-workflows without --consumer-root (or with an empty consumer root
setup) and assert the upgrade still succeeds with UPGRADE_COMPLETE, while
keeping the existing positive path around write_consumer_agents for coverage.
🪄 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
Run ID: a2e04f52-5d02-49ac-b9c6-a877067bab54
📒 Files selected for processing (35)
README.mdbin/agent-workflow-seam-doctorbin/agent-workflow-seam-doctor-test.rbbin/install-agent-workflows-test.bashbin/push-downstreambin/push-downstream-test.rbbin/upgrade-agent-workflowsdocs/adoption.mddocs/coordination-backend.mddocs/downstream-sync.mddocs/installation-and-upgrades.mddocs/seam-design.mddownstream.ymlseam-presets.ymlskills/address-review/SKILL.mdskills/autoreview/SKILL.mdskills/plan-pr-batch/SKILL.mdskills/post-merge-audit/SKILL.mdskills/pr-batch/SKILL.mdskills/qa-stress/SKILL.mdskills/replicate-ci/SKILL.mdskills/run-ci/SKILL.mdskills/tdd/SKILL.mdskills/update-changelog/SKILL.mdskills/verify-pr-fix/SKILL.mdskills/verify/SKILL.mdtest/fixtures/consumer-repo/.agents/agent-workflow.ymltest/fixtures/consumer-repo/.agents/bin/README.mdtest/fixtures/consumer-repo/.agents/bin/testtest/fixtures/consumer-repo/.agents/bin/validatetest/fixtures/consumer-repo/AGENTS.mdworkflows/address-review.mdworkflows/continuous-evaluation-loop.mdworkflows/pr-processing.mdworkflows/tdd.md
✅ Files skipped from review due to trivial changes (6)
- docs/coordination-backend.md
- test/fixtures/consumer-repo/.agents/bin/README.md
- docs/installation-and-upgrades.md
- workflows/continuous-evaluation-loop.md
- skills/verify-pr-fix/SKILL.md
- docs/downstream-sync.md
Summary
Adds
bin/push-downstreamanddownstream.ymlso the managed## Agent Workflow Configurationseam can be rolled into consumer repos as onePR per repo, while each repo's seam values stay repo-owned. This is the
repeatable, version-controlled form of consumer adoption (see
docs/adoption.md); it never copies skill or workflow content into a repo.AgentWorkflowSeamDoctor::REQUIRED_KEYSandits parser, so the command and the seam doctor can never drift.
n/a-filled seam whenabsent; preserves existing values (including multi-line) and extra optional
keys when present. Re-running an already-current repo is a no-op.
--applyclones, reconciles,validates with the seam doctor, pushes
agent-workflows/seam-sync, and opensone PR per repo.
--root DIRreconciles a local checkout with no network.downstream.ymlseeds the 16 public consumer repos.react_on_railsis intentionally excluded as the reference seam; private andarchived repos are out of scope.
Validation
bin/validate— full gate green (unit tests, installer/seam tests, RuboCop 1.87.0)ruby bin/push-downstream-test.rb— 12 tests, 97 assertionsbin/push-downstream --config downstream.yml— plans all 16 targets🤖 Generated with Claude Code
Summary by CodeRabbit
push-downstreamCLI to sync the agent-workflow binstub/policy contract into downstream repos using registry config (downstream.yml+ presets), with plan-only output, canary (--only), full rollout (--all), and local--rootmode, creating or reusing PRs as needed.docs/downstream-sync.md..agents/agent-workflow.ymlas the canonical configuration source (instead ofAGENTS.md).push-downstreamunit tests and updated fixtures.bin/validateto runpush-downstreamtests and a downstream registry dry-run.