Add runtime prompt compatibility gate - #697
Conversation
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review summaryReviewed the new Overall: the design is solid — read-only helper, explicit One correctness gap found (left as an inline comment on No security issues found — regex interpolations in |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Review summaryI did a deep static review of this PR — it's large (1428 additions, 24 files), so I traced through the core new logic by hand and cross-checked it against the accompanying test suite and JSON schema rather than skimming. Note: Core script (
Left one inline nitpick: a couple of entries in Overall this is carefully constructed, fail-closed-by-default code with strong test coverage (schema validation, byte-preservation assertions, adversarial fixtures for ambiguous/malformed input). I did not find any confirmed correctness or security bugs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b275fe3fe2
ℹ️ 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".
justin808
added
the
agent-claimed
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b43397448
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54521c94f0
ℹ️ 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".
| CONTRACT_PATHS = [ | ||
| File.expand_path("../../../docs/host-adapter/contract.md", __dir__), | ||
| File.expand_path("../docs/host-adapter/contract.md", __dir__) | ||
| ].freeze |
There was a problem hiding this comment.
Drift risk: two copies of the host-adapter contract, nothing keeps them in sync.
This introduces a second full copy of the contract at skills/pr-batch/docs/host-adapter/contract.md (used as CONTRACT_PATHS[1], the fallback for an isolated skill-only install — see run_helper_from_isolated_skill_copy in the test file). It's byte-identical to docs/host-adapter/contract.md today, but:
bin/validatehas no step that diffs the two files.bin/validate-doc-linksonly checks a narrow, explicitly-named list of docs (docs/operator-handbook.md), so it won't catch this either.- The runtime
adapter-contract-unavailablegate only checks that the file contains thePrompt compatibility protocol: 1substring — it doesn't verify the two copies are actually equivalent in content.
So a future edit to the canonical contract (new rule, tightened invariant, even a protocol version bump) that misses this nested copy would silently leave any "isolated skill copy" install (e.g. a marketplace/plugin bundle that ships just skills/pr-batch/) running stale contract text while the helper still reports portable and passes the marker check.
Worth either generating this file from the canonical doc at install/packaging time, or adding a bin/validate-run check that asserts the two files are byte-identical so drift fails CI instead of failing silently at runtime.
ReviewFocused mostly on the one piece of real executable logic in this PR — Security / correctness of the gate itself: looks solid. It's genuinely read-only (no One finding worth addressing — left as an inline comment: the PR ships a second full copy of
No other correctness or security issues found in the executable code. The bulk of the remaining diff is workflow/skill prose (header-metadata rendering rules, conversion invariants, etc.) that I can't meaningfully bug-hunt via static reading beyond what's already covered by the |
| ].freeze | ||
| HEADER_NAMES = ["Prompt host", "Prompt mode", "Preferred route", "Route requirement"].freeze | ||
| RESERVED_HEADER_LIKE = | ||
| /^[ \t]*(?:Prompt[ \t._-]+(?:host|mode)|Preferred[ \t._-]+route|Route[ \t._-]+requirement)(?:[ \t]*(?::+|=)|[ \t]+[^\r\n]+)/i |
There was a problem hiding this comment.
Correctness: RESERVED_HEADER_LIKE can fail-close on legitimate prose, not just header injection.
The second alternative [ \t]+[^\r\n]+ matches any line that starts with Prompt host, Prompt mode, Preferred route, or Route requirement followed by whitespace and arbitrary text — not just malformed header-style lines. Because this check (malformed_reserved_header, lines 312-315) scans every line of the whole prompt body, not just the first 4 header lines, a perfectly valid compatible/portable prompt whose free-text body happens to contain a line starting with e.g. Preferred route selection stays advisory for this batch. would fail CANONICAL_HEADER (no literal : right after the phrase) and trigger emit_error("invalid-metadata", ...), rejecting an otherwise-valid prompt.
Failure scenario: a batch prompt with complete, well-formed headers but a body line like Notes: Preferred route must not weaken any gate. (assuming it starts the line) — or any multi-line field whose continuation happens to start with one of these four phrases — gets rejected with invalid-metadata even though nothing is actually wrong with it. Suggest anchoring the "malformed header" check to lines that already look like a Name<sep>value header pattern (e.g. requiring a following :/=) rather than triggering on prose that merely begins with the phrase.
| @@ -0,0 +1,345 @@ | |||
| # Host Adapter Contract | |||
There was a problem hiding this comment.
Reuse/maintenance: byte-identical duplicate of docs/host-adapter/contract.md with no drift check.
This file is a full copy of docs/host-adapter/contract.md (same blob, 1d47faf8e), committed so skills/pr-batch/bin/prompt-compatibility's CONTRACT_PATHS fallback (../docs/host-adapter/contract.md relative to bin/) can find a contract copy when the skill is installed standalone without the full repo doc tree.
No test or install/lint step verifies these two copies stay in sync: prompt-compatibility-test.rb#run_helper_from_isolated_skill_copy manually copies the canonical docs/host-adapter/contract.md into a temp dir at test time rather than exercising this committed duplicate, so this file's actual content is never checked against the source of truth. If a future edit updates docs/host-adapter/contract.md (including bumping Prompt compatibility protocol: N) without updating this copy, a standalone-installed skill would silently serve stale contract text from its local fallback — ironically undermining the very "fails closed on stale/mixed-revision adapter" guarantee this contract describes. Consider generating this file at install/build time, symlinking it, or adding a CI check that diffs the two paths.
|
|
||
| has_goal_wrapper = prompt.match?(GOAL_WRAPPER) | ||
| header_text = has_goal_wrapper ? prompt.sub(GOAL_WRAPPER, "") : prompt | ||
| header_lines = header_text.lines.first(4).map { |line| line.delete_suffix("\n").delete_suffix("\r") } |
There was a problem hiding this comment.
Correctness (minor): Batch size target: <host>;wave: 0/0 passes validation.
batch_target_error only rejects when wave_cap > max_batch_wave_capacity(...) or wave_items > wave_cap. Both are false for wave: 0/0, so a batch declaring zero capacity/zero items is accepted as valid rather than flagged as invalid-batch-size-target. Consider also requiring wave_cap >= 1 (and typically wave_items >= 1) so a degenerate zero-item batch target is rejected the same way other malformed batch-size declarations are.
|
Additional finding (could not be posted inline — anchor line is outside the diff hunks):
|
justin808
added
agent-claimed
|
The maintainer accepted the delivery/compatibility recommendation. Durable decision: #372 (comment) This clears the product-design question only. Continue to fail closed on missing, contradictory, or incompatible route/contract evidence. Current review findings, validation, exact-head QA, and merge authority remain separate gates. |
Resolves two conflicts against main: - bin/install-agent-workflows: keep this branch's loops over pack_doc_directories=(host-adapter schemas solutions) in verify_pack_doc_destinations, both branches of verify_install_destinations, copy_pack_docs, and link_pack_docs. They cover main's hardcoded docs/solutions and docs/schemas entries identically and add host-adapter. Main's validate-review-findings helper, managed_pack_helper_copy_fingerprints, pack_helper_destination_owned, and the refusal to replace an unowned docs/schemas symlink are all kept. - docs/installation-and-upgrades.md: combine main's managed_pack_helper_copy_fingerprints wording with this branch's sentence covering every installed prompt-compatibility contract, schema, <target>/docs/solutions/* document, and the third-party notice. Decided rule (main wins): docs/schemas keeps main's refusal to replace an unowned directory symlink from #695, together with main's test_flat_install_refuses_an_unowned_schema_directory_symlink in skills/pr-batch/bin/task-review-loop-test.rb. Only docs/host-adapter is replaced without following children. Test change: test_install_replaces_prompt_contract_directory_symlinks_without_following_children in bin/install-agent-workflows-test.bash now loops over host-adapter only, with a comment above the loop noting the docs/schemas rule. Nothing else in that test changed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a87cfca67a
ℹ️ 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".
| SKILLS_ROOT = File.expand_path("../..", __dir__) | ||
| # Keep the pack-wide skill-name set stable so partial installs do not change | ||
| # cross-host skill detection. | ||
| KNOWN_SKILL_NAMES = %w[ |
There was a problem hiding this comment.
Fresh evidence beyond the prior partial-install fix is that this supposedly pack-wide list omits the existing skills/audit-chats skill. A Codex prompt containing Use $audit-chats before closeout therefore converts to Claude without rewriting or rejecting that invocation, and reclassifying the converted prompt on Claude returns compatible with execution allowed; add audit-chats to this stable set so the required host-specific skill call cannot be silently skipped.
AGENTS.md reference: AGENTS.md:L5-L6
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65cebea89f
ℹ️ 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".
|
|
||
| def validate_cross_host_mechanics(body, prompt_host, active_host) | ||
| source_sigil = prompt_host == "codex" ? "$" : "/" | ||
| unsupported_skills = skill_mechanics(body, source_sigil) - SKILL_NAMES |
There was a problem hiding this comment.
When a consumer repository defines a domain skill such as $deploy-production, this check searches only KNOWN_SKILL_NAMES, so a Codex-to-Claude conversion leaves that invocation unchanged; reclassifying the converted prompt on Claude then returns compatible with execute_allowed: true, potentially skipping a required consumer-specific step. Detect any command-like source-host skill sigil and reject names outside the explicitly rewritable set rather than limiting detection to this pack’s static list. Consumer-specific domain skills are expected to live outside this repository.
AGENTS.md reference: AGENTS.md:L32-L33
Useful? React with 👍 / 👎.
| def rewritable_mechanic_line?(line, sigil) | ||
| escaped = Regexp.escape(sigil) | ||
| line.match?(/\AUse #{escaped}pr-batch\b/) || | ||
| line.match?(/\ABase:[^\r\n]*verify #{escaped}pr-batch\+workflow;/) || | ||
| line.match?(/\A- Resolve `#{escaped}pr-batch`;/) || | ||
| line.match?(/\A- ask=>#{escaped}pr-walkthrough;/) | ||
| end |
There was a problem hiding this comment.
Bug: rewritable_mechanic_line? doesn't match the line skills/triage/SKILL.md actually mandates, so legitimate cross-host batch prompts fail closed.
skills/triage/SKILL.md:351 requires every generated prompt to include this exact line:
- Resolve `base_branch` via repo/`AGENTS.md` config; fetch/prune origin; verify `$pr-batch`+workflow; unresolved=>UNKNOWN.
This line contains `$pr-batch` (detected by skill_mechanics, since the preceding backtick isn't excluded by its negative lookbehind), but it matches none of the four rewritable_mechanic_line? alternatives — it starts with - Resolve `base_branch` rather than Base:, - Resolve `$pr-batch`;, Use $pr-batch, or - ask=>$pr-walkthrough;.
Failure scenario: feed a codex-originated triage batch prompt containing this exact line to prompt-compatibility --active-host claude. Since prompt_host ("codex") != active_host ("claude"), validate_cross_host_mechanics runs, unsupported_supported_mechanic becomes true, and the tool emits unsupported-host-mechanic and exits 2 — rejecting a fully legitimate, sigil-correct prompt. Same-host use is unaffected (that path skips validate_cross_host_mechanics), so this specifically breaks the cross-host conversion path that is this tool's whole purpose.
| malformed_reserved_header = header_text.lines.any? do |line| | ||
| line.match?(RESERVED_HEADER_LIKE) && !line.match?(CANONICAL_HEADER) | ||
| end | ||
| emit_error("invalid-metadata", active_host) if malformed_reserved_header |
There was a problem hiding this comment.
Over-broad RESERVED_HEADER_LIKE can reject a legitimate prompt whose body prose merely starts a line with a reserved phrase.
RESERVED_HEADER_LIKE (lines 14-15) matches any line that begins with Prompt host, Prompt mode, Preferred route, or Route requirement followed by any whitespace + text (not just a header-formatted : value):
RESERVED_HEADER_LIKE =
/^[ \t]*(?:Prompt[ \t._-]+(?:host|mode)|Preferred[ \t._-]+route|Route[ \t._-]+requirement)(?:[ \t]*(?::+|=)|[ \t]+[^\r\n]+)/iThis check runs against every line of header_text (the whole prompt, not just the first 4 header lines). A legitimate line anywhere in the body such as Preferred route selection remains advisory during recovery. matches RESERVED_HEADER_LIKE (no colon) but not CANONICAL_HEADER, so malformed_reserved_header becomes true and the entire otherwise-valid prompt is rejected with invalid-metadata — even though the line was ordinary prose, not an attempted header spoof.
This is clearly intentional as a defense against header-spoofing in the body (see test_malformed_header_like_text_cannot_hide_behind_legacy_detection), but the pattern is broad enough to also catch harmless prose using these four phrases, causing false-positive rejections of legitimate prompts.
| wave_cap = wave_match[2].to_i | ||
| wave_items = wave_match[3].to_i | ||
| return "invalid-batch-size-target" if wave_cap > max_batch_wave_capacity(expected_target) | ||
| return "invalid-batch-size-target" if wave_items > wave_cap |
There was a problem hiding this comment.
batch_target_error has no lower bound — wave: 0/0 passes as a valid batch size target.
wave_cap = wave_match[2].to_i
wave_items = wave_match[3].to_i
return "invalid-batch-size-target" if wave_cap > max_batch_wave_capacity(expected_target)
return "invalid-batch-size-target" if wave_items > wave_capOnly an upper bound is enforced. Batch size target: codex;wave: 0/0 passes both checks (0 > 10 is false, 0 > 0 is false), so a degenerate zero-capacity/zero-item wave declaration is accepted instead of being rejected as invalid-batch-size-target, even though "wave: <cap/items>" implies both should be positive integers. Low severity (not an escalation, since 0 is minimal not maximal), but an unguarded contract gap worth a wave_cap < 1 / wave_items < 1 check.
| /\bsandbox_permissions["'`]?[ \t]*:[ \t]*["'`]?(?:require_escalated|use_default)\b["'`]?/ | ||
| ].freeze | ||
| GOAL_WRAPPER = %r{\A/goal\r?\n} | ||
| LEGACY_CODEX = %r{\A/goal\r?\nUse \$pr-batch(?:\s|\z)} |
There was a problem hiding this comment.
Doc/code mismatch: contract text describes legacy detection more loosely than the actual regex.
docs/host-adapter/contract.md (and its duplicate at skills/pr-batch/docs/host-adapter/contract.md, around line 126-127) says legacy detection is bounded to "a leading Codex /goal immediately followed by a $pr-batch invocation." The actual regex only recognizes the literal phrase Use $pr-batch:
LEGACY_CODEX = %r{\A/goal\r?\nUse \$pr-batch(?:\s|\z)}A prompt like /goal\n$pr-batch: complete this (no leading "Use ") would not match LEGACY_CODEX and would fall through to unrecognized-prompt, even though the contract text implies any $pr-batch invocation right after /goal should be recognized. Safe-direction only (stricter code than documented → more fail-closed rejections, no security exposure), but the contract overstates what the tool actually recognizes.
| def rewritable_mechanic_line?(line, sigil) | ||
| escaped = Regexp.escape(sigil) | ||
| line.match?(/\AUse #{escaped}pr-batch\b/) || | ||
| line.match?(/\ABase:[^\r\n]*verify #{escaped}pr-batch\+workflow;/) || | ||
| line.match?(/\A- Resolve `#{escaped}pr-batch`;/) || | ||
| line.match?(/\A- ask=>#{escaped}pr-walkthrough;/) | ||
| end |
There was a problem hiding this comment.
Bug: legitimate cross-host triage prompts get rejected as unsupported-host-mechanic.
skills/triage/SKILL.md:351 mandates every generated batch prompt contain this exact line:
- Resolve `base_branch` via repo/`AGENTS.md` config; fetch/prune origin; verify `$pr-batch`+workflow; unresolved=>UNKNOWN.
This line contains `$pr-batch` (matched by skill_mechanics), but it matches none of the four patterns here — it starts with - Resolve `base_branch` rather than Base: or - Resolve `$pr-batch`;.
In validate_cross_host_mechanics (around line 258), any line where skill_mechanics matches but rewritable_mechanic_line? doesn't triggers emit_error("unsupported-host-mechanic", ...). So a fully spec-compliant, codex-originated triage batch prompt fails closed the moment it's validated for cross-host use with --active-host claude — exactly the scenario this tool exists to support. Same-host validation is unaffected.
Suggest adding a pattern that matches the triage base-resolution line shape (or generalizing the match to any line containing a backtick-quoted `$pr-batch` outside the "protected field" positions already excluded elsewhere).
| @@ -0,0 +1,345 @@ | |||
| # Host Adapter Contract | |||
There was a problem hiding this comment.
Reuse: this file is a byte-for-byte duplicate of docs/host-adapter/contract.md with no test keeping the two in sync.
Both files currently share the same git blob (1d47faf), so they're identical today, but nothing in the test suite (bin/install-agent-workflows-test.bash, skills/pr-batch/bin/*-test.rb) asserts that these two source-of-truth copies stay identical going forward — the existing cmp -s checks only compare an installed target copy against $ROOT, never these two in-repo sources against each other. A future edit to one (e.g. bumping Prompt compatibility protocol: 1 to 2) without updating the other would silently diverge: flat/top-level installs would serve one contract revision and skill-embedded installs (via CONTRACT_PATHS in prompt-compatibility) would serve the other, with no CI failure to catch it.
| wave_cap = wave_match[2].to_i | ||
| wave_items = wave_match[3].to_i | ||
| return "invalid-batch-size-target" if wave_cap > max_batch_wave_capacity(expected_target) | ||
| return "invalid-batch-size-target" if wave_items > wave_cap |
There was a problem hiding this comment.
batch_target_error only enforces an upper bound on the wave declaration. Batch size target: codex;wave: 0/0 passes both checks (0 > max_cap is false, 0 > 0 is false) and is accepted as a valid, non-degenerate batch target, even though a zero-capacity/zero-item wave contradicts the documented wave: <cap/items> contract (both should be positive). Consider adding return "invalid-batch-size-target" if wave_cap < 1 || wave_items < 1.
| body = converted.sub(%r{\AUse [/$]pr-batch\b}) do |invocation| | ||
| invocation.sub(%r{[/$]}, "/") | ||
| end | ||
| headers_for(target_host, "batch", preferred_route, ending) + body |
There was a problem hiding this comment.
Simplification: this re-derivation is dead code — rewrite_host_mechanics already fixed the sigil by this point.
body = converted.sub(%r{\AUse [/$]pr-batch\b}) do |invocation|
invocation.sub(%r{[/$]}, "/")
end
headers_for(target_host, "batch", preferred_route, ending) + bodyThis branch only runs when source_mode == "legacy-goal" (i.e. LEGACY_CODEX matched), which guarantees the body starts with the literal phrase Use $pr-batch. rewrite_host_mechanics, called a few lines earlier in the same function, already matches that exact phrase via rewritable_mechanic_line?'s /\AUse #{escaped}pr-batch\b/ pattern and rewrites it to Use /pr-batch for target_host == "claude". By the time this .sub runs, converted already starts with Use /pr-batch, so this second regex is a no-op restating work already done — worth removing or replacing with a comment explaining why it's redundant, so a future reader doesn't assume it's load-bearing.
| @@ -0,0 +1,345 @@ | |||
| # Host Adapter Contract | |||
There was a problem hiding this comment.
This file is a byte-identical duplicate of docs/host-adapter/contract.md (confirmed via diff), with nothing in CI enforcing the two copies stay in sync. bin/install-agent-workflows-test.bash only compares an installed target copy back to $ROOT, never the two in-repo source files against each other. A future edit to one copy (e.g. bumping "Prompt compatibility protocol: 2") that misses the other would silently desync the contract text served by flat/full installs vs. skill-embedded/isolated installs. Consider making one of these a symlink to the other, or adding a CI check that diffs them.
| if [[ -L "$target/docs/schemas" ]]; then | ||
| echo "Refusing to replace unowned schema directory symlink: $target/docs/schemas" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This pre-existing hardcoded docs/schemas symlink refusal was never folded into the new generic pack_doc_directories loop (host-adapter schemas solutions) added alongside it in verify_pack_doc_destinations. That loop now treats schemas as a "managed" linked-parent and continues past its per-file ownership check — but this unconditional early exit always fires first for copy and symlink modes alike, so the generic loop's schemas branch is dead weight in practice (confirmed intentional by the new test's comment restricting symlink-replacement coverage to host-adapter only, citing #695). Not a functional bug, but worth a one-line comment here (or folding this check into the loop) so the next person adding a 4th pack_doc_directories entry doesn't have to rediscover why schemas is special-cased.
justin808
added
complexity:complexify
1 participant
Problem
Pasteable PR-batch prompts can carry Codex- or Claude-specific mechanics. Without an intake gate, a wrong-host paste can begin the wrong workflow before anyone notices.
Refs #372.
Execution boundary
prompt-compatibilityhelper at the canonical PR-batch intake boundary, before target interpretation, worker launch, repository mutation, coordination mutation, or GitHub writes.compatible,portable, andconversion-required.conversion-requiredreturns inert converted text, sets execution false, validates that the result will classify on relaunch, and stops the current run.docs/host-adapter/contract.mdboundary. Copy, symlink, and plugin-companion installs package the exact contract and schema without rewriting installed shared Markdown.Conversion invariants
Conversion changes only explicit host/mode metadata, the optional Codex Goal wrapper, structurally recognized PR-batch/PR-walkthrough sigils, and an explicit host batch-size target. Objective, targets, scope, dependencies, permissions, safety gates, QA, review, merge authority, and preferred advisory route remain unchanged.
The gate rejects malformed host or batch-target metadata, unsupported host/mode pairs, qualified routes with unresolved delivery ownership, protected-field rewrites, non-convertible skill commands, and host runtime mechanics that do not belong to the declared host. Legacy detection remains limited to a leading Codex
/goalfollowed immediately byUse $pr-batch; incidental Codex or Claude names never trigger conversion.The generated prompt headers replace two redundant prompt-resident route-observation lines. Detailed requested and observed routes remain in the durable Batch Plan/manifest. The tight Codex templates retain 349–350 characters of headroom, up from the pre-existing 299-character failure on main.
Tests
ruby skills/pr-batch/bin/prompt-compatibility-test.rb— 24 runs, 495 assertionsruby skills/pr-batch/bin/model-routing-contract-test.rb— 40 runs, 8,749 assertionsruby skills/pr-batch/bin/goal-completion-contract-test.rb— 127 runs, 1,670 assertionsAGENT_WORKFLOWS_SOURCE_CHECKOUT=1 ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb— pass; tightest Codex headroom 349 charactersbin/install-agent-workflows-test.bashruby bin/validate-doc-links-test.rbandbin/validate-doc-linksruby bin/host-adapter-syntax-test.rbandbin/validate-host-adapter-syntaxbin/lint, Ruby/Bash syntax, schema parse and validation, andgit diff --checkPer lane direction, local full
bin/validatewas not run; hosted Validate is the final full-suite source.Review remediation
Independent and hosted exact-head reviews found and verified fixes for installed-document packaging, pre-security helper provenance, unsupported mechanics, protected-field preservation, host/mode and schema invariants, malformed and legacy batch targets, stale compact-routing assertions, and unguarded relaunchable recovery/continuation prompts. The latest round also rejects Claude-only review/loop commands during conversion and verifies post-render walkthrough routes for every host. Duplicate metadata-like lines remain deliberately fail-closed.
Overlap and integration
Live overlap recorded before implementation:
813d738cebe455910c89cdab48285decaa852d56overlapsbin/validate, both batch skills, triage, prompt intake,workflows/pr-processing.md, the size guard, and goal-completion tests. Its short readable prompt is intentionally supported through metadata-driven conversion without requiring the legacy invocation-first body.05787dc6a2ec353ef32c8e6b704ca606b92d8df5overlaps the same generator/intake surfaces exceptbin/validate. Its prompt-headroom work remains independently owned; this PR reclaims space only by replacing redundant route fields with required compatibility metadata.At integration, retain #575's short readable body, make ordinary Codex delivery
batchwith Goal optional, preserve #662's canonical title ownership, and reapply the four-field renderer rather than restoring current long templates.Risk and exact follow-up
The helper fail-closes on unsupported mechanics and binds portable execution to prompt-compatibility protocol v1, reducing mixed-revision risk. The remaining product decision is a trusted active flat-versus-native
scwdelivery-route source; this PR does not infer it from prose or file presence.Exact follow-up: add a versioned read-only active-delivery-route probe and use it to render or reject qualified
scwinvocation syntax before #372 is closed.Changelog:
deferred_to_update_changelog; no changelog edit.