Add one-command consumer seam initialization

[justin808]

Summary

• add agent-workflow-seam-doctor --init as the one-command consumer seam initializer
• detect executable root bin/validate and bin/test, or exact JavaScript scripts with one unambiguous package-manager lockfile
• generate fail-closed wrappers with precise override guidance when commands cannot be detected safely
• preserve valid existing seam files, keep initialization idempotent, set the base branch for new policy, and update init-managed wrappers through explicit command pairs
• advertise the flow through installer output, adoption/installation/seam docs, README, and the Unreleased changelog

Closes #99

Validation

• ruby bin/agent-workflow-seam-doctor-test.rb (109 runs, 529 assertions)
• bash bin/install-agent-workflows-test.bash
• ruby bin/push-downstream-test.rb (45 runs, 240 assertions)
• bin/validate
• manual explicit-command PASS, byte-stable idempotent rerun, and unknown-repository fail-closed smoke checks
• TDD regressions cover conservative JavaScript detection, validation-before-write, explicit managed-wrapper updates, repo-owned-wrapper conflicts, bare-init preservation, README accuracy, and scoped argument forwarding

The local independent codex review gate was attempted twice but was unavailable: the default review model requires a newer CLI, and the compatible-model retry hit the local usage limit. Hosted current-head review is therefore required before merge.

Decisions

• Non-blocking: how much repository command detection should init perform?
  • Decision: use conservative detection for executable root binstubs and exact JavaScript scripts with exactly one lockfile; otherwise fail closed and require both explicit commands.
  • Why: this provides a useful one-command path without guessing ambiguous consumer tooling.
  • Review later: expand detection only with tested, portable repository patterns.
• Non-blocking: should this lane add or change seam presets and downstream-maintainer tooling?
  • Decision: no. The live base already contained seam-presets.yml; this PR does not edit it, bin/push-downstream, examples, or fixtures.
  • Why: those paths were outside the discovered One-command seam init scaffold for consumer repos #99 path reservation and would collide with separate concerns.
  • Review later: none.

QA Evidence

• QA lane: m1-qa-seam-init, separate worktree; completed Sol-worker matrix reconciled by the parent coordinator on the exact final head
• Scope checked: installed copy and symlink modes, generated consumer seams, package-runner forwarding, preservation/idempotence, and fail-closed/error paths
• Tested at: a2e136793bd337d605a1f77c09486d08a336a3d2
• Automated checks: exact-head doctor suite (109 runs, 529 assertions), installer suite, bin/validate, hosted validate, and hosted Claude review all passed
• Manual checks: full installed copy/symlink and consumer-seam matrix passed at 159c24a; the only final delta was inspected and replayed at a2e1367, including caller-controlled env -S/--split-string preservation
• Findings: no open product finding; all 22 P2 findings have fixed dispositions and zero review threads remain unresolved
• QA required: yes
• QA required rationale: installer/seam behavior changes the developer workflow and requires an independent installed-flow exercise
• QA lane status: satisfied
• Release-blocking status: clear
• Process-gap disposition: checklist+replay

[coderabbitai]

Walkthrough

Adds --init support to agent-workflow-seam-doctor for creating and validating consumer repository seams, including wrappers, policy, trust configuration, and AGENTS.md integration. Installer output, tests, README content, and adoption documentation now describe the initialization workflow.

Changes

Consumer seam initialization

Layer / File(s)	Summary
Initialization engine and CLI bin/agent-workflow-seam-doctor	Adds --init, command detection and validation, managed wrapper generation, policy/trust updates, AGENTS.md reconciliation, fail-closed reporting, and post-init contract checks.
Initialization coverage bin/agent-workflow-seam-doctor-test.rb, bin/install-agent-workflows-test.bash	Adds end-to-end coverage for initialization, command forwarding, detection, failure handling, YAML behavior, installation, reporting, and idempotency.
Installation and adoption guidance bin/install-agent-workflows, README.md, docs/*.md, CHANGELOG.md	Adds installer guidance and documents the initialization command, detection rules, managed wrapper behavior, trust setup, and validation workflow.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConsumerRepo
  participant SeamDoctor
  participant PackageMetadata
  ConsumerRepo->>SeamDoctor: Run --init
  SeamDoctor->>ConsumerRepo: Read existing seam files
  SeamDoctor->>PackageMetadata: Detect scripts and lockfile
  PackageMetadata-->>SeamDoctor: Return command candidates
  SeamDoctor->>ConsumerRepo: Write wrappers and configuration
  SeamDoctor->>ConsumerRepo: Validate initialized seam

Loading

Possibly related PRs

• shakacode/agent-workflows#22: Both modify seam contract tooling used to validate or reconcile downstream consumer seams.

🚥 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	The PR implements the requested one-command init, starter trust config, AGENTS pointer, and validation flow for #99.
Out of Scope Changes check	✅ Passed	The README, docs, installer, and tests all support the init workflow and stay within the issue scope.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly captures the main change: adding one-command consumer seam initialization.

✨ 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 codex/99-seam-init

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Review: Add one-command consumer seam initialization (#107)

Overview

Adds agent-workflow-seam-doctor --init, which detects (or accepts explicit) validate/test commands, scaffolds .agents/bin/*, .agents/agent-workflow.yml, .agents/trusted-github-actors.yml, and the AGENTS.md pointer section, then runs the existing check in the same invocation. Detection is intentionally conservative (root bin/validate+bin/test, or exact JS scripts behind exactly one recognized lockfile) and falls back to fail-closed placeholder wrappers otherwise. Docs/README/CHANGELOG/installer output are updated to match, and the test suite (bin/agent-workflow-seam-doctor-test.rb, bin/install-agent-workflows-test.bash) gets solid new coverage for detection, idempotency, validation-before-write, and fail-closed behavior.

Strengths

• Validation happens before any writes (validate_init_base_branch!, validate_init_command_inputs!, YAML/trust mapping checks) — confirmed by tests that assert nothing is created on the filesystem when validation fails.
• Fail-closed default is well designed: ambiguous detection produces a wrapper that explicitly errors with a pointer to the fix, and script_issues is taught to flag the placeholder marker so check won't silently pass.
• Good idempotency test coverage for the "nothing to do" path (test_init_preserves_an_existing_valid_seam_and_is_idempotent, test_bare_init_preserves_previously_generated_valid_wrappers).
• Detection logic is appropriately narrow (rejects directories misnamed bin/validate, ambiguous multi-lockfile repos, blank JS scripts) — avoids the temptation to guess.
• Docs (README/adoption/installation/seam-design) were updated consistently with the new flow.

Correctness issue (posted inline)

write_init_script (bin/agent-workflow-seam-doctor:194-217) returns as soon as the existing wrapper passes script_issues, before ever comparing it to the newly requested command. In practice this means once a valid wrapper exists, re-running --init with different explicit --validate-command/--test-command values never updates it — silently contradicting the PR description's claim to "support explicit base-branch and command overrides." Worse, write_init_readme does regenerate .agents/bin/README.md to reflect the new command in this scenario, so the generated docs and the actual wrapper script end up describing two different commands. See inline comment for a traced repro and a suggested fix (compare/replace instead of short-circuiting on "already valid").

Minor / non-blocking notes

• The "--validate-command and --test-command must be provided together" invariant (bin/agent-workflow-seam-doctor:668-669) is enforced only in the CLI arg-parsing branch, not inside AgentWorkflowSeamDoctor.init itself. Calling init directly (e.g. from other Ruby code or future callers) with only one of the two commands bypasses this check — the result is still safe (the missing command falls back to a fail-closed placeholder), but it'd be more robust to assert the pairing inside init itself rather than relying on every caller to replicate the CLI's check.
• write_init_readme's embedded command strings are inserted directly into a Markdown table cell; a command containing a literal | (a real possibility for compound shell commands) will break the table's column alignment. Cosmetic only.
• write_init_policy/write_init_trust's "preserve existing" semantics mean a later --base-branch override likewise has no effect once .agents/agent-workflow.yml already exists (existing values win in the merge) — this appears to be intentional given "preserve valid existing seam files" is an explicit goal, but is worth calling out since it's easy to conflate with the "override" framing in the PR description.

Security

No significant concerns. Command strings passed via --validate-command/--test-command are embedded verbatim into a wrapper script the consumer repo controls and executes for itself — same trust boundary as a developer hand-authoring the wrapper, and multiline/NUL/empty values are rejected before any write. YAML parsing uses safe_load(aliases: false) consistent with the rest of the file.

Test coverage

Thorough for the shipped surface area (idempotency, detection ambiguity, validation ordering, JSON/text parity, missing-root handling). No test currently exercises re-running --init with different explicit commands over an already-valid wrapper — adding one would have caught the issue above.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20ddd7ced6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Add one-command consumer seam initialization

Overview

This PR adds agent-workflow-seam-doctor --init, a conservative, fail-closed initializer that scaffolds .agents/bin/{validate,test}, .agents/agent-workflow.yml, .agents/trusted-github-actors.yml, and the AGENTS.md pointer section for a consumer repo, then immediately runs check against the result. Detection is intentionally narrow (executable root bin/validate/bin/test, or exact validate/test npm/pnpm/yarn scripts with exactly one lockfile) and falls back to clearly-marked fail-closed wrappers otherwise. Docs (README, adoption guide, install/upgrade guide, seam-design doc, CHANGELOG) and the installer's printed instructions are updated to advertise the new flow, and a large, well-organized test suite (CLI + library-level) covers idempotency, validation ordering, detection, and JSON/text parity.

Strengths

• Validation is front-loaded: all input checks (validate_init_command_pair!, validate_init_base_branch!, validate_init_command_inputs!, YAML mapping/trust validation) run before any file is touched, and tests explicitly assert nothing is written on rejected input.
• write_if_changed + INIT_MARKER gives byte-stable idempotent reruns and protects hand-authored files from being clobbered by a later --init run.
• Detection is appropriately conservative (all-or-nothing on root binstubs, requires exactly one lockfile, rejects blank JS scripts) rather than guessing.
• Good breadth of tests, including the "does not guess when ambiguous" and "blank scripts not detected" TDD-style regressions called out in the PR description.

Issues found (posted as inline comments)

1. Explicit --validate-command/--test-command can be silently ignored (write_init_script, bin/agent-workflow-seam-doctor:201-224) — if a pre-existing, hand-authored (non-INIT_MARKER) wrapper already sits at .agents/bin/validate/test, the explicit override is dropped with no warning, and the subsequent check reports a false PASS. Only "replace a previously init-generated wrapper" is tested; "override a pre-existing hand-authored wrapper" is not.
2. Generated README always shows n/a for optional scripts (write_init_readme, bin/agent-workflow-seam-doctor:234-268) — commands only ever carries validate/test, so setup/lint/build/docs/ci-detect are hardcoded to "n/a" regardless of whether those wrappers actually exist in .agents/bin/, which can misdocument a repo that already has working optional scripts.
3. Minor: detect_javascript_commands (bin/agent-workflow-seam-doctor:158-179) only rescues JSON::ParserError; a syntactically valid but non-object package.json (e.g. top-level array) raises an unrescued TypeError that crashes the whole script instead of failing closed gracefully.

Security

No significant concerns — the tool writes shell commands the invoking maintainer supplies themselves (not derived from untrusted external input), and YAML is loaded via YAML.safe_load(aliases: false).

Suggestion

Consider adding test coverage for (1) — explicit command override vs. a pre-existing non-managed wrapper — since it's the one scenario the docs promise ("complete the seam in one run") but the code doesn't fully deliver.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff26d6bd90

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Add one-command consumer seam initialization

Overview

This PR adds agent-workflow-seam-doctor --init, a new fail-closed initializer that scaffolds .agents/bin/{validate,test} wrappers, .agents/bin/README.md, .agents/agent-workflow.yml, .agents/trusted-github-actors.yml, and the AGENTS.md pointer section for a consumer repo in one command — conservatively detecting executable root bin/validate/bin/test or exact npm/pnpm/yarn validate/test scripts when exactly one lockfile disambiguates the package manager, and otherwise writing marked, fail-closed placeholder wrappers. Docs (README, adoption.md, installation-and-upgrades.md, seam-design.md) and the changelog are updated to advertise the new flow, and the test suite gets ~30 new cases covering idempotency, explicit-command overrides, ambiguous-detection fail-closed behavior, and installer integration.

Strengths

• Very thorough test coverage for the happy paths: idempotent reruns, explicit-command precedence over detection, ambiguous JS lockfiles correctly refusing to guess, invalid YAML rejected before any writes, base-branch validation, NUL/multiline command rejection.
• Good "validate everything before writing anything" discipline in init — all input validation and existing-config parsing happens before FileUtils.mkdir_p/any file writes, so a rejected --init call leaves the repo untouched (verified by several tests).
• Detection is appropriately conservative (single unambiguous lockfile, non-blank scripts, executable root binstubs only) rather than guessing.
• The INIT_MARKER/init_managed_file? mechanism to distinguish tool-generated files from repo-owned ones is a solid design for safe idempotent reruns.

Issues found (posted as inline comments)

1. Correctness / robustness — detect_javascript_commands only rescues JSON::ParserError, but a syntactically-valid, non-object package.json (e.g. [], null, true, a bare number) makes package["scripts"] raise TypeError/NoMethodError. That's not caught anywhere (the CLI dispatcher only rescues InitError), so --init crashes with a raw backtrace instead of the documented fail-closed FAIL, for a plausible corrupted/placeholder input.
2. Correctness — write_init_readme's generated table only consults the commands hash (validate/test from detection or explicit flags), not the actual on-disk state. A preserved, hand-written .agents/bin/validate/test gets mislabeled "unconfigured (fail-closed)", and any pre-existing optional script (setup, lint, build, docs, ci-detect) is always shown as n/a regardless of whether it exists — even though check() can still report PASS.
3. UX / correctness gap — write_init_script silently ignores an explicit --validate-command/--test-command when a non-init-managed wrapper already exists at that path, with no warning, and check still reports PASS. This contradicts the PR's own docs, which imply explicit flags "complete the seam in one run."

None of these are security issues (all inputs are supplied by the local repo maintainer running their own tooling, not attacker-controlled), and the core detection/idempotency/validation-ordering logic is solid and well-tested. The three issues above are edge cases worth tightening before merge, particularly #1 (crash) and #2 (misleading generated docs).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ad08413fad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review Summary

Reviewed the agent-workflow-seam-doctor --init addition (bin/agent-workflow-seam-doctor + tests, docs, install script, README).

Overview

This adds a one-command consumer seam initializer that conservatively detects validate/test commands (root bin/ binstubs or unambiguous JS package scripts), writes fail-closed wrappers when detection isn't safe, and preserves any repo-owned content. It's backed by an extensive Minitest suite (76 runs / 320 assertions per the PR description) covering idempotency, conflict detection, validation-before-write ordering, and CLI/API parity.

Strengths

• Validate-before-write discipline: init validates YAML/base-branch/command inputs and checks for repo-owned wrapper conflicts before any FileUtils.mkdir_p/File.write call — confirmed by tests asserting no .agents directory is created on failure paths.
• Fail-closed by default: ambiguous detection (multiple lockfiles, partial JS scripts, non-object package.json) correctly falls back to placeholder wrappers marked with INIT_PLACEHOLDER_MARKER, which script_issues surfaces as an explicit doctor failure rather than silently succeeding.
• Ownership tracking via INIT_MARKER: managed vs. repo-owned files are distinguished by a marker string, so reruns don't clobber hand-edited wrappers/README/AGENTS.md sections — this is exercised well by the idempotency tests.
• Docs (README, adoption.md, installation-and-upgrades.md, seam-design.md) and CHANGELOG are updated consistently with the new flow.

Issues found (posted inline)

1. Unhandled encoding exception risk in detect_javascript_commands (bin/agent-workflow-seam-doctor:173) — reads package.json via File.read(..., encoding: "UTF-8") and only rescues JSON::ParserError. A package.json with invalid UTF-8 bytes (e.g. a stray BOM) can raise an uncaught encoding error instead of failing closed cleanly, unlike the rest of the file which consistently uses binread(...).force_encoding("UTF-8").scrub for exactly this reason.
2. Explicit npm run <script> commands may not forward args correctly — init_command_line (bin/agent-workflow-seam-doctor:245) appends "$@" directly without an npm run-style -- separator when the caller doesn't already include one. The auto-detected JS path correctly includes -- "$@", but an explicit --validate-command 'npm run validate' override does not, and this combination isn't covered by the test suite.

Minor / non-blocking

• write_init_script's permission-only fix path (File.chmod(0o755, path) unless File.executable?(path)) doesn't count toward scripts_changed, so a chmod-only repair on an already-managed script won't trigger a README regen. Very low impact, likely not worth a fix on its own.
• Security: writing generated shell wrappers from user-supplied --validate-command/--test-command strings is expected/local-trust-boundary behavior (equivalent to hand-editing a repo config file) — no injection concern since this is not attacker-controlled input.

Overall this is a solid, well-tested addition; the two inline findings are worth addressing but neither is a blocking correctness issue for the documented conservative-detection use cases.

[claude]

Review: Add one-command consumer seam initialization

Overview

Adds agent-workflow-seam-doctor --init, which scaffolds a consumer repo's .agents/ seam (command wrappers, agent-workflow.yml, trusted-github-actors.yml, AGENTS.md pointer section) and immediately runs the existing check validator. Detection is intentionally conservative: only executable root bin/validate+bin/test, or exact validate/test npm/pnpm/yarn scripts with exactly one unambiguous lockfile; everything else gets a clearly marked, fail-closed placeholder wrapper. Docs (README, adoption guide, installation guide, seam-design, CHANGELOG) are updated to match.

Code quality

• The write path is carefully staged: all validation (validate_init_command_pair!, validate_init_base_branch!, validate_init_command_inputs!, YAML validity, repo-owned wrapper conflicts) happens before any file is touched, so a rejected --init leaves zero side effects. Verified this against the test suite (test_init_validates_existing_yaml_before_writing, test_init_rejects_*_before_writing, etc.) and it holds up.
• write_init_script/write_init_readme/write_init_policy/write_init_trust all use an "only touch init-managed files, and only when content actually differs" pattern (init_managed_file? + write_if_changed), which gives the idempotent, byte-stable reruns the PR description calls out. I traced several non-obvious interactions (bare re-init after a placeholder becomes detectable, bare re-init preserving an explicit wrapper even when root binstubs later appear) and the logic holds together correctly.
• reconcile_pointer_section (new AGENTS.md pointer-section rewriting) correctly reuses the existing pointer_section_issues/extract_section semantics; traced several cases by hand (no section + no trailing newline, stale section followed by another ## heading, section as the last content in the file) and all converge to a stable, idempotent result.
• Detection logic matches its documented scope: it does not merge partial root/JS signals, does not guess among multiple lockfiles, and treats malformed/non-object/non-UTF8 package.json as "unknown" rather than raising.

Potential issues (minor, left as an inline comment)

• init_command_line silently drops exec + "$@" argument forwarding for any explicit command containing | & ; < >. This is a defensible safety choice for compound commands, but it's undocumented and has no regression test, so a user supplying e.g. --validate-command 'make clean && make validate' would get no argument forwarding with no indication why. Worth a doc note or a test either way.

Security

No remote/untrusted input is executed — package.json is only JSON-parsed (not eval'd) to check for script presence, and explicit commands come from the same local operator running the CLI. Input validation (no NUL/CR/LF, non-empty, pair-must-match) is enforced before any write. I didn't find injection or path-traversal concerns.

Test coverage

Very thorough for a first pass — 320 assertions covering explicit/detected/ambiguous/unknown command paths, conflict-with-repo-owned-wrapper rejection, idempotency, JSON parity with text output, and installer integration. Nice work.

Nothing blocking found.

[claude]

Review: Add one-command consumer seam initialization

Overview

This PR adds agent-workflow-seam-doctor --init, which scaffolds .agents/bin/{validate,test} wrappers, .agents/agent-workflow.yml, .agents/trusted-github-actors.yml, and the AGENTS.md pointer section in one command, with conservative auto-detection (executable root bin/validate/bin/test, or exact npm/pnpm/yarn validate/test scripts when exactly one lockfile is present) and fail-closed placeholder wrappers otherwise. Docs (README, adoption, installation, seam-design) and CHANGELOG are updated to match, and bin/install-agent-workflows-test.bash gets a nice incidental fix (replacing a shared /tmp/install-agent-workflows-test.out path with a per-test $tmp path, improving test isolation).

Code quality

• The validate-before-write ordering is careful and well tested: all fallible checks (InitErrors for bad base branch, bad commands, invalid existing YAML, unsafe YAML merges, wrapper conflicts) run before FileUtils.mkdir_p/any writes, so a failed --init never leaves partial state. Confirmed by walking through init in bin/agent-workflow-seam-doctor:102-133.
• The compound_shell_command?/init_npm_run_command?/init_command_line trio (shell-quote-aware compound detection, npm -- separator insertion, exec/env-assignment handling) is intricate but has strong, explicit test coverage for the documented edge cases (quoting, env/exec prefixes, npm aliases like rum/urn, backtick/pipe escaping in the generated README table).
• prepare_init_mapping's "append missing keys, then re-parse and diff against the intended merge" trick for safely growing existing YAML without clobbering comments/formatting is a clever, well-guarded approach (fails closed on flow-style or otherwise un-appendable YAML rather than guessing).
• Left two inline comments on smaller quality nits: an unreachable require_arrays: true code path in validate_trust_mapping!, and a README-patching branch in write_init_readme that looks redundant with (and slightly riskier than) the full-regeneration path beneath it.

Potential issues / risks

• validate_init_base_branch! shells out to git check-ref-format on every --init call; if git isn't on PATH this raises Errno::ENOENT, which is caught by the top-level SystemCallError rescue but produces a fairly generic message. Low risk given the tool already assumes a git repo elsewhere.
• Detection is intentionally conservative (won't combine a partial bin/validate-only setup, won't guess among multiple JS lockfiles, doesn't attempt Ruby/Rake detection) — this is called out in the docs, so it's a documented trade-off rather than a gap.

Security

• No shell-injection concerns: git check-ref-format is invoked via Open3.capture2e with an argv array (no shell interpolation), and user-supplied commands are only ever written into generated wrapper scripts under the caller's own control (same trust boundary as hand-writing the wrapper).
• The generated .agents/trusted-github-actors.yml defaults to empty/fail-closed, consistent with the stated security posture.

Test coverage

Coverage is unusually thorough for this kind of feature (idempotency, byte-stable reruns, repo-owned-wrapper conflict detection, malformed YAML/JSON/UTF-8 inputs, JSON edge cases like non-object package.json). I didn't find gaps worth calling out beyond the two inline nits.

Overall: solid, carefully tested change. The two inline comments are minor cleanup suggestions, not blockers.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8a2d3fc42

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Add one-command consumer seam initialization

Overview

This PR adds agent-workflow-seam-doctor --init, a one-command initializer that scaffolds a consumer repo's .agents/ seam (command wrappers, policy YAML, trust YAML, AGENTS.md pointer section) and immediately validates it. It's conservative by design: it only auto-detects commands from an executable bin/validate/bin/test pair or from exact validate/test npm/pnpm/yarn scripts gated on exactly one lockfile, and it fails closed (with precise remediation guidance) everywhere else. Existing repo-owned content is preserved via an explicit "init marker" ownership model, and reruns are idempotent. Docs (README.md, docs/adoption.md, docs/installation-and-upgrades.md, docs/seam-design.md) and the installer output are updated accordingly, plus a CHANGELOG.md entry.

Code quality

• The fail-closed philosophy is applied consistently: invalid YAML, ambiguous command detection, base-branch validation, and repo-owned-wrapper conflicts are all validated before any file is written (confirmed by tests asserting no partial writes on failure).
• The "init marker" ownership model (tool-owned vs. repo-owned files) is a clean way to make wrapper regeneration safe without a lockfile/state file.
• prepare_init_mapping's append-missing-keys-then-reparse-and-compare approach to updating existing YAML without disturbing comments/formatting is a nice, safe technique — it fails closed rather than risking a silent semantic change.
• Test coverage is unusually thorough (105 runs / 501 assertions per the PR description) and specifically targets the tricky cases: idempotency, quoting/escaping edge cases, npm/pnpm/yarn argument-forwarding differences, malformed YAML, non-UTF8/non-object package.json, and fenced-markdown-safe AGENTS.md section detection.
• One maintainability concern (left inline): the custom shell/npm-command classification logic (compound_shell_command?, init_npm_run_command?, discard_env_options!, valid_npm_option_prefix?, ~130 lines) reimplements a nontrivial slice of shell/npm CLI parsing by hand. It held up under manual tracing against the test matrix, but it's a meaningful surface for future bugs as npm's CLI evolves (e.g. npx, corepack, new subcommand aliases aren't recognized). The failure mode is benign (falls back to not adding --), so this is a nice-to-have, not a blocker.
• One minor robustness note (left inline): the README "fast path" update uses String#sub on a per-row regex, which is a silent no-op if a row's format ever drifts from what's currently generated — low risk given the marker-ownership model, but worth a defensive check if this code evolves further.

Bugs / correctness

No functional bugs found. I traced through the trickier parts by hand (compound-command detection, npm -- separator insertion, quoting/escaping, base-branch validation via git check-ref-format, YAML key-merging) and the behavior matches both the docs and the test expectations. Validation order (missing root → command pair → base branch → command inputs → existing YAML validity → wrapper conflicts) is applied before any writes, which is the right shape for a tool that mutates a consumer repo's files.

Security

• Generated wrappers embed the caller-supplied --validate-command/--test-command verbatim as shell code — this is by design (the tool is meant to emit a literal shell command), and the only sanitization is rejecting empty/multi-line/NUL-containing input. Since this is a local CLI invoked by a repo maintainer with trusted input (not attacker-controlled/remote), this is appropriate; just worth being aware of if --init were ever wired into an automated/less-trusted invocation path in the future.
• New .agents/trusted-github-actors.yml defaults to empty lists for all trust keys — correct fail-closed default for a security-sensitive trust config.
• validate_init_base_branch! shells out to git check-ref-format via Open3.capture2e with an argv array (not a shell string), so no shell-injection risk there.

Performance

No concerns — this is a one-shot CLI tool operating on small local files; nothing here is hot-path or scales with repo size in a way that matters.

Test coverage

Strong. Only gap I noticed: the install-flow test (test_installed_doctor_initializes_consumer_repo) exercises the happy path end-to-end through the installed binary, but there's no installed-flow test for the fail-closed/unconfigured-wrapper path. Given the PR body notes independent installed-flow QA is still pending, that may be intentionally left to the QA lane rather than this PR's unit tests.

Nice, minor, unrelated-but-welcome fix: bin/install-agent-workflows-test.bash switching from a shared /tmp/install-agent-workflows-test.out to a per-test $tmp/... path removes a latent cross-test/cross-run collision risk.

Overall this is a well-scoped, carefully validated change with strong fail-closed defaults and excellent test coverage. The two inline notes are suggestions for future maintainability, not blockers.

[claude]

Review: Add one-command consumer seam initialization

Overview

This adds agent-workflow-seam-doctor --init, a one-command initializer for consumer repos: it detects/generates .agents/bin/{validate,test} wrappers (executable root bin/validate+bin/test, or exact npm/pnpm/yarn scripts when exactly one lockfile is present), creates fail-closed placeholders when detection is ambiguous, seeds .agents/agent-workflow.yml and a now-empty .agents/trusted-github-actors.yml, patches the AGENTS.md pointer section, and re-runs check() immediately after writing. Docs (README, adoption, installation, seam-design) and the changelog are updated to match, and bin/install-agent-workflows's test suite gets a real bug fix along the way (deduplicating a shared /tmp/install-agent-workflows-test.out path across tests into per-test tmp files, removing a source of parallel-test flakiness).

Code quality

• The write path is careful: every validation that can raise (InitError for command pairing, base-branch format, YAML safety, trust-role overlap, repo-owned-wrapper conflicts) happens before any file is touched, so failed --init runs leave the repo untouched — this is well covered by tests (refute File.exist?(...) assertions after failure paths).
• init_managed_file? (marker-based ownership) is a clean way to distinguish tool-owned vs. repo-owned wrappers, and the fail-closed-by-default posture (unconfigured commands become a placeholder script that exits 1 with actionable guidance) matches the stated design goal well.
• prepare_init_mapping's "append missing keys, then re-parse and diff against the intended merge" trick for YAML is a nice way to preserve comments/formatting while still failing closed if the append can't be represented safely (e.g. flow-style top-level mappings) — good defensive design.
• Test coverage is unusually thorough for the tricky parts (npm/pnpm/yarn argument-forwarding separator logic, env/exec/assignment prefixes, idempotency, fenced-heading-aware AGENTS.md patching).

Issues found

• Bash syntax edge case in compound_shell_command? (bin/agent-workflow-seam-doctor): left an inline comment. The compound-command detector only recognizes | & ; < > and #-at-token-start; it doesn't treat bare (/) as special. A command like --validate-command '(bin/validate)' (no other operator) is misclassified as "simple," producing exec (bin/validate) "$@" in the generated wrapper — a bash syntax error. Since the doctor never executes wrappers, --init would report success for a broken wrapper. Narrow trigger, but worth a fix or an explicit test documenting the limitation.

Minor / non-blocking

• init_npm_command_info returns false in a couple of early-out branches and nil in others as its "not npm" sentinel. Functionally identical (both falsy), but worth normalizing to one sentinel for consistency.
• No test exercises calling --init twice with identical explicit --validate-command/--test-command values to confirm byte-for-byte idempotency (the existing idempotency tests cover bare --init reruns and different explicit commands, not same-value reruns). The PR description says this was checked manually; a regression test would lock it in.

Security

No injection concerns: user-supplied commands are only ever written into locally-generated script files by the same operator who invoked the CLI (no remote/untrusted input), and the base-branch validation shells out to git check-ref-format via Open3.capture2e with an argv array (no shell interpolation).

Overall this is a well-scoped, heavily-tested change that matches its stated design goals; the one correctness gap above is narrow but worth addressing before merge given the "fail-closed" promise the tool makes elsewhere.

[claude]

Review: Add one-command consumer seam initialization (#107)

Overview
This adds agent-workflow-seam-doctor --init, a conservative, fail-closed scaffolder for consumer-repo seams (command wrappers, policy YAML, trust YAML, AGENTS.md pointer section), plus matching docs/README/changelog updates and a large, well-organized test suite (bin/agent-workflow-seam-doctor-test.rb).

Code quality

• The implementation is methodically decomposed (detect_init_commands, write_init_script, prepare_init_mapping, reconcile_pointer_section, etc.) and each helper is narrowly scoped and testable.
• Fail-closed-before-write ordering is consistent: base branch, command shape, trust-mapping overlap, and wrapper-conflict checks all run and can raise before any file is touched (verified by tests like test_init_fails_before_writing_when_existing_yaml_cannot_be_safely_appended and test_init_rejects_explicit_commands_that_would_replace_repo_owned_wrappers).
• Idempotency is explicitly tested for both bare and explicit-command init (test_init_preserves_an_existing_valid_seam_and_is_idempotent, test_bare_init_preserves_previously_generated_valid_wrappers), including a mode-only repair path (test_bare_init_restores_managed_wrapper_mode_without_rewriting_content).
• Docs (README.md, docs/adoption.md, docs/installation-and-upgrades.md, docs/seam-design.md) are updated consistently with the new flags actual behavior.
• Nice incidental fix: bin/install-agent-workflows-test.bash switches several tests from a shared /tmp/install-agent-workflows-test.out path to a per-test $tmp/... path, removing a latent test-isolation race.

Potential issues (minor, non-blocking)

1. compound_shell_command? (bin/agent-workflow-seam-doctor:300) treats pipe, ampersand, semicolon, redirects, and parens as compound but not backtick command substitution. A command that is only a backtick substitution will not be classified as compound and will fall through to automatic "$@" appending in init_command_line, which is untested and could append arguments in a surprising place relative to the substitution. Likely rare in practice, but worth either treating backticks as compound too, or adding a test documenting the intended behavior.
2. validate_init_base_branch! (bin/agent-workflow-seam-doctor:145) shells out to git check-ref-format. The CLI entry point rescues SystemCallError alongside InitError, so a missing git binary degrades gracefully there, but AgentWorkflowSeamDoctor.init called directly as a library API (as several tests do) would raise an uncaught Errno::ENOENT instead of the documented InitError if git is not on PATH. Minor robustness gap for non-CLI callers.

Security

• Good practice: YAML.safe_load(..., aliases: false) is used consistently for both policy and trust config parsing, avoiding alias/anchor-based YAML expansion issues.
• Command/base-branch validation rejects NUL bytes and embedded newlines before writing them into generated bash wrappers, which prevents smuggling extra shell statements into the generated script via the CLI flags.
• git check-ref-format is invoked via Open3.capture2e with argv array form (no shell interpolation), so branch-name input cannot reach a shell.
• No new user-facing network or eval surface is introduced.

Performance

• No notable concerns; --init does a handful of stat/read/JSON-parse/YAML-parse calls plus a couple of subprocess invocations (git check-ref-format, bash -n for syntax-checking existing scripts), all bounded and appropriate for a one-shot CLI tool.

Test coverage

• Very thorough: explicit/detected command paths, npm/pnpm/yarn argument-forwarding variants, YAML key-merging edge cases, fenced-heading handling in AGENTS.md, and fail-closed/idempotency guarantees are all covered.

Overall this is a well-tested, carefully sequenced change. The two notes above are low-severity edge cases, not blockers.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 159c24a7ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Add one-command consumer seam initialization (#107)

Overview

This PR adds agent-workflow-seam-doctor --init, a one-shot initializer for the .agents/ consumer seam: it detects (conservatively) or accepts explicit validate/test commands, generates fail-closed wrappers, backfills agent-workflow.yml/trusted-github-actors.yml policy keys without clobbering existing formatting/comments, repairs the AGENTS.md pointer section (fence-aware), and re-runs the existing check validator at the end. Docs (README, adoption, installation, seam-design) and the installer's printed guidance are updated to match, and the change ships with a large, thorough test suite (109 assertions in agent-workflow-seam-doctor-test.rb, plus new installer integration tests).

Code quality

• The new code is well-organized: detection, validation, and writing are cleanly separated into small module_functions, and every failure path raises InitError before any file is written (verified this against the test suite — e.g. invalid existing YAML, overlapping trust roles, and repo-owned-wrapper conflicts all fail closed with nothing touched on disk).
• Shell-command classification (compound_shell_command?, env_split_string_command?, init_npm_command_info) is hand-rolled but careful — quote/escape-aware, and defensively rescues Shellwords::ArgumentError. The npm/pnpm/yarn argument-forwarding logic has clearly been through several rounds of edge-case hardening (leading exec, environment-variable prefixes, env -S/--split-string, run-command aliases, etc.), and I traced through the trickier cases (compound commands, npm run with no script name, caller-supplied "$@") without finding a mismatch against the tests.
• init_command_line and its helpers carry fairly high branching complexity for a single method — not wrong, but a candidate for future decomposition if more command shapes get added.
• Minor inconsistency: init_npm_command_info mixes nil and false as its "not an npm command" sentinel (both falsy, so no functional bug, just a readability nit) — flagged inline.

Potential issues

• Flagged inline: write_init_script is invoked once for validate and once for test in sequence; only filesystem-level errors (not validation errors) can fail this step, but if they do, one wrapper can end up written while the other isn't, leaving the seam partially initialized. Low severity — a rerun of the doctor surfaces the incomplete state — but worth a comment or explicit acknowledgment since the rest of the PR is explicit about "fail closed before writing."
• Detection is intentionally conservative (root bin/validate+bin/test, or exactly one JS lockfile with both scripts present) and falls back to a clearly-marked fail-closed placeholder otherwise — this is the right tradeoff over guessing.

Security

• No shell-injection concerns: the one place a caller-controlled string reaches a subprocess (git check-ref-format --branch <base_branch>) uses Open3.capture2e with argv array form, not shell interpolation.
• Generated wrapper scripts embed the caller-supplied command text verbatim into a bash file that the same operator will execute locally — this is the intended local-tooling use case (akin to a Makefile target), not an externally-reachable injection surface.
• The generated trusted-github-actors.yml is intentionally empty/fail-closed by default, which is the right default for a security-sensitive trust config.

Test coverage

• Coverage is extensive and specifically targets the tricky argument-forwarding/detection edge cases, idempotency (byte-stable reruns), and failure-before-write guarantees. I did not run the suite in this sandbox (no bash execution available), so I verified behavior via careful static trace-through instead of by executing bin/agent-workflow-seam-doctor-test.rb.

Note on PR description

The PR description embeds an unusual amount of process metadata (QA-evidence and "priority-finding-dispositions" HTML comment blocks referencing ~20 previously-fixed review threads). This reads as tooling output from this repo's own review/QA automation rather than a human-authored note, which is plausible given the project's purpose — flagging it only for visibility, not as a concern with the code itself.

Overall

Solid, well-tested, appropriately conservative feature. The two inline notes are minor/non-blocking; nothing here should block merge on its own.

[justin808]

Review triage complete

• Scan scope: full PR history through exact head a2e136793bd337d605a1f77c09486d08a336a3d2.
• Current-head hosted validate and claude-review: successful.
• Review decision: approved; merge state: clean.
• Priority findings: 22 P2 items, all fixed with replayable dispositions in the PR body.
• Discussed/optional notes: explicitly replied to and resolved; no untriaged item remains.
• Review threads: zero unresolved after the final current-head sweep.
• QA: required and satisfied; exact-head closeout replay verdict SATISFIED.
• Local verification: doctor 109 runs / 529 assertions, installer suite, and full bin/validate passed.

No deferred review work or follow-up issue is required for this batch target.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2e136793b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".