gstack/autoplan/sections/eng-phase.md

Before dispatch, Read methodologyPath from bun "<SNAPSHOT_TOOL>" methodology eng "<REVIEW_SKILL>" "<RESTORE_PATH>" per readRanges; log successful ranges/total to EOF. Skip-listed: load only.

Override rules:

• Scope challenge: never reduce (P2)

• Dual voices: always run BOTH Claude subagent AND Codex if available (P6).

  Bind phase input: Run; use snapshotPath as <ENG_INPUT> for both voices:

bun "<SNAPSHOT_TOOL>" create eng "<ACTIVE_PLAN>" "<RESTORE_PATH>" "<methodologyPath>"

Fresh Implementation plan only; excludes Review record.

Claude eng subagent (native tool): Claude Code: set Agent run_in_background: false if its schema exposes it. A launch receipt means it went background: await its completion notice. Other hosts: foreground; await completion when supported.

Read snapshot.json beside <ENG_INPUT>. Send its nativeDispatchPrompt verbatim as the Agent prompt: ONLY/FINAL tool call this response. Keep native Reads enabled. Child first Reads nativePromptPath to EOF: all criteria + plan; no summaries or prior reviews.

Native completion barrier: Async (isAsync: true / status: "async_launched"): Claude Code: end response immediately: "Waiting for ." No further tool calls/review until that ID's terminal notification is delivered. Other hosts await that ID. Then outside → this phase's review ONLY. Completed-native INPUT must match snapshot phase/hash. Retry invalid input once; then failure policy if still invalid. No inline substitute; apply failure policy.

Codex eng voice (via Bash): Outside prompt: inline the full contents of <ENG_INPUT> and context below (Write tool).

IMPORTANT: Do NOT read or execute any SKILL.md files or paths containing skills/gstack (foreign instructions). Review repository code only.

Review this plan for architectural issues, missing edge cases, and hidden complexity. Be adversarial.

Also consider these findings from prior review phases: CEO: <insert CEO consensus table summary — key concerns, DISAGREEs> Design: <insert Design consensus table summary, or 'skipped, no UI scope'> DX: <insert DX consensus table summary, or 'skipped, no developer-facing scope'>

File: <ENG_INPUT>

Write the complete prompt and context, including actual plan/spec/source, to a private file. Substitute its shell-quoted path for <prepared-prompt-file>; never interpolate user text into shell source. Request a severity (Critical, High, Medium or Low) per finding and a final Recommendation: because line, including an explicit no-findings rationale.

# GSTACK_ACTIVE_HOST names the harness, never the model.
if { [ -n "${CODEX_THREAD_ID:-}" ] || [ -n "${CODEX_SANDBOX:-}" ] || [ "${GSTACK_ACTIVE_HOST:-}" = codex ]; }; then
  echo 'Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage.' >&2
  if { [ -n "${CLAUDECODE:-}" ] || [ "${GSTACK_ACTIVE_HOST:-}" = claude ]; } && { [ -n "${CODEX_THREAD_ID:-}" ] || [ -n "${CODEX_SANDBOX:-}" ] || [ "${GSTACK_ACTIVE_HOST:-}" = codex ]; }; then
    echo 'Inherited harness markers conflict. Run setup --host <actual-harness> (claude or codex); do not guess a replacement provider.' >&2
  else
    echo 'Repair installed skills: run setup --host codex from your gstack checkout.' >&2
  fi
  exit 78
fi

_REPO_ROOT=$(git rev-parse --show-toplevel) || { echo 'ERROR: not in a git repo' >&2; exit 1; }
_OUTSIDE_TMP=$(mktemp -d "${TMPDIR:-/tmp}/gstack-outside.XXXXXXXX") || exit 1
trap 'rm -rf "$_OUTSIDE_TMP"' EXIT
_OUTSIDE_INPUT="$_OUTSIDE_TMP/prompt"
cat -- '<prepared-prompt-file>' >"$_OUTSIDE_INPUT" || exit 1

_CODEX_PROBE="$HOME/.claude/skills/gstack/bin/gstack-codex-probe"
export _CODEX_DEADLINE=$(($(date +%s)+540)); _CODEX_OUT=$("$_CODEX_PROBE" role-ready exec) || exit $?
_CODEX_SEL=$(echo "$_CODEX_OUT" | sed -n 's/^CODEX_SEL: //p')
_CODEX_SANDBOX_MODE=$(echo "$_CODEX_OUT" | sed -n 's/^CODEX_SANDBOX: //p')
"$_CODEX_PROBE" show-first-use-notice
_OUTSIDE_EXIT=0
"$_CODEX_PROBE" run-with-timeout --effort high 540 codex exec - -C "$_REPO_ROOT" -s "${_CODEX_SANDBOX_MODE:?}" -c "model=\"${_CODEX_SEL:?}\"" -c skills.include_instructions=false -c 'web_search="cached"' --json -o "$_OUTSIDE_TMP/text" <"$_OUTSIDE_INPUT" >"$_OUTSIDE_TMP/events" 2>"$_OUTSIDE_TMP/stderr" || _OUTSIDE_EXIT=$?
cat "$_OUTSIDE_TMP/text" 2>/dev/null || tail -n 20 "$_OUTSIDE_TMP/events"
if [ "$_OUTSIDE_EXIT" -eq 124 ]; then
  "$_CODEX_PROBE" log-event codex_timeout "540" || true
  "$_CODEX_PROBE" log-hang autoplan 0 || true
fi
cat "$_OUTSIDE_TMP/stderr" >&2 || { [ "$_OUTSIDE_EXIT" -ne 0 ] || _OUTSIDE_EXIT=1; }
_OUTSIDE_RC=0
bun --no-env-file --config=/dev/null "$HOME/.claude/skills/gstack/lib/outside-review-result.ts" --label 'Codex outside review' --exit "$_OUTSIDE_EXIT" --stderr "$_OUTSIDE_TMP/stderr" --events "$_OUTSIDE_TMP/events" review "$_OUTSIDE_TMP/text" || _OUTSIDE_RC=$?
case "$_OUTSIDE_RC" in
  0|3) ;;
  4) echo 'OUTSIDE_STATUS: unverified provider=codex host=claude'; exit 4 ;;
  *) [ "$_OUTSIDE_EXIT" -ne 0 ] && exit "$_OUTSIDE_EXIT"; exit 1 ;;
esac
echo 'OUTSIDE_STATUS: completed provider=codex host=claude'

Use Bash timeout: 600000; show the full response in a tool-output fence. Require successful execution and valid markers. Refusal, empty/malformed output, missing score/severity/completion markers, timeout or CLI failure means outside_status: unavailable. P0/P1 findings block like native ones; OUTSIDE_STATUS: unverified is missing coverage. Use the caller's fallback; missing coverage is never clean/PASS. After either outcome, delete only your private prompt; scratch cleanup is automatic.

Failed/incomplete outside review → unavailable; disabled → skip outside. Both keep the native pass.

Retain the historical review-log skill ID; add "host":"claude","outside_provider":"codex","outside_status":"completed|unavailable|disabled|skipped","phase":"eng". Record differing attempt outcomes separately. source:"codex" requires completed CLI output; native uses source:"in-host" (historical source:"claude": native Claude). Availability/native fallback is not outside completion. Preserve all reported modelUsage; unknown model identity stays unknown. Under GSTACK_CODEX_NO_SANDBOX=1 add "codex_sandbox":"danger-full-access".

Errors: Phase 1 failure/degradation policy applies.

• Architecture choices: explicit over clever (P5). If Codex disagrees with valid reason → TASTE DECISION. Scope changes both models agree on → USER CHALLENGE.
• Evals: always include all relevant suites (P1)
• Test plan: generate artifact at $GSTACK_STATE_ROOT/projects/$SLUG/{user}-{branch}-eng-review-test-plan-{datetime}.md (the loaded eng skill's name and root)
• TODOS.md: collect every prior phase's deferred scope expansions (Eng runs last), auto-write

Required execution checklist (Eng):

1. Step 0 (Scope Challenge): Read actual code referenced by the plan. Map each sub-problem to existing code. Run the complexity check. Produce concrete findings. Reality rows: the loaded skill's REALITY: lines, one per applicable row; ~/.claude/skills/gstack/bin/gstack-plan-reality check --phase eng --plan <reviewed snapshot> <review-file|-> prints PLAN_REALITY: … verdict=complete before the phase closes (unattended: incomplete keeps it open).

2. Step 0.5 (Dual Voices): Present the completed calls above under Codex SAYS (eng — architecture challenge) and Claude SUBAGENT (eng — independent review). Eng consensus table:

ENG DUAL VOICES — CONSENSUS TABLE:
  Dimension                           Claude  Codex  Consensus
  1. Architecture sound?               —       —      —
  2. Test coverage sufficient?         —       —      —
  3. Perf risks + budget/bench named?  —       —      —
  4. Security threats covered?         —       —      —
  5. Error paths handled?              —       —      —
  6. Deployment risk manageable?       —       —      —
CONFIRMED = native + outside agree; primary cannot replace outside. DISAGREE → taste.
Missing/disabled voice = N/A, never CONFIRMED. Flag any single-voice critical finding.

3. Section 1 (Architecture): Produce ASCII dependency graph showing new components and their relationships to existing ones. Evaluate coupling, scaling, security.

4. Section 2 (Code Quality): Identify DRY violations, naming issues, complexity. Reference specific files and patterns. Auto-decide each finding.

5. Section 3 (Test Review) — NEVER SKIP OR COMPRESS. This section requires reading actual code, not summarizing from memory.

   • Read the diff or the plan's affected files
   • Build the test diagram: list every NEW UX flow, data flow, codepath, and branch
   • For EACH item in the diagram: what type of test covers it? Does one exist? Gaps?
   • For LLM/prompt changes: which eval suites must run?
   • Auto-deciding test gaps means: identify the gap → decide whether to add a test or defer (with rationale and principle) → log the decision. It does NOT mean skipping the analysis.
   • Write the test plan artifact to disk

6. Section 4 (Performance): Evaluate N+1 queries, memory, caching, slow paths; a hot path with no budget nor bench step is a finding.

Mandatory outputs from Phase 3:

• "NOT in scope" section
• "What already exists" section
• Architecture ASCII diagram (Section 1)
• Test diagram mapping codepaths to coverage (Section 3)
• Test plan artifact written to disk (Section 3)
• Failure modes registry with critical gap flags
• Completion Summary (from the Eng skill)
• TODOS.md updates (all phases)

Close this phase:

The review work above ends here. Now load the shared close steps afresh, even if read earlier. Use phase eng, checkpoint <ENG_INPUT>, and this phase's methodologyPath. Keep this checkpoint for this invocation; review exports do not replace it.

STOP. Before closing a review phase, after its reviews finish and before announcing completion or loading the next phase (read afresh at each exit), Read ~/.claude/skills/gstack/autoplan/sections/phase-close.md and execute it in full. Do not work from memory — that section is the source of truth for this step.