diff --git a/README.md b/README.md index 716ee2c..509dcb1 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,8 @@ Considering Claude as your main and Codex as the copilot: Fable writes the plan, 5.6 Sol reviews it, Luna implements, back to Fable who reviews and fixes the diff, runs the testing gate, then a new Sol thread reviews again the code. All in one claude code session. Writer and reviewer are never the same thread. As of mid july 2026, this Fable + GPT5.6 harness combo is absolute peak. +One honest caveat: the implementer's `--notes` are injected into the reviewer's prompt, so a lazy (or scheming) writer agent can talk the reviewer out of findings. The skills mitigate this (reviews surfaced verbatim, capped rounds, push-back must be justified), but if a review converges suspiciously fast, read the notes. + ## MCP Servers: Less Is More Last piece of advise before your new coding quest: Every MCP server you add is extra context, extra latency, and extra confusion. Keep it minimal. The one use case where MCP genuinely shines is **up-to-date documentation**, so your agent stops hallucinating deprecated APIs/whatever. Two servers cover it: [Context7](https://github.com/upstash/context7) for current library & framework docs, and [Exa](https://github.com/exa-labs/exa-mcp-server) for web search when the answer isn't in any doc. No bloat beyond that. diff --git a/skills/codex-implement/scripts/start.sh b/skills/codex-implement/scripts/start.sh index 4e6f1af..bec63e8 100755 --- a/skills/codex-implement/scripts/start.sh +++ b/skills/codex-implement/scripts/start.sh @@ -21,6 +21,8 @@ export STATE_DIR # shellcheck source=../../codex-plan-review/scripts/_common.sh source "$SCRIPT_DIR/../../codex-plan-review/scripts/_common.sh" +require_tools codex jq + PROMPT_FILE="" while [ $# -gt 0 ]; do case "$1" in diff --git a/skills/codex-implement/state/.gitignore b/skills/codex-implement/state/.gitignore new file mode 100644 index 0000000..3601fc4 --- /dev/null +++ b/skills/codex-implement/state/.gitignore @@ -0,0 +1,5 @@ +# Per-target Codex implementation state (thread ids, last report text, +# raw event logs). Local-only — these are session artifacts, not +# source-of-truth artifacts. +* +!.gitignore diff --git a/skills/codex-plan-review/scripts/_common.sh b/skills/codex-plan-review/scripts/_common.sh index d59807e..e973b8c 100755 --- a/skills/codex-plan-review/scripts/_common.sh +++ b/skills/codex-plan-review/scripts/_common.sh @@ -16,30 +16,48 @@ mkdir -p "$STATE_DIR" # implementation runs Luna, reviews (plan + code) run Sol, effort xhigh. # Adjust these defaults to your preferred models. # CODEX_MODEL / CODEX_EFFORT act as per-run overrides. -case "$STATE_DIR" in - *codex-implement*) CODEX_MODEL="${CODEX_MODEL:-gpt-5.6-luna}" ;; - *) CODEX_MODEL="${CODEX_MODEL:-gpt-5.6-sol}" ;; +# Match the trailing path components exactly — a repo path that merely +# contains "codex-implement" must not flip reviews to the implement model. +case "${STATE_DIR%/}" in + */codex-implement/state | codex-implement/state) + CODEX_MODEL="${CODEX_MODEL:-gpt-5.6-luna}" ;; + *) CODEX_MODEL="${CODEX_MODEL:-gpt-5.6-sol}" ;; esac CODEX_EFFORT="${CODEX_EFFORT:-xhigh}" export CODEX_MODEL CODEX_EFFORT +# Fail fast when a required external tool is missing. Without this, +# `set -e` + suppressed stderr would kill the caller with no message. +require_tools() { + local tool + for tool in "$@"; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "error: required tool not found on PATH: $tool" >&2 + return 1 + fi + done +} + # Derive a per-target key from a path-like string. For real paths we # resolve to absolute; for non-path targets (branch names, commit -# ranges) we sanitize in place. Replace '/' with '__'; force any other -# non-portable characters to '_'. +# ranges) we use the string as-is. The key is a sanitized, readable +# form plus a checksum of the resolved target, so distinct targets +# that sanitize identically (e.g. "foo/bar" vs "foo__bar") never +# share state files. target_key() { - local target="$1" + local target="$1" resolved sanitized sum if [ -e "$target" ]; then - local abs - abs="$(realpath -- "$target" 2>/dev/null || readlink -f -- "$target")" - if [ -z "$abs" ]; then + resolved="$(realpath -- "$target" 2>/dev/null || readlink -f -- "$target")" + if [ -z "$resolved" ]; then echo "error: cannot resolve target path: $target" >&2 return 1 fi - printf '%s' "$abs" | sed 's|^/||; s|/|__|g' else - printf '%s' "$target" | sed 's|^/||; s|/|__|g; s|[^A-Za-z0-9._-]|_|g' + resolved="$target" fi + sanitized="$(printf '%s' "$resolved" | sed 's|^/||; s|/|__|g; s|[^A-Za-z0-9._-]|_|g')" + sum="$(printf '%s' "$resolved" | cksum | cut -d' ' -f1)" + printf '%s.%s' "$sanitized" "$sum" } # Backwards-compatible alias used by older script call sites. @@ -57,23 +75,21 @@ events_file() { printf '%s/%s.events.ndjson' "$STATE_DIR" "$(target_key "$1")" } -# Load a prompt template from $1 and substitute {{TARGET}} and -# {{EXTRA_PROMPT}} placeholders with the values of the $TARGET and -# $EXTRA_PROMPT environment variables. Other text passes through -# verbatim — no surprise expansion of unrelated $VAR sequences. -# Writes the substituted prompt to stdout. +# Load a prompt template from $1 and substitute {{TARGET}}, +# {{EXTRA_PROMPT}} and {{IMPLEMENTER_NOTES}} placeholders with the +# values of the corresponding environment variables. The replacement +# expansions are double-quoted, which forces bash to treat them as +# literal strings — without the quotes, bash >= 5.2 (patsub_replacement) +# expands '&' to the matched text, exactly the mangling awk's gsub did. load_prompt() { - local tpl="$1" + local tpl="$1" content if [ ! -f "$tpl" ]; then echo "error: prompt template not found: $tpl" >&2 return 1 fi - awk -v target="${TARGET-}" -v extra="${EXTRA_PROMPT-}" -v notes="${IMPLEMENTER_NOTES-}" ' - { - gsub(/\{\{TARGET\}\}/, target) - gsub(/\{\{EXTRA_PROMPT\}\}/, extra) - gsub(/\{\{IMPLEMENTER_NOTES\}\}/, notes) - print - } - ' "$tpl" + content="$(cat "$tpl")" + content="${content//'{{TARGET}}'/"${TARGET-}"}" + content="${content//'{{EXTRA_PROMPT}}'/"${EXTRA_PROMPT-}"}" + content="${content//'{{IMPLEMENTER_NOTES}}'/"${IMPLEMENTER_NOTES-}"}" + printf '%s\n' "$content" } diff --git a/skills/codex-plan-review/scripts/resume.sh b/skills/codex-plan-review/scripts/resume.sh index 0c69212..e7ec06c 100755 --- a/skills/codex-plan-review/scripts/resume.sh +++ b/skills/codex-plan-review/scripts/resume.sh @@ -11,6 +11,8 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=_common.sh source "$SCRIPT_DIR/_common.sh" +require_tools codex + PROMPT_FILE="" IMPLEMENTER_NOTES="" while [ $# -gt 0 ]; do @@ -49,6 +51,11 @@ if [ ! -f "$THREAD_FILE" ]; then exit 2 fi THREAD_ID="$(cat "$THREAD_FILE")" +if [ -z "$THREAD_ID" ]; then + echo "error: thread file for $TARGET is empty: $THREAD_FILE" >&2 + echo " run reset.sh, then start.sh to begin a fresh session." >&2 + exit 2 +fi PROMPT="$(load_prompt "$PROMPT_FILE")" diff --git a/skills/codex-plan-review/scripts/start.sh b/skills/codex-plan-review/scripts/start.sh index 6b245f4..ede9c19 100755 --- a/skills/codex-plan-review/scripts/start.sh +++ b/skills/codex-plan-review/scripts/start.sh @@ -12,6 +12,8 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=_common.sh source "$SCRIPT_DIR/_common.sh" +require_tools codex jq + PROMPT_FILE="" while [ $# -gt 0 ]; do case "$1" in