Skip to content

Commit 1e521c9

Browse files
HoracehxwHoracehxw
committed
fix(explore): address PR review feedback
1 parent 5af2849 commit 1e521c9

9 files changed

Lines changed: 128 additions & 33 deletions

‎scripts/ask-codex.sh‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -248,11 +248,14 @@ CODEX_DISABLE_HOOKS_ARGS=()
248248
_CODEX_DISABLE_HOOKS_CACHE="$SKILL_DIR/.codex-disable-hooks-supported"
249249
if [[ -f "$_CODEX_DISABLE_HOOKS_CACHE" ]]; then
250250
[[ "$(cat "$_CODEX_DISABLE_HOOKS_CACHE")" == "yes" ]] && CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks)
251-
elif codex --help </dev/null 2>&1 | grep -q -- '--disable'; then
252-
CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks)
253-
echo "yes" > "$_CODEX_DISABLE_HOOKS_CACHE" 2>/dev/null || true
254251
else
255-
echo "no" > "$_CODEX_DISABLE_HOOKS_CACHE" 2>/dev/null || true
252+
CODEX_HELP_OUTPUT="$(codex --help </dev/null 2>&1 || true)"
253+
if grep -q -- '--disable' <<< "$CODEX_HELP_OUTPUT"; then
254+
CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks)
255+
echo "yes" > "$_CODEX_DISABLE_HOOKS_CACHE" 2>/dev/null || true
256+
else
257+
echo "no" > "$_CODEX_DISABLE_HOOKS_CACHE" 2>/dev/null || true
258+
fi
256259
fi
257260

258261
# Build codex exec arguments (same pattern as loop-codex-stop-hook.sh)

‎scripts/install-codex-hooks.sh‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,16 @@ require_codex_hooks_support() {
7777
die "Codex CLI with native hooks support is required. Install Codex 0.114.0+ first."
7878
fi
7979

80-
if ! codex features list 2>/dev/null | grep -qE '^codex_hooks[[:space:]]'; then
80+
local codex_features_output
81+
local codex_help_output
82+
83+
codex_features_output="$(codex features list 2>/dev/null || true)"
84+
if ! grep -qE '^codex_hooks[[:space:]]' <<< "$codex_features_output"; then
8185
die "Installed Codex CLI does not expose the codex_hooks feature. Humanize Codex install requires Codex 0.114.0+."
8286
fi
8387

84-
if ! codex --help 2>&1 | grep -q -- '--disable'; then
88+
codex_help_output="$(codex --help 2>&1 || true)"
89+
if ! grep -q -- '--disable' <<< "$codex_help_output"; then
8590
die "Installed Codex CLI supports codex_hooks but lacks the --disable flag. Humanize's stop hook uses --disable codex_hooks to prevent recursive hook invocation. Please upgrade Codex."
8691
fi
8792
}

‎scripts/validate-directions-json.sh‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ if jq -e '
5656
# exactly one primary direction
5757
and ((.directions | map(select(.is_primary == true)) | length) == 1)
5858
59-
# direction_id: present, is a string, and unique across all entries
59+
# direction_id: present, is a string, unique, and safe as a whitespace-delimited token
6060
and (.directions | map(has("direction_id") and ((.direction_id | type) == "string")) | all)
61+
and (.directions | map(.direction_id) | all(test("^dir-[0-9]{2}-[a-z0-9-]+$")))
6162
and ((.directions | map(.direction_id) | unique | length) == (.directions | length))
6263
6364
# dir_slug: present, is a string, unique, and branch/path safe (lowercase alphanumeric + hyphens)

‎scripts/validate-explore-idea-io.sh‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,11 +318,12 @@ EFFECTIVE_CONCURRENCY=$(( CONCURRENCY < SELECTED_COUNT ? CONCURRENCY : SELECTED_
318318
# ========================================
319319

320320
PROJECT_ROOT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)"
321-
if git -C "$PROJECT_ROOT" diff --name-only HEAD 2>/dev/null | grep -q .; then
321+
DIRTY_FILES="$(git -C "$PROJECT_ROOT" diff --name-only HEAD -- 2>/dev/null || true)"
322+
if [[ -n "$DIRTY_FILES" ]]; then
322323
echo "ERROR: Main checkout has uncommitted tracked changes." >&2
323324
echo " Commit or stash changes before running explore-idea." >&2
324325
echo " Dirty files:" >&2
325-
git -C "$PROJECT_ROOT" diff --name-only HEAD 2>/dev/null | sed 's/^/ /' >&2
326+
printf '%s\n' "$DIRTY_FILES" | sed 's/^/ /' >&2
326327
exit 7
327328
fi
328329

@@ -344,9 +345,11 @@ fi
344345
# ========================================
345346
#
346347
# Worker base-anchor contract (enforced by worker-prompt.md):
347-
# Each worker MUST: (1) git checkout BASE_BRANCH in its worktree,
348-
# (2) assert HEAD == BASE_COMMIT, and (3) only then create the explore branch.
349-
# A HEAD mismatch is a fatal worker error (worker emits failure result immediately).
348+
# Workers are created at BASE_COMMIT in detached HEAD state.
349+
# Do NOT run `git checkout <BASE_BRANCH>` in worker setup because the coordinator
350+
# checkout may already have that branch checked out. Each worker asserts
351+
# HEAD == BASE_COMMIT before creating its explore branch.
352+
# A HEAD mismatch is a fatal worker error.
350353
# Workers MUST run only targeted tests for the files they touched, not the full test suite.
351354

352355
BASE_BRANCH="$(git -C "$PROJECT_ROOT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "unknown")"

‎tests/test-ask-codex.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,9 @@ cat > "$PROBE_BIN_DIR/codex" << 'PROBE_MOCK_SUPPORTS'
529529
#!/usr/bin/env bash
530530
if [[ "${1:-}" == "--help" ]] || echo "$*" | grep -q -- '--help'; then
531531
echo "--disable <feature> Disable a named feature"
532+
for i in $(seq 1 5000); do
533+
printf -- "--noise-%s\n" "$i"
534+
done
532535
exit 0
533536
fi
534537
if [[ -n "${MOCK_CODEX_STDERR:-}" ]]; then echo "$MOCK_CODEX_STDERR" >&2; fi

‎tests/test-codex-hook-install.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ if [[ "${1:-}" == "--help" ]]; then
4646
Usage: codex [OPTIONS] [PROMPT]
4747
--disable <feature> Disable a named feature for this invocation
4848
HELP
49+
for i in $(seq 1 5000); do
50+
printf ' --noise-%s\n' "$i"
51+
done
4952
exit 0
5053
fi
5154

‎tests/test-directions-json-schema.sh‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -96,126 +96,147 @@ run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
9696
[[ $EXIT_CODE -ne 0 ]] && pass "duplicate direction_id: exits non-zero" \
9797
|| fail "duplicate direction_id: exits non-zero" "non-zero" "$EXIT_CODE"
9898

99-
# NT-6: Duplicate dir_slug
99+
# NT-6: Empty direction_id
100+
F=$(make_fixture "empty-direction-id" '.directions[0].direction_id = ""')
101+
EXIT_CODE=0
102+
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
103+
[[ $EXIT_CODE -ne 0 ]] && pass "empty direction_id: exits non-zero" \
104+
|| fail "empty direction_id: exits non-zero" "non-zero" "$EXIT_CODE"
105+
106+
# NT-7: Whitespace-only direction_id
107+
F=$(make_fixture "whitespace-direction-id" '.directions[0].direction_id = " "')
108+
EXIT_CODE=0
109+
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
110+
[[ $EXIT_CODE -ne 0 ]] && pass "whitespace-only direction_id: exits non-zero" \
111+
|| fail "whitespace-only direction_id: exits non-zero" "non-zero" "$EXIT_CODE"
112+
113+
# NT-8: direction_id contains spaces
114+
F=$(make_fixture "spaced-direction-id" '.directions[0].direction_id = "dir 00 command history"')
115+
EXIT_CODE=0
116+
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
117+
[[ $EXIT_CODE -ne 0 ]] && pass "direction_id with spaces: exits non-zero" \
118+
|| fail "direction_id with spaces: exits non-zero" "non-zero" "$EXIT_CODE"
119+
120+
# NT-9: Duplicate dir_slug
100121
F=$(make_fixture "dup-dir-slug" '.directions[1].dir_slug = .directions[0].dir_slug')
101122
EXIT_CODE=0
102123
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
103124
[[ $EXIT_CODE -ne 0 ]] && pass "duplicate dir_slug: exits non-zero" \
104125
|| fail "duplicate dir_slug: exits non-zero" "non-zero" "$EXIT_CODE"
105126

106-
# NT-7: Duplicate source_index
127+
# NT-10: Duplicate source_index
107128
F=$(make_fixture "dup-source-index" '.directions[1].source_index = .directions[0].source_index')
108129
EXIT_CODE=0
109130
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
110131
[[ $EXIT_CODE -ne 0 ]] && pass "duplicate source_index: exits non-zero" \
111132
|| fail "duplicate source_index: exits non-zero" "non-zero" "$EXIT_CODE"
112133

113-
# NT-8: display_order is a string (not integer)
134+
# NT-11: display_order is a string (not integer)
114135
F=$(make_fixture "display-order-string" '.directions[0].display_order = "zero"')
115136
EXIT_CODE=0
116137
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
117138
[[ $EXIT_CODE -ne 0 ]] && pass "display_order string: exits non-zero" \
118139
|| fail "display_order string: exits non-zero" "non-zero" "$EXIT_CODE"
119140

120-
# NT-9: dir_slug contains uppercase
141+
# NT-12: dir_slug contains uppercase
121142
F=$(make_fixture "dir-slug-uppercase" '.directions[0].dir_slug = "CommandHistory"')
122143
EXIT_CODE=0
123144
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
124145
[[ $EXIT_CODE -ne 0 ]] && pass "dir_slug uppercase: exits non-zero" \
125146
|| fail "dir_slug uppercase: exits non-zero" "non-zero" "$EXIT_CODE"
126147

127-
# NT-10: dir_slug contains spaces
148+
# NT-13: dir_slug contains spaces
128149
F=$(make_fixture "dir-slug-space" '.directions[0].dir_slug = "command history"')
129150
EXIT_CODE=0
130151
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
131152
[[ $EXIT_CODE -ne 0 ]] && pass "dir_slug with spaces: exits non-zero" \
132153
|| fail "dir_slug with spaces: exits non-zero" "non-zero" "$EXIT_CODE"
133154

134-
# NT-11: Missing required per-direction field (name)
155+
# NT-14: Missing required per-direction field (name)
135156
F=$(make_fixture "missing-name" '.directions[0] |= del(.name)')
136157
EXIT_CODE=0
137158
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
138159
[[ $EXIT_CODE -ne 0 ]] && pass "missing direction.name: exits non-zero" \
139160
|| fail "missing direction.name: exits non-zero" "non-zero" "$EXIT_CODE"
140161

141-
# NT-12: objective_evidence is not an array
162+
# NT-15: objective_evidence is not an array
142163
F=$(make_fixture "evidence-not-array" '.directions[0].objective_evidence = "single string"')
143164
EXIT_CODE=0
144165
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
145166
[[ $EXIT_CODE -ne 0 ]] && pass "objective_evidence not array: exits non-zero" \
146167
|| fail "objective_evidence not array: exits non-zero" "non-zero" "$EXIT_CODE"
147168

148-
# NT-13: known_risks is not an array
169+
# NT-16: known_risks is not an array
149170
F=$(make_fixture "risks-not-array" '.directions[0].known_risks = "single string"')
150171
EXIT_CODE=0
151172
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
152173
[[ $EXIT_CODE -ne 0 ]] && pass "known_risks not array: exits non-zero" \
153174
|| fail "known_risks not array: exits non-zero" "non-zero" "$EXIT_CODE"
154175

155-
# NT-14: Invalid confidence value
176+
# NT-17: Invalid confidence value
156177
F=$(make_fixture "bad-confidence" '.directions[0].confidence = "maybe"')
157178
EXIT_CODE=0
158179
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
159180
[[ $EXIT_CODE -ne 0 ]] && pass "invalid confidence: exits non-zero" \
160181
|| fail "invalid confidence: exits non-zero" "non-zero" "$EXIT_CODE"
161182

162-
# NT-15: metadata.n_returned mismatch
183+
# NT-18: metadata.n_returned mismatch
163184
F=$(make_fixture "n-returned-mismatch" '.metadata.n_returned = 99')
164185
EXIT_CODE=0
165186
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
166187
[[ $EXIT_CODE -ne 0 ]] && pass "n_returned mismatch: exits non-zero" \
167188
|| fail "n_returned mismatch: exits non-zero" "non-zero" "$EXIT_CODE"
168189

169-
# NT-16: Missing required top-level key (directions)
190+
# NT-19: Missing required top-level key (directions)
170191
F=$(make_fixture "missing-directions-key" 'del(.directions)')
171192
EXIT_CODE=0
172193
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
173194
[[ $EXIT_CODE -ne 0 ]] && pass "missing .directions key: exits non-zero" \
174195
|| fail "missing .directions key: exits non-zero" "non-zero" "$EXIT_CODE"
175196

176-
# NT-17: Missing required top-level key (title)
197+
# NT-20: Missing required top-level key (title)
177198
F=$(make_fixture "missing-title-key" 'del(.title)')
178199
EXIT_CODE=0
179200
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
180201
[[ $EXIT_CODE -ne 0 ]] && pass "missing .title key: exits non-zero" \
181202
|| fail "missing .title key: exits non-zero" "non-zero" "$EXIT_CODE"
182203

183-
# NT-18: Missing required top-level key (original_idea)
204+
# NT-21: Missing required top-level key (original_idea)
184205
F=$(make_fixture "missing-original-idea" 'del(.original_idea)')
185206
EXIT_CODE=0
186207
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
187208
[[ $EXIT_CODE -ne 0 ]] && pass "missing .original_idea key: exits non-zero" \
188209
|| fail "missing .original_idea key: exits non-zero" "non-zero" "$EXIT_CODE"
189210

190-
# NT-19: Missing required top-level key (metadata)
211+
# NT-22: Missing required top-level key (metadata)
191212
F=$(make_fixture "missing-metadata" 'del(.metadata)')
192213
EXIT_CODE=0
193214
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
194215
[[ $EXIT_CODE -ne 0 ]] && pass "missing .metadata key: exits non-zero" \
195216
|| fail "missing .metadata key: exits non-zero" "non-zero" "$EXIT_CODE"
196217

197-
# NT-20: Missing direction_id (per-direction required field)
218+
# NT-23: Missing direction_id (per-direction required field)
198219
F=$(make_fixture "missing-direction-id" '.directions[0] |= del(.direction_id)')
199220
EXIT_CODE=0
200221
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
201222
[[ $EXIT_CODE -ne 0 ]] && pass "missing direction_id: exits non-zero" \
202223
|| fail "missing direction_id: exits non-zero" "non-zero" "$EXIT_CODE"
203224

204-
# NT-21: source_index is a string (not integer)
225+
# NT-24: source_index is a string (not integer)
205226
F=$(make_fixture "source-index-string" '.directions[0].source_index = "0"')
206227
EXIT_CODE=0
207228
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
208229
[[ $EXIT_CODE -ne 0 ]] && pass "string source_index: exits non-zero" \
209230
|| fail "string source_index: exits non-zero" "non-zero" "$EXIT_CODE"
210231

211-
# NT-22: title is not a string (numeric type)
232+
# NT-25: title is not a string (numeric type)
212233
F=$(make_fixture "title-numeric" '.title = 123')
213234
EXIT_CODE=0
214235
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?
215236
[[ $EXIT_CODE -ne 0 ]] && pass "numeric title: exits non-zero" \
216237
|| fail "numeric title: exits non-zero" "non-zero" "$EXIT_CODE"
217238

218-
# NT-23: objective_evidence items are not strings (numeric array)
239+
# NT-26: objective_evidence items are not strings (numeric array)
219240
F=$(make_fixture "evidence-items-numeric" '.directions[0].objective_evidence = [1, 2]')
220241
EXIT_CODE=0
221242
run_validate "$F" > /dev/null 2>&1 || EXIT_CODE=$?

‎tests/test-validate-explore-idea-io.sh‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,37 @@ else
186186
fail "exit 7 for dirty checkout" "exit 7" "exit=$EXIT_CODE"
187187
fi
188188

189+
# Exit 7: dirty checkout with enough files to catch git|grep SIGPIPE regressions
190+
DIRTY_MANY_REPO="$TEST_DIR/dirty-many-repo"
191+
init_test_git_repo "$DIRTY_MANY_REPO"
192+
cp "$VALID_FIXTURE" "$DIRTY_MANY_REPO/valid.directions.json"
193+
(
194+
cd "$DIRTY_MANY_REPO"
195+
mkdir -p dirty-files
196+
for i in $(seq 1 2000); do
197+
printf 'clean\n' > "dirty-files/file-$i.txt"
198+
done
199+
git add valid.directions.json dirty-files
200+
git commit -q -m "add many tracked files"
201+
for i in $(seq 1 2000); do
202+
printf 'dirty\n' >> "dirty-files/file-$i.txt"
203+
done
204+
)
205+
EXIT_CODE=0
206+
DIRTY_OUTPUT=$(
207+
cd "$DIRTY_MANY_REPO"
208+
CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" bash "$VALIDATE_SCRIPT" "$DIRTY_MANY_REPO/valid.directions.json" 2>&1
209+
) || EXIT_CODE=$?
210+
if [[ $EXIT_CODE -eq 7 ]] \
211+
&& grep -q "Dirty files:" <<<"$DIRTY_OUTPUT" \
212+
&& grep -q "dirty-files/file-1.txt" <<<"$DIRTY_OUTPUT"; then
213+
pass "exit 7 and lists dirty files when many tracked files are modified"
214+
else
215+
fail "exit 7 and lists dirty files when many tracked files are modified" \
216+
"exit 7 + dirty file list" \
217+
"exit=$EXIT_CODE output=$DIRTY_OUTPUT"
218+
fi
219+
189220
# Exit 9: missing worker prompt template
190221
NO_TMPL_PLUGIN="$TEST_DIR/plugin-no-tmpl"
191222
mkdir -p "$NO_TMPL_PLUGIN/scripts"
@@ -279,5 +310,27 @@ else
279310
fail "EFFECTIVE_CONCURRENCY capped to direction count" "1" "$EFFECTIVE"
280311
fi
281312

313+
echo ""
314+
echo "--- Static Contract Tests ---"
315+
echo ""
316+
317+
if grep -q 'Do NOT run `git checkout <BASE_BRANCH>`' "$VALIDATE_SCRIPT" \
318+
&& grep -q "detached HEAD" "$VALIDATE_SCRIPT"; then
319+
pass "worker base-anchor contract documents detached HEAD without checking out BASE_BRANCH"
320+
else
321+
fail "worker base-anchor contract documents detached HEAD without checking out BASE_BRANCH" \
322+
"detached HEAD + no checkout language" \
323+
"missing"
324+
fi
325+
326+
if grep -q 'diff --name-only HEAD --' "$VALIDATE_SCRIPT" \
327+
&& ! grep -q 'diff --name-only HEAD .*| grep -q' "$VALIDATE_SCRIPT"; then
328+
pass "dirty checkout check captures git diff output without grep -q pipeline"
329+
else
330+
fail "dirty checkout check captures git diff output without grep -q pipeline" \
331+
"capture-first dirty check" \
332+
"missing"
333+
fi
334+
282335
echo ""
283336
print_test_summary "validate-explore-idea-io.sh Test Summary"

‎tests/test-worker-result-contract.sh‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ REQUIRED_PLACEHOLDERS=(
7777
"<MAX_WORKER_ITERATIONS>"
7878
"<CODEX_TIMEOUT_MIN>"
7979
"<BASE_BRANCH>"
80+
"<BASE_COMMIT>"
8081
"<ORIGINAL_IDEA>"
8182
)
8283

@@ -113,6 +114,7 @@ REQUIRED_FIELDS=(
113114
"what_worked"
114115
"what_didnt"
115116
"bitlesson_action"
117+
"error"
116118
)
117119

118120
for field in "${REQUIRED_FIELDS[@]}"; do
@@ -141,11 +143,12 @@ else
141143
fail "template forbids nested skills/slash commands"
142144
fi
143145

144-
# No git push constraint
145-
if grep -q "No git push\|git push" "$WORKER_PROMPT"; then
146+
# No git push constraint: require explicitly prohibitive wording, not a passing
147+
# incidental mention of the command.
148+
if grep -q "No git push" "$WORKER_PROMPT" && grep -qi "Do not push .*remote" "$WORKER_PROMPT"; then
146149
pass "template forbids git push"
147150
else
148-
fail "template forbids git push"
151+
fail "template forbids git push" "explicit no-push phrasing" "missing"
149152
fi
150153

151154
# ask-codex.sh scope constraint

0 commit comments

Comments
 (0)