coalesce-labs/phase-review

phase-review

You are the review phase agent. You run inside claude --bg and own a single responsibility: pre-landing review of the worker branch using the gstack [[review]] skill, producing ${ORCH_DIR}/workers/<TICKET>/review.json and at most one remediation commit. You then emit phase.review.complete.<ticket> and exit. Built on the [[_phase-agent-template]] contract.

You do not invoke /ultrareview — that command is reserved for the user to trigger interactively (it costs real money via the multi-agent cloud review).

Prelude

set -uo pipefail

: "${CATALYST_ORCHESTRATOR_DIR:?required (set by phase-agent-dispatch)}"
: "${CATALYST_ORCHESTRATOR_ID:?required}"
: "${CATALYST_PHASE:?required}"
: "${CATALYST_TICKET:?required}"

ORCH_DIR="$CATALYST_ORCHESTRATOR_DIR"
ORCH_ID="$CATALYST_ORCHESTRATOR_ID"
PHASE="$CATALYST_PHASE"
TICKET="$CATALYST_TICKET"
CHANNEL="orch-${ORCH_ID}"

SIGNAL_FILE="${ORCH_DIR}/workers/${TICKET}/phase-${PHASE}.json"
[[ -f "$SIGNAL_FILE" ]] || { echo "phase-${PHASE}: signal file missing" >&2; exit 1; }

PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-$(cd "$(dirname "$0")/../../.." && pwd)}"

COMMS="${PLUGIN_ROOT}/scripts/catalyst-comms"
[[ -x "$COMMS" ]] || COMMS="$(command -v catalyst-comms 2>/dev/null || true)"
if [[ -n "$COMMS" ]]; then
  "$COMMS" join "$CHANNEL" --as "$TICKET" \
    --capabilities "phase-${PHASE}: ${TICKET}" \
    --orch "$ORCH_ID" --parent orchestrator --ttl 3600 >/dev/null 2>&1 || true
  "$COMMS" send "$CHANNEL" "phase-review started" --as "$TICKET" --type info \
    --orch "$ORCH_ID" >/dev/null 2>&1 || true
fi

SESSION_SCRIPT="${PLUGIN_ROOT}/scripts/catalyst-session.sh"
if [[ -x "$SESSION_SCRIPT" ]]; then
  CATALYST_SESSION_ID=$("$SESSION_SCRIPT" start \
    --skill "phase-${PHASE}" \
    --ticket "$TICKET" \
    --workflow "${CATALYST_SESSION_ID:-}")
  export CATALYST_SESSION_ID
fi

TS=$(date -u +%Y-%m-%dT%H:%M:%SZ)
TMP="${SIGNAL_FILE}.tmp.$$"
# CTL-496: persist catalystSessionId so orchestrate-roll-usage --phase can
# attribute cost to the right session_metrics row.
jq --arg ts "$TS" --arg sid "${CATALYST_SESSION_ID:-}" '
  .status = "running"
  | .updatedAt = $ts
  | if $sid != "" then .catalystSessionId = $sid else . end
' "$SIGNAL_FILE" > "$TMP" \
  && mv "$TMP" "$SIGNAL_FILE"

# Prior-phase artifact: verify.json from phase-verify.
VERIFY_ARTIFACT="${ORCH_DIR}/workers/${TICKET}/verify.json"
if [[ ! -f "$VERIFY_ARTIFACT" ]]; then
  echo "phase-review: prior verify.json missing" >&2
  "${PLUGIN_ROOT}/scripts/phase-agent-emit-complete" \
    --phase "$PHASE" --ticket "$TICKET" --status failed \
    --reason "prior_artifact_missing:verify.json"
  exit 1
fi
REGRESSION_RISK=$(jq -r '.regression_risk // 0' "$VERIFY_ARTIFACT")

<!-- Linear status is written by the coordinator (CTL-558): the execution-core scheduler / orchestrate-phase-advance applies the mapped state on every committed phase transition. The phase agent no longer transitions Linear. -->

/goal

/goal "I have written ${ORCH_DIR}/workers/${TICKET}/review.json with
       {findings:[...], remediationCommit:string|null, reviewPassed:bool} AND any
       HIGH-severity finding with a deterministic fix has a corresponding
       remediation commit on HEAD. I have printed the path on stdout."

Work block

1. Run /review (gstack) — never /ultrareview

Invoke the /review skill via the Task tool. It analyzes the diff against the base
branch for SQL safety, LLM trust boundary violations, conditional side effects,
and other structural issues. Capture its output as raw text and parse into the
review findings array.

DO NOT invoke /ultrareview from this phase. If a future iteration wants multi-agent cloud review, the user runs it interactively before merge.

2. Merge findings from verify.json

The findings array in verify.json (from [[phase-verify]]) is the upstream source of truth. Treat each verify finding as a candidate review item:

• HIGH severity + deterministic fix → remediation commit
• HIGH severity + ambiguous fix → record in review.json for human attention
• MEDIUM / LOW → record but do not commit

The fix decision is deterministic when:

• The finding's recommendation is a single concrete code change with a clear before/after, AND
• The fix is local (no cross-file refactor), AND
• The fix doesn't change a public API or test expectation.

3. Create at most ONE remediation commit

If you make any code changes, batch them into a single commit:

git add -A
git commit -m "fix(${ticket-scope}): phase-review remediations for ${TICKET}

Addresses HIGH-severity findings surfaced by phase-verify and /review:
- <one line per finding>

Refs: ${TICKET}"

Scope (dev/pm/meta etc.) comes from the project's existing convention; if unclear, use dev. Never use --no-verify or --no-gpg-sign.

4. Write the artifact

ARTIFACT="${ORCH_DIR}/workers/${TICKET}/review.json"
REMEDIATION_SHA=$(git log -1 --grep="phase-review remediations for ${TICKET}" \
  --format=%H 2>/dev/null || echo "")
REVIEW_PASSED=$([[ -z "$BLOCKING_FINDINGS" ]] && echo "true" || echo "false")

jq -nc \
  --argjson findings "$REVIEW_FINDINGS_JSON" \
  --arg sha "$REMEDIATION_SHA" \
  --argjson passed "$REVIEW_PASSED" \
  --arg ts "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \
  '{findings: $findings,
    remediationCommit: (if $sha == "" then null else $sha end),
    reviewPassed: $passed,
    generatedAt: $ts}' > "${ARTIFACT}.tmp" \
  && mv "${ARTIFACT}.tmp" "$ARTIFACT"

Findings array shape — each entry mirrors phase-verify's shape plus an addressedBy field:

{
  "severity": "high|medium|low",
  "kind": "review|sql|trust-boundary|side-effect|...",
  "file": "path/to/file.ts",
  "line": 42,
  "message": "Short description",
  "addressedBy": "remediation-commit|deferred-to-human|none"
}

Inbox check (CTL-749)

After /review Task returns, check for mid-flight context updates from the human:

1. If ${ORCH_DIR}/workers/${TICKET}/inbox.jsonl exists and is non-empty, read it fully.
2. Parse each JSONL line — entries have kind: "comment" or kind: "description_changed".
3. For each entry, decide:
   • Absorb and continue: the update is additive context (clarification, extra constraints, "also handle X") — fold it into your working context and continue. Post a brief reply comment acknowledging the update (one sentence).
   • Pause and replan: the update fundamentally changes scope or invalidates the current approach — emit failed with reason: "mid_flight_replan_needed" via ${PLUGIN_ROOT}/scripts/phase-agent-emit-complete and post the reason to Linear as a comment before exiting.
4. After reading, archive processed entries:

   [[ -f "${ORCH_DIR}/workers/${TICKET}/inbox.jsonl" ]] && \
     mv "${ORCH_DIR}/workers/${TICKET}/inbox.jsonl" \
        "${ORCH_DIR}/workers/${TICKET}/inbox.processed-$(date +%s).jsonl" || true
   

5. If no inbox file or it is empty, continue normally.

End block

TS=$(date -u +%Y-%m-%dT%H:%M:%SZ)
TMP="${SIGNAL_FILE}.tmp.$$"
jq --arg ts "$TS" --arg artifact "$ARTIFACT" \
  '.updatedAt = $ts | .artifact = $artifact' \
  "$SIGNAL_FILE" > "$TMP" && mv "$TMP" "$SIGNAL_FILE"

Mirror the phase output to Linear as a single comment (CTL-632). Renders the PASS/FAIL result label, findings-by-severity, remediation commit SHA, and the full findings JSON inside a <details> block. Body is hard-truncated to 30,000 bytes. Fail-open and idempotent via the per-phase marker file.

LINEAR_MIRROR_MARKER="${ORCH_DIR}/workers/${TICKET}/.linear-mirror-${PHASE}"
if [[ ! -e "${LINEAR_MIRROR_MARKER}" ]]; then
  if [[ "${REVIEW_PASSED}" == "true" ]]; then
    RESULT_LABEL="PASS"
  else
    RESULT_LABEL="FAIL"
  fi
  FINDINGS_COUNT="$(printf '%s' "${REVIEW_FINDINGS_JSON}" | jq -r 'length' 2>/dev/null || echo 0)"
  FINDINGS_BY_SEVERITY="$(printf '%s' "${REVIEW_FINDINGS_JSON}" | jq -r '
    group_by(.severity)
    | map("- " + (.[0].severity // "unknown") + ": " + (length|tostring))
    | join("\n")' 2>/dev/null)"
  FINDINGS_PRETTY="$(printf '%s' "${REVIEW_FINDINGS_JSON}" | jq -r '.' 2>/dev/null)"
  if [[ -n "${REMEDIATION_SHA}" ]]; then
    REMEDIATION_LINE="- **Remediation commit**: \`${REMEDIATION_SHA}\`"
  else
    REMEDIATION_LINE="- **Remediation commit**: _none_"
  fi
  MIRROR_BODY="$(cat <<EOF
**Phase Review**

- **Result**: ${RESULT_LABEL}
- **Findings**: ${FINDINGS_COUNT}
${REMEDIATION_LINE}

**Findings by severity**:
${FINDINGS_BY_SEVERITY:-_none_}

<details>
<summary>Full findings JSON</summary>

\`\`\`json
${FINDINGS_PRETTY}
\`\`\`

</details>

_Posted automatically by phase-review (CTL-632)._
EOF
)"
  MIRROR_FOOTER=""
  if [[ -n "${PLUGIN_ROOT:-}" && -x "${PLUGIN_ROOT}/scripts/lib/phase-mirror-footer.sh" ]]; then
    MIRROR_FOOTER="$("${PLUGIN_ROOT}/scripts/lib/phase-mirror-footer.sh" --orch-dir "${ORCH_DIR}" --ticket "${TICKET}" --phase "${PHASE}" 2>/dev/null || true)"
  fi
  [[ -n "${MIRROR_FOOTER}" ]] && MIRROR_BODY="${MIRROR_BODY}
${MIRROR_FOOTER}"
  if [[ ${#MIRROR_BODY} -gt 30000 ]]; then
    MIRROR_BODY="${MIRROR_BODY:0:30000}

_... (truncated)_"
  fi
  COMMENT_POST="${CATALYST_COMMENT_POST_HELPER:-${PLUGIN_ROOT}/scripts/lib/linear-comment-post.sh}"
  if [[ ! -x "$COMMENT_POST" ]]; then COMMENT_POST="$(command -v linear-comment-post.sh 2>/dev/null || true)"; fi
  if [[ -n "$COMMENT_POST" && -x "$COMMENT_POST" ]] && "$COMMENT_POST" "${TICKET}" "${MIRROR_BODY}" >/dev/null; then
    : > "${LINEAR_MIRROR_MARKER}"
  else
    echo "phase-review: linear-comment-post failed (continuing)" >&2
  fi
fi

Step N — Capture friction (compound loop, CTL-789)

Before you emit completion, append this phase's friction to the shared per-ticket friction log. This is the PRODUCER half of the compound-engineering loop: the ticket-compound curator harvests thoughts/shared/friction/${TICKET}.md to distil durable learnings and ADR proposals. REPLACE each <…> placeholder below with your real experience THIS phase — terse, 3–6 lines total; "None." is a valid value when the phase was frictionless. ${TICKET} is already resolved in the Prelude — do not re-derive it. This append is off the critical path and best-effort: it must NEVER fail or block emit-complete.

# --- Compound-engineering friction capture (CTL-789, Slice 1). Off critical path; NEVER block emit. ---
FRICTION_LOG="thoughts/shared/friction/${TICKET}.md"
mkdir -p "$(dirname "$FRICTION_LOG")"
[ -f "$FRICTION_LOG" ] || printf '# Friction log — %s\n' "${TICKET}" > "$FRICTION_LOG"
cat >> "$FRICTION_LOG" <<EOF

## review · ${TICKET} · $(date +%Y-%m-%dT%H:%M:%S%z)
- **Backtracks / redone work:** <where you backtracked or redid work this phase — or "None.">
- **Missing / wrong / hard-to-find context:** <context that was absent, stale, or hard to locate — or "None.">
- **If I'd known:** <the ADR / guidance / past learning that would have saved this — the compounding signal — or "None.">
EOF

"${PLUGIN_ROOT}/scripts/phase-agent-emit-complete" \
  --phase "$PHASE" --ticket "$TICKET" --status complete

# Self-halt after complete to prevent zombie workers (CTL-778 step 2).
# Read our own bg_job_id from the signal file and ask Claude to stop us.
# Best-effort: a failed stop is covered by the daemon reaper backstop.
if [[ -n "${ORCH_DIR:-}" && -f "${ORCH_DIR}/workers/${TICKET}/phase-${PHASE}.json" ]]; then
  _SELF_BG=$(jq -r '.bg_job_id // empty' \
    "${ORCH_DIR}/workers/${TICKET}/phase-${PHASE}.json" 2>/dev/null || true)
  [[ -n "$_SELF_BG" ]] && claude stop "${_SELF_BG:0:8}" >/dev/null 2>&1 || true
fi

[[ -n "$COMMS" ]] && "$COMMS" done "$CHANNEL" --as "$TICKET" >/dev/null 2>&1 || true

Failure handling

"${PLUGIN_ROOT}/scripts/phase-agent-emit-complete" \
  --phase "$PHASE" --ticket "$TICKET" --status failed \
  --reason "<short reason>"
[[ -n "$COMMS" ]] && "$COMMS" send "$CHANNEL" \
  "phase-review failed: <reason>" --as "$TICKET" --type attention \
  --orch "$ORCH_ID" >/dev/null 2>&1 || true
exit 1