Skip to content

[FORGE-105] feat(skills,cli): auto-codex in-skill hints (P2.5-T13)#169

Merged
firatcand merged 2 commits into
mainfrom
feat/FORGE-105-auto-codex-in-skill-hooks
May 17, 2026
Merged

[FORGE-105] feat(skills,cli): auto-codex in-skill hints (P2.5-T13)#169
firatcand merged 2 commits into
mainfrom
feat/FORGE-105-auto-codex-in-skill-hooks

Conversation

@firatcand
Copy link
Copy Markdown
Owner

@firatcand firatcand commented May 17, 2026

Summary

Add forge codex-suggest <event> CLI verb that prints a one-line hint at the end of /plan-task and /ship skills suggesting /codex review-{plan,impl}. Implements SPEC §Auto-codex skill-level hooks (lines 942-963) without host-level Claude Code hooks (Feature 7 was dropped 2026-05-17).

  • New CLI verb forge codex-suggest <event> — reads .forge/settings.yaml (cwd or via git rev-parse --git-common-dir fallback), checks codex.auto_codex_enabled + FORGE_AUTO_CODEX env, prints the locked hint text or stays silent.
  • Schema additions per SPEC §234-248: codex.*, decisions.*, doctor.* blocks — all .default({}) so existing settings.yaml parses unchanged. Only codex.auto_codex_enabled is consumed in this PR; the other blocks are forward-compat for /update-spec and forge doctor.
  • Skill wiring: step 9 added to /plan-task (with Bash(forge*) in tools), and forge codex-suggest ship appended to /ship Output section.

Why

P2.5-T13. Multi-model second opinion (ETHOS §6) is mandated on CRITICAL.md paths but historically required users to remember to invoke /codex manually. The in-skill hint surfaces the next-step recommendation at the exact moment it's needed (post-plan, pre-PR) without auto-executing — user types or skips. No host hooks → no Claude Code version coupling.

Forks asked & answered (in plan)

Fork User answer Resulting branch
Q1 /update-spec scope Skip, narrow scope Hint deferred until the skill exists. Event registered in EVENT_TO_VERB for zero-CLI-patch wiring when it lands.
Q2 Token-cap enforcement Drop the budget auto_codex_token_cap in schema as RESERVED (no consumer). SPEC amendment to drop "tokens/session" language filed as follow-up FORGE-124.
Q3 Suggestion mechanism New forge codex-suggest CLI verb TypeScript implementation, 15 unit tests. Top-level (not nested under orchestrate) since it's stateless.
Q4 Schema scope Add all three blocks per SPEC Schema matches SPEC §Settings exactly. decisions + doctor blocks inert in this PR.

Codex 2nd-pass (CRITICAL path: src/cli/init/scaffold.ts)

codex exec review, 7 attack vectors. One actionable finding:

  • F1 (confidence 8/10) — Path-trust hardening. git rev-parse --git-common-dir honors GIT_DIR/GIT_WORK_TREE/GIT_COMMON_DIR env vars, which an attacker could use to redirect settings discovery to a controlled repo. Fixed in commit 3eedc12 — strip those vars (plus GIT_CEILING_DIRECTORIES) before invoking git. Regression test plants a poisoned GIT_DIR and asserts the suggestion still fires.
  • F2-7: All 9/10 confidence, no action (no shell injection; scaffold widening safe; dead field preserved through schema parse; mirror text correct; no concurrency hazard; error path doesn't rethrow or hang).

Scope deviations from issue body

Both deviations were architectural forks decided during /plan-task with explicit user answers — not silent drops:

  • /update-spec hint — skill doesn't exist yet. The hint is a 3-line addition; whoever creates /update-spec adds it then. Linear acceptance criterion needs a comment trail.
  • Token-cap enforcement — was a legacy artifact of the dropped Feature 7 (auto-executing host hooks). For passive suggestions it bounds nothing meaningful. Follow-up FORGE-124 to amend SPEC and either remove the field or build session accounting.

Known follow-ups (filed separately, not coupled to this PR)

  • FORGE-124 (to file): SPEC amendment — remove the "bounded by tokens/session" language from §Auto-codex skill-level hooks (line 955), or implement real session accounting.
  • Worktree node_modules hydration: /pickup-task doesn't npm install in worktrees. Pre-existing fragility — 6 e2e tests in this repo shell out to repoRoot/node_modules/.bin/tsx. They pass on main, fail in fresh worktrees. Worth a separate ticket for /pickup-task to either install or symlink node_modules during hydration.
  • SPEC mirror-faithfulness clarity (Codex F5): SPEC line 953 gives only one example of the full printed line. Code generates the same pattern for all three events; SPEC could list all three full literals for clarity.

Test plan

  • npm test — 1062 pass / 11 skipped / 0 fail (15 new codex-suggest tests + 9 new schema tests)
  • npm run typecheck — clean
  • npm run build — clean (ESM + CJS, 6 files, 318 KB ESM / 344 KB CJS)
  • gitleaks detect — no leaks
  • Smoke test: node dist/bin/forge.cjs codex-suggest plan-task → prints 💡 Suggested next: /codex review-plan (run with FORGE_AUTO_CODEX=0 to disable)
  • Smoke test: FORGE_AUTO_CODEX=0 node dist/bin/forge.cjs codex-suggest plan-task → silent
  • Smoke test: node dist/bin/forge.cjs codex-suggest ship → prints /codex review-impl
  • Smoke test: node dist/bin/forge.cjs codex-suggest update-spec → prints /codex review-decision (reserved)
  • Smoke test: node dist/bin/forge.cjs codex-suggest bogus → exit 1 with stderr message
  • Codex /codex review (CRITICAL path src/cli/init/scaffold.ts touched) — F1 fixed, F2-7 no action

Linked

Closes FORGE-105 (P2.5-T13).

🤖 Generated with Claude Code

firatcand and others added 2 commits May 18, 2026 01:39
…dex schema (P2.5-T13)

Add `forge codex-suggest <event>` CLI verb that prints a one-line hint
suggesting `/codex review-{plan,impl,decision}` at the end of /plan-task
and /ship skills. Honors `FORGE_AUTO_CODEX=0` env and
`codex.auto_codex_enabled: false` in settings.yaml as disable levers.

Schema additions per SPEC §Settings (lines 234-248):
- `codex.auto_codex_enabled` (consumed by codex-suggest)
- `codex.auto_codex_token_cap` (RESERVED — see scope note below)
- `decisions.*` (forward-compat for /update-spec; no consumer in this PR)
- `doctor.*` (forward-compat for `forge doctor` verb; no consumer in this PR)

All new blocks have `.default({})` — existing settings.yaml files parse cleanly.

Scope deviations from issue body (decided during /plan-task forks):
- /update-spec hint deferred: skill doesn't exist yet. Event registered in
  EVENT_TO_VERB for zero-CLI-patch wiring when the skill lands.
- Token-cap accounting dropped: was a legacy artifact of the dropped
  Feature 7 (auto-executing host hooks). Passive suggestions bound nothing
  meaningful. SPEC amendment to remove the "tokens/session" language is
  filed as a follow-up.

Decision: /update-spec scope -> skip per user answer Q1 (skill doesn't exist)
Decision: token-cap concept -> drop per user answer Q2 (legacy artifact)
Decision: suggestion mechanism -> CLI verb per user answer Q3 (testable)
Decision: schema scope -> add all three blocks per user answer Q4 (SPEC parity)

Test coverage: 14 unit tests for codex-suggest + 9 schema tests for new
blocks. Full suite: 1061 pass, 11 skipped, 0 fail.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fore invoking git

Codex 2nd-pass: F1 (confidence 8/10) — `git rev-parse --git-common-dir`
honors GIT_DIR / GIT_WORK_TREE / GIT_COMMON_DIR env vars, which an attacker
could use to redirect settings.yaml discovery to a controlled repo with
auto_codex_enabled toggled.

Strip those vars (plus GIT_CEILING_DIRECTORIES) before invoking git so
resolution stays anchored on cwd. Regression test plants a poisoned
GIT_DIR and asserts the suggestion still fires (sanitization holds).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@firatcand firatcand force-pushed the feat/FORGE-105-auto-codex-in-skill-hooks branch from 3eedc12 to abb1028 Compare May 17, 2026 22:39
@firatcand firatcand merged commit 517bd97 into main May 17, 2026
10 checks passed
@firatcand firatcand deleted the feat/FORGE-105-auto-codex-in-skill-hooks branch May 17, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant