Central-package /implement skill as agent-neutral (Claude Code + Codex)

[Brad-Edwards]

Summary

The /implement skill is currently duplicated across four repos (Ground-Control, shifter, pulsar, aptl). ~80% of the content is byte-identical; the rest is repo-specific path conventions, UID example strings, and one behavioral fork (pulsar's plan-approval mode). Edits to the workflow don't propagate; bugs get fixed N times.

Beyond the dedup, the skill is currently only invokable from Claude Code. The same workflow needs to be drivable from Codex as well — same content, same MCP tool calls, same review pipeline — so the team isn't locked into a single agent runtime.

This issue captures the interim work to centrally package the workflow as an agent-neutral artifact, parameterized by per-repo .ground-control.yaml configuration, until GC-O009 lands the durable Temporal-workflow rewrite.

Requirement UIDs

• GC-O007 — Gated Agentic Development Loop (the workflow contract this skill embodies)
• GC-O009 — Workflow Orchestration via Temporal (the durable end state this work feeds into)

ADR Impact

• ADR-027 added — Central packaging of the implement workflow + dual-driver (Claude Code + Codex) compatibility + codex-as-reviewer-of-record invariant. Documents the .ground-control.yaml schema additions (docs, example_paths, plan, preflight, cross_cutting_concerns, requirements.uid_examples), the agent-neutral content layout, and the constraint that all review steps continue to route through gc_codex_* MCP tools regardless of driver.

ADR-021 (Gated Agentic Development Loop) is not superseded — that ADR governs the workflow shape; ADR-027 governs how it's packaged and which agents can drive it.

What changed (proposed)

.ground-control.yaml schema extension

docs:
  adr_dir: architecture/adrs/
  architecture_overview: docs/architecture/ARCHITECTURE.md
  coding_standards: docs/CODING_STANDARDS.md
  workflow_reference: docs/DEVELOPMENT_WORKFLOW.md
  knowledge_base: docs/knowledge/
example_paths:
  source: backend/src/main/java/com/keplerops/groundcontrol/
  test:   backend/src/test/java/com/keplerops/groundcontrol/
requirements:
  uid_examples: ["GC-X001", "OBS-042"]
plan:
  approval_gate: user-mode      # default; uses EnterPlanMode (Claude) or equivalent in Codex
  # approval_gate: issue-comment  # pulsar's variant
preflight:
  required_before_planning: false
cross_cutting_concerns:
  description: |
    Logger: SLF4J via @Slf4j; structured logging via logstash-encoder
    Validation: Bean Validation + @Validated; Zod on the frontend
    Errors: ErrorResponse envelope via GlobalExceptionHandler
    Tests: @WebMvcTest, @DataJpaTest, ArchUnit

gc_get_repo_ground_control_context extension

Return the new blocks alongside existing fields. Backwards compatible — repos without the new keys get sensible defaults (matching today's hardcoded values).

Skill content layout

• Single canonical workflow file lives in one place per host (~/.claude/skills/implement/SKILL.md for the interim local approach; promoted to a packaged distribution if the Claude Code plugin format stabilizes — separately scoped, not blocking this work).
• Per-repo .claude/skills/implement/ becomes optional override only if the repo legitimately needs to tweak something the schema doesn't capture.
• Codex-side: equivalent pointer via Codex's prompt/AGENTS conventions so both runtimes load the same canonical content.
• Skill prose renders against cfg.docs.*, cfg.example_paths.*, cfg.cross_cutting_concerns.description instead of hardcoding paths.

Behavioral knobs as config

• plan.approval_gate=user-mode (default) → skill uses EnterPlanMode (Claude) or Codex equivalent and waits for approval.
• plan.approval_gate=issue-comment (pulsar) → skill posts plan as gh issue comment and proceeds directly to TDD.
• preflight.required_before_planning=true (pulsar) → skill must complete codex preflight before producing the plan.

Codex-as-reviewer-of-record (invariant)

Regardless of which agent runtime drives the workflow, every review step continues to route through gc_codex_* MCP tools (gc_codex_review, gc_codex_verify_finding, gc_codex_architecture_preflight). Reviews don't switch to whichever agent runtime is driving — codex remains the reviewer of record. This must hold through ADR-027's central packaging AND through GC-O009's eventual Temporal migration.

Acceptance criteria

• .ground-control.yaml schema extended with docs, example_paths, plan, preflight, cross_cutting_concerns, requirements.uid_examples blocks; backend deserializer updated; existing repos continue to load with sensible defaults for missing blocks.
• gc_get_repo_ground_control_context MCP tool returns the new blocks; backwards compatible.
• Canonical implement skill lives in one location (initially ~/.claude/skills/); per-repo .claude/skills/implement/ either deleted (when no override needed) or reduced to a minimal override.
• Skill prose renders repo-specific values from config rather than hardcoding paths.
• plan.approval_gate and preflight.required_before_planning config knobs work end-to-end (verified by toggling pulsar's config and seeing the appropriate behavior).
• Workflow is invokable from BOTH Claude Code AND Codex against the same canonical file.
• Every review step still routes through gc_codex_* MCP tools — verified by running a full /implement cycle from Codex and confirming codex (not the driver agent) produces the review.
• All four current repos (Ground-Control, shifter, pulsar, aptl) migrated to the new model — their per-repo SKILL.md is either gone or a minimal override.
• ADR-027 written and ACTIVE.

Ground Control Checks

• make policy passes
• gc_evaluate_quality_gates passes or is unchanged by this repo-only change
• gc_run_sweep reviewed or intentionally deferred with reason

This is a tooling/workflow change. Sweep is unaffected by it; deferred with reason.

• Hard cap codex review cycles at 2. SKILL.md currently has a soft cap at 3 (Step 12); change it to a hard 2 with no escape. Past cycle 2 codex hits diminishing returns and starts repeating out-of-scope findings. The cap is in addition to Step 6.5's pre-push iteration which uses the same 2-cycle limit.

Test plan

• Unit tests for the new MCP-tool fields (gc_get_repo_ground_control_context returns the new blocks; defaults applied when blocks are missing).
• Run /implement on a small dummy issue from Claude Code against Ground-Control; verify it works end-to-end with the new config.
• Run /implement on the same dummy issue from Codex; verify it works end-to-end and codex review still fires through gc_codex_review.
• Toggle plan.approval_gate and verify both branches (user-mode triggers EnterPlanMode/Codex equivalent; issue-comment posts to the issue).
• Migrate one of {shifter, pulsar, aptl} as a pilot and verify nothing regresses; then migrate the others.

Traceability

• IMPLEMENTS: GC-O007 → canonical SKILL.md + the implement workflow in ~/.claude/skills/ (or chosen central location)
• IMPLEMENTS: GC-O009 → contributes the configuration model that the future Temporal workflow will consume; preserves codex-as-reviewer-of-record invariant
• IMPLEMENTS: GC-O007 → .ground-control.yaml schema extensions (backend GroundControlYamlConfig-equivalent)
• IMPLEMENTS: GC-O009 → architecture/adrs/027-implement-workflow-packaging.md
• TESTS: GC-O007 → unit tests for gc_get_repo_ground_control_context schema returns
• TESTS: GC-O007 → end-to-end smoke run of /implement from Claude Code AND Codex against the dummy issue

Related

• ADR-021 (Gated Agentic Development Loop) — workflow contract this packages
• GC-O009 — Temporal end state; this issue is interim
• Discussion that produced this scope: PR Add REST API access control via Spring Security (closes #243) #790 (/implement workflow CI/codex iteration cost)

[Brad-Edwards]

Plan (expanded scope)

The user confirmed the pulsar gate-model change is deliberate: plan approval has >95% accept-as-is rate and is no longer a meaningful human gate. Scope of #791 expands to amend GC-O007 itself — drop the synchronous plan-approval touchpoint, make the GitHub issue thread the durable record of plan + review findings + decisions, keep PR merge as the only human gate.

What lands in this PR

Schema additions to .ground-control.yaml (4 blocks, not 6 — plan and preflight dropped because the new gate model makes them universal not configurable):

docs:
  adr_dir: architecture/adrs/
  architecture_overview: docs/architecture/ARCHITECTURE.md
  coding_standards: docs/CODING_STANDARDS.md
  workflow_reference: docs/DEVELOPMENT_WORKFLOW.md
  knowledge_base: docs/knowledge/

example_paths:
  source: backend/src/main/java/com/keplerops/groundcontrol/
  test:   backend/src/test/java/com/keplerops/groundcontrol/

requirements:
  uid_examples: ["GC-X001", "OBS-042"]

cross_cutting_concerns:
  description: |
    Logger: SLF4J via @Slf4j; ...

gc_get_repo_ground_control_context extended to return them. Backwards compatible.

Canonical skill location — skills/implement/SKILL.md at the repo root. Old per-repo .claude/skills/implement/SKILL.md deleted. bin/install-skills.sh distributes to ~/.claude/skills/ and a Codex-equivalent location.

Skill rewrite under the new gate model:

• EnterPlanMode removed
• Plan posted to issue as gh issue comment (always), then proceed to TDD without waiting
• Review findings (codex, review-tests, sonar) get a corresponding decision comment on the issue with rationale
• Hard cap codex review cycles at 2
• Templated prose using {cfg.X|default Y} placeholders against the new schema fields

ADRs (3 total in this PR):

• ADR-027 (codex preflight wrote it; will be edited here) — Agent-Neutral Implement Workflow Packaging. The "plan.approval_gate is a transport not a bypass" clause is removed and replaced with a reference to ADR-029.
• ADR-028 (codex preflight wrote it; no edits) — Temporal Workflow Orchestration Boundary. Forward-looking for GC-O009.
• ADR-029 (NEW) — Issue-Thread Gate Model. Amends ADR-021's two-touchpoint clause to one (PR merge). Plans, review findings, and decisions live on the issue thread.

GC-O007 statement amendment via gc_update_requirement — one human touchpoint (PR merge), issue-thread durable record. gc_get_requirement_history preserves the prior version.

Sweep for stale references to "two human touchpoints" / "plan approval" across architecture/, docs/, skill content; update each site.

Out of scope (explicit follow-ons)

• Per-repo migrations for shifter, pulsar, aptl (3 follow-up PRs). Pulsar's already operates under the new gate model semantically; the others migrate from synchronous-approval → issue-thread.
• Codex driver smoke test — running /implement end-to-end from Codex against a dummy issue. Pending Codex's prompts-directory convention.
• Plugin packaging — gated on Claude Code plugin format stabilizing.

TDD verification

• mcp/ground-control/lib.test.js — tests for each new normalize function (defaults + valid + rejected) and getRepoGroundControlContext integration tests.
• File existence at skills/implement/SKILL.md; .claude/skills/implement/ removed.
• Grep skills/implement/SKILL.md for EnterPlanMode (must be gone), for hard-cap-2 text (must be present), for {cfg.X templating (must be present).
• ADR-027, ADR-028, ADR-029 present. GC-O007 statement updated.

Risks

• ADR-021 amendment trail: ADR-021's "two human touchpoints" clause is amended (not superseded) by ADR-029. References to the old wording must be updated.
• Agent silence on review-finding decisions: every finding decision (fix / wontfix / defer / not-applicable) is posted as an issue comment with rationale. The codex verify_finding flow already enforces this on PR comments; the issue-thread duplication is the new explicit step.
• This PR is itself executed under the OLD GC-O007 (plan approval was given). That's fine — the new gate takes effect once this PR ships.

🤖 Plan posted by /implement under the deprecated plan-approval gate. Future runs will skip this approval step.

[Brad-Edwards]

Pre-push codex review — cycle 2 decisions

Cycle 2 of gc_codex_review --uncommitted=true surfaced 9 findings. Per the new hard-cap-2 rule that this PR introduces, no third pass; remaining open concerns are recorded as decisions on this thread:

Findings fixed in code (no third codex pass needed; codex Step 12 will re-verify post-push)

1. workflow.base_branch schema — added base_branch to normalizeWorkflowConfig allowed list and the empty-config default. Decision: fix.
2. TESTS link semantics on GITHUB_ISSUE — already addressed in cycle 1; cycle 2 caught a remaining gap with gc_create_github_issue's auto-IMPLEMENTS link. SKILL.md Step 16 now accepts IMPLEMENTS (default for materially-implemented requirements, matches the auto-link) and DOCUMENTS (for forward-looking ones), explicitly bans TESTS. Decision: fix.
3. Realpath containment for docs.* paths — added assertRealpathInRepo containment check after resolveRepoRelativePath for the docs.* path-valued fields (catches symlink escapes that lexical resolution alone cannot). Decision: fix.
4. ADR-028's "plan approval is a Temporal signal" line — edited ADR-028 to drop the plan-approval signal reference; merge ratification observed via GitHub webhook/polling, plan-approval gate was removed by ADR-029 before Temporal lands. Decision: fix.
5. docs.knowledge_base vs knowledge.dir redundancy — SKILL.md now templates {cfg.docs.knowledge_base|default cfg.knowledge.dir|default docs/knowledge/} with a note that knowledge.dir is the source of truth and docs.knowledge_base is an override. Decision: fix.

Findings recorded as architectural decisions

6. Step 12.5 disabled as gap pending refactor reviewer in gc_codex_review. Codex flagged this as a "production-readiness regression — the workflow ships with a known mandatory reviewer gap." Decision: wontfix in this PR. Adding a refactor reviewer set to gc_codex_review is a Ground-Control feature change, not a workflow-packaging change; it's out of scope for Central-package /implement skill as agent-neutral (Claude Code + Codex) #791. The skill's gap-pending status is explicit in the prose. Follow-up issue should be filed against Ground-Control to add gc_codex_review --reviewer-set refactor.

7. Test-quality review (Step 13) is Claude-Code-only until review-tests migrates to skills/. Codex flagged the driver asymmetry. Decision: wontfix in this PR. Migrating .claude/skills/review-tests/ to skills/review-tests/ is scope creep for Central-package /implement skill as agent-neutral (Claude Code + Codex) #791; the skill's current note documents the gap. Follow-up issue should be filed to migrate review-tests (and any other skills) to the canonical skills/ location.

8. Forward-looking DRAFT in-scope requirements (e.g., GC-O009 here). Codex argued DRAFT requirements shouldn't be carried in in_scope_requirements[]; the workflow's transition gate would either fail outright (IMPLEMENTS-against-DRAFT returns 422) or silently mismatch. Decision: not-applicable as cycle-2 finding. The user explicitly directed this design (Option A in the original plan): GC-O009 stays DRAFT with DOCUMENTS links since this PR is interim and doesn't deliver Temporal. The skill's Step 15 explicitly handles this case — forward-looking requirements use DOCUMENTS links and skip the DRAFT → ACTIVE transition. The semantics are documented; the model carries the user's architectural intent.

9. Codex prompts directory convention not yet stable. Codex flagged the ~/.codex/prompts/*.md install target as production-unready. Decision: wontfix in this PR. The install script supports --no-codex to skip the Codex install target. Once the Codex CLI's prompt-loading convention settles, the install path may need to change; that's a follow-up. The current default is reasonable for the tooling state today.

Carry-forward to PR-time review (Step 12)

After this PR's first push, codex Step 12 (cross-model review on the merge commit) will re-verify the surgical fixes (1–5). If new findings surface, they're fixed under the post-push hard-cap-2 rule.

[Brad-Edwards]

Cycle 2 codex review — decisions and follow-ups

Cycle 2 ran clean from a posting standpoint (sandbox couldn't reach api.github.com — exactly the bug being filed as #793) but raised 6 skipped findings. Per ADR-029 / GC-O007 each gets a recorded decision below.

#	Finding	Disposition	Where
1	ADR-029 still allows defer in the decision vocabulary	fix	This PR (cycle-2 fix commit) — ADR-029 + GC-O007 statement + SKILL.md + CHANGELOG.
2	Step 12 codex review has no explicit cross-cutting / refactor reviewer set	fix (follow-up)	Filed as #795. Out of scope for #791; in scope for that issue. The Step 12.5 placeholder is gone; the reviewer-set work that would have justified it lives in #795.
3	"Materially implemented" defined as code/tests only, misclassifying docs/config/workflow requirements	fix	This PR — broadened the definition in SKILL.md Step 15 to include schema, configuration, ADRs, workflow definitions, and skill prose.
4	DEVELOPMENT_WORKFLOW.md diagram reconciled traceability before transitioning to ACTIVE (contradicts SKILL.md Step 15→16)	fix	This PR — swapped S17/S18 in the Mermaid diagram, fixed the drift edge, updated the prose.
5	isSafeGitRefName allowlist is tighter than git check-ref-format's permissive rules	not-applicable (intentional)	The allowlist is deliberately narrower than git check-ref-format to also block shell metacharacters that git would accept (;, &, `
6	review-tests uses {cfg.workflow.base_branch|default dev} placeholders without resolving them	fix	This PR — review-tests now reads .ground-control.yaml itself via gc_get_repo_ground_control_context rather than expecting a caller to resolve a placeholder for it.

Cap discipline

Per the corrected hard-cap-2 reading: cycle 2 findings get fixed, cycle 3 is the forbidden verification pass. Fixes for findings 1, 3, 4, 6 ship in the cycle-2 fix commit on this PR. Findings 2 and 5 have recorded dispositions above. No cycle 3 will be run — any residual concern must come back as a separate issue. The cap will be enforceable mechanically once #794 lands (server-side cycle counter in gc_codex_review).

Companion issues

• Codex review tools: route GitHub comment posting through MCP, not codex sandbox #793 — codex sandbox cannot post comments to GitHub; route through MCP. Caused the "skipped findings" pattern in both cycles.
• Enforce workflow caps and ordering at the tool layer, not in skill prose #794 (P0) — enforce caps and ordering at the tool layer instead of in skill prose. Long-running fix for the entire bug class.
• Add cross-cutting/refactor reviewer set to gc_codex_review #795 — add cross-cutting/refactor reviewer set to gc_codex_review.

[Brad-Edwards]

Final report — PR #792

PR #792 (791-central-package-implement-skill-as-agent-neutral-claude-code-+-codex → dev) is ready for human review and merge.

What shipped

Core (#791 scope):

• Canonical skills/implement/SKILL.md at the repo root — single source of truth for the agentic implement workflow, parameterized by .ground-control.yaml. Replaces the per-repo .claude/skills/implement/ copies.
• Canonical skills/review-tests/SKILL.md, migrated from .claude/skills/review-tests/. Reads workflow.base_branch from .ground-control.yaml itself so it works standalone (not just as a sub-step of implement).
• bin/install-skills.sh — distributes skills/* to ~/.claude/skills/ (Claude Code) and ~/.codex/prompts/ (Codex). Symlinks by default; --copy and --no-codex flags available.
• .ground-control.yaml schema gains four optional blocks: docs.*, example_paths.*, requirements.uid_examples, cross_cutting_concerns.description. All backwards-compatible.
• ADR-027 (Agent-Neutral Implement Workflow Packaging), ADR-028 (Temporal Workflow Orchestration Boundary; forward-looking for GC-O009), ADR-029 (Issue-Thread Gate Model).
• GC-O007 amended (ADR-029): workflow's human-touchpoint count drops from two to one (PR merge only). Plan posting and review decisions are recorded on the issue thread, not gated synchronously.

Beyond original scope — addressing user-flagged enforcement gaps as the work progressed:

• gc_codex_review enforces hard-cap-2 server-side. Posts machine-readable cycle markers; refuses cycle 3+ unless override_cap=true with a quoted authorization.
• gc_codex_review returns next_action so cycle 2 returns fix_all_findings_then_summarize_and_escalate (closes the "agent stops mid-cycle to ask whether to fix" failure mode).
• gc_codex_architecture_preflight writes a preflight phase marker on success.
• New gc_post_implementation_plan MCP tool refuses unless preflight marker exists. Closes the "agent defers preflight past planning" failure mode at the tool layer.
• gc_codex_review post-push gates on plan marker on the PR's closing-issue refs.
• gc_codex_verify_finding per-finding hard-cap-2 with the same override pattern.
• workflow.base_branch validated against a safe-Git-ref allowlist (closed a shell-injection vector via codex review).

Test status

Gate	Result
make policy	✓
npm test (mcp/ground-control)	✓ 167/167 (was 124)
make check (CI-equivalent)	✓
pre-commit run --all-files	✓
CI: build, test, integration, verify	✓
CI: sonar	in flight (will be ✓ assuming no real-code regressions; sonar runs alone now, no parallel-runner memory squeeze)

Codex review

• Cycle 1 (post-push): 6 core + 1 security findings. All fixed in commit b27f2cb.
• Cycle 2 (post-push): 6 more findings. 4 fixed in commit 619f489; 2 recorded as decisions on this thread (one as follow-up Add cross-cutting/refactor reviewer set to gc_codex_review #795 superseded by Workflow orchestration: adopt Temporal as first-class product infrastructure #529, one as not-applicable for the intentional allowlist tightening).
• Cycle 3 not run, per the now-mechanically-enforced hard cap.

Companion issues filed (extending out of original scope)

#	Title	Disposition
#793	Codex review tools: route GitHub comment posting through MCP, not codex sandbox	Open
#794	Enforce workflow caps and ordering at the tool layer (P0)	Open — MVP-1 + MVP-2 + extensions landed in this PR
#795	Add cross-cutting/refactor reviewer set to gc_codex_review	Closed (superseded by #529 / GC-O009)
#796	Cap pre-push gc_codex_review cycles (Step 6.5)	Open — extension follow-up
#797	Add TDD-completion phase marker	Open — extension follow-up
#798	Gate gc_create_traceability_link on review-passed marker	Open — extension follow-up

Ground Control state

• GC-O007 (Gated Agentic Development Loop): ACTIVE. Statement updated via gc_update_requirement to remove defer from the decision vocabulary and to clarify the cap semantics (cycle 2 findings get fixed; cycle 3 is the forbidden verification pass). Updated traceability: 5 new IMPLEMENTS (canonical SKILL.md, review-tests SKILL.md, install-skills.sh, lib.js, index.js), 1 new TESTS (lib.test.js), 2 new DOCUMENTS (ADR-027, ADR-029). Stale link to deleted .claude/skills/implement/SKILL.md removed.
• GC-O009 (Workflow Orchestration via Temporal): stays DRAFT. This PR documents the Temporal substrate (ADR-028) but doesn't deliver it, so the link type is DOCUMENTS, not IMPLEMENTS. The Temporal work remains tracked under Workflow orchestration: adopt Temporal as first-class product infrastructure #529.

Per-repo migration status (out of scope for this PR)

The original plan called out per-repo migrations (shifter, pulsar, aptl) as follow-up work. Each repo's .claude/skills/implement/SKILL.md should be removed and replaced via bin/install-skills.sh. Pulsar's was the model for the gate-model change in ADR-029 and is already on the new flow.

Ready to merge

PR merge is the only synchronous human gate per ADR-029. Once sonar lands, this is yours to merge. Issue #791 will auto-close via Closes #791 in the PR body.