Skip to content

fix(planning): check-open-questions refuses duplicate headings, names its bound line, skips fences #4065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kyle-sexton wants to merge 1 commit into
base: main
Choose a base branch
from
+312 −36
Open

fix(planning): check-open-questions refuses duplicate headings, names its bound line, skips fences #4065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/planning/.claude-plugin/plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json",
"name": "planning",
"version": "0.39.6",
"version": "0.39.7",
"userConfig": {
"use_ask_user_question": {
"type": "boolean",
Expand Down
21 changes: 21 additions & 0 deletions plugins/planning/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,27 @@
All notable changes to the `planning` plugin are documented here. Format follows
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning.

## [0.39.7]

### Fixed

- **`scripts/check-open-questions.sh`:** a ledger or Brief holding more than one heading that
matches the section the gate grades now exits 2 naming every matching line, instead of binding
silently to the first. A ledger copied from the checklist template that kept the template's own
`## Open-question register` (whose example rows are unfenced) above a live one was graded on the
example rows and reported a phantom `status=open`.
- **`scripts/check-open-questions.sh`:** every register-derived ungradeable message (zero rows,
malformed row or id, duplicate or gapped id, unknown status) and the Brief's missing-id message
now name the heading the gate bound to and its line number, so a bind to the wrong section is
visible from stderr alone.
- **`scripts/check-open-questions.sh`:** heading detection and section extraction skip fenced
code blocks. The template's register section carries a fenced `bash` block whose `# Step 3`
comment lines were read as headings, so a live row written after that fence was invisible and
the gate reported an empty register.
- **`scripts/check-open-questions.sh`:** a fenced block opened and never closed is ungradeable,
named as the cause. Every row after such a fence was skipped as documentation, so a register with
one answered row and an open row hidden behind a stray fence graded clean.

## [0.39.6]

### Fixed
Expand Down
144 changes: 109 additions & 35 deletions plugins/planning/scripts/check-open-questions.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
#
# Exit 0 = every registered question is resolved (register is clean)
# Exit 1 = at least one question is still `open` (the contract is not locked)
# Exit 2 = ungradeable: no ledger, no register section, an empty register, a
# malformed row, an unknown status, a duplicate or non-contiguous Q id,
# or a named `--brief` that is missing
# Exit 2 = ungradeable: no ledger, no register section, a duplicate register or
# deferred-questions heading, an unterminated fenced block, an empty
# register, a malformed row, an unknown status, a duplicate or
# non-contiguous Q id, or a named `--brief` that is missing
#
# Usage:
# bash check-open-questions.sh --ledger <interview-checklist.md> [--brief <PLAN.md>]
Expand Down Expand Up @@ -49,18 +50,50 @@ usage() {
sed -n '/^# Mechanical/,/^# `registered=/p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//'
}

# Print a markdown section's body: everything after the first heading whose text
# matches `pattern` (matched case-insensitively, so a ledger that title-cases the
# section still grades), up to the next heading of any level or end of file.
# Exits 3 when no such heading exists — distinguishable from an empty section.
extract_section() {
# List the headings whose text matches `pattern` (case-insensitively, so a ledger
# that title-cases the section still grades), one per line as
# `<line number><TAB><heading>`. A heading-shaped line inside a fenced block
# (``` or ~~~) is documentation, never a heading: the template's own register
# carries a fenced bash block whose `# Step 3 ...` comment lines would otherwise
# count. This helper and extract_section share that rule so they agree on what a
# heading is; a fenced comment must neither bind a section nor terminate one. A
# trailing CR is stripped so a CRLF ledger names its heading cleanly. No output
# means no match. A fence still open at end of file exits 4: every heading after
# it was hidden, and hiding is the silent drop this gate exists to refuse.
heading_matches() {
awk -v pattern="$1" '
/^#+[[:space:]]/ {
if (inside) { exit }
if (tolower($0) ~ pattern) { inside = 1; found = 1; next }
/^[[:space:]]*(```|~~~)/ { fenced = !fenced; next }

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track the actual Markdown fence delimiter

For valid nested-fence documentation, this parity toggle exposes headings that are still inside a code block. For example, a four-backtick fence may legitimately contain a three-backtick ledger example; the inner opening toggles fenced off, so an example ## Open-question register is counted as a second live heading and the gate exits 2. The same problem occurs when ~~~ appears inside a backtick fence. Track the opener's character and length, and only close it with a compatible delimiter.

Useful? React with 👍 / 👎.

@claude claude Bot Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor maintainability note: the fence-marker regex ^[[:space:]]*(\``|~~~)is now duplicated three times — here, inextract_section(line 92), and in the row loop (line 199, bash=~form). The comment above this function already flags thatheading_matchesandextract_section` "must agree on what a heading is"; a future change to fence detection (e.g. requiring matched fence length/char per GFM) has three call sites to update in lockstep, with nothing enforcing they stay in sync. Not blocking, but worth a shared constant or helper given how central "don't silently drift" is to this script's own design philosophy.

fenced { next }
/^#+[[:space:]]/ && tolower($0) ~ pattern {
heading = $0
sub(/\r$/, "", heading)
print NR "\t" heading
}
inside { print }
END { if (!found) { exit 3 } }
END { if (fenced) { exit 4 } }
' "$2"
}

# Count of, and comma-separated line numbers from, a heading_matches result.
match_count() { printf '%s\n' "$1" | wc -l | tr -d '[:space:]'; }
match_lines() { printf '%s\n' "$1" | cut -f1 | paste -sd, - | sed 's/,/, /g'; }

# Print a section body: every line after `start` (the matched heading's line
# number) up to the next unfenced heading of any level or end of file. Taking
# the line number rather than re-matching a pattern binds the body graded to the
# heading the caller already named in its diagnostics. A fence opened in the
# section and never closed exits 4: the row loop would otherwise skip every row
# after it as documentation and grade the register clean with a question hidden.
# A caller that ran heading_matches first never sees that exit (a heading only
# binds at even fence parity, so the whole-file check fires first); the guard is
# for a caller that extracts by line number without it.
extract_section() {
awk -v start="$1" '
NR <= start { next }
/^[[:space:]]*(```|~~~)/ { fenced = !fenced; print; next }
fenced { print; next }
/^#+[[:space:]]/ { exit }
{ print }
END { if (fenced) { exit 4 } }
' "$2"
}

Expand Down Expand Up @@ -115,13 +148,36 @@ if [[ "$brief_named" -eq 1 ]]; then
[[ -f "$brief" ]] || die_ungradeable "--brief not found: $brief"
fi

section="$(extract_section 'open-question register' "$ledger")"
awk_status=$?
register_matches="$(heading_matches 'open-question register' "$ledger")"
matches_status=$?
if [[ "$matches_status" -eq 4 ]]; then
die_ungradeable "unterminated fenced block in: $ledger (every heading after it is hidden; close the fence)"
elif [[ "$matches_status" -ne 0 ]]; then
die_ungradeable "could not read the headings of: $ledger"
fi
[[ -n "$register_matches" ]] || die_ungradeable "no '## Open-question register' section in: $ledger"

if [[ "$awk_status" -eq 3 ]]; then
die_ungradeable "no '## Open-question register' section in: $ledger"
elif [[ "$awk_status" -ne 0 ]]; then
die_ungradeable "could not read the register section from: $ledger"
# The gate reads exactly one section. Binding to the first match would grade a
# template's instructional copy (whose example rows parse as data) instead of
# the live register below it; binding to the last would guess. Either hides the
# ambiguity, so two matches are a refusal that names both.
register_count="$(match_count "$register_matches")"
if [[ "$register_count" -gt 1 ]]; then
die_ungradeable "$register_count headings match 'open-question register' in: $ledger (lines $(match_lines "$register_matches")); the gate reads exactly one section, so delete the template's instructional copy and keep the single live register"
fi

register_line="${register_matches%%$'\t'*}"
register_heading="${register_matches#*$'\t'}"
# Every register-derived error names the heading it bound to and its line, so a
# bind to the wrong section is visible from stderr alone.
where="register '$register_heading' at line $register_line"

section="$(extract_section "$register_line" "$ledger")"
extract_status=$?
if [[ "$extract_status" -eq 4 ]]; then
die_ungradeable "unterminated fenced block in the register section; rows after it would be skipped as documentation ($where in: $ledger)"
elif [[ "$extract_status" -ne 0 ]]; then
die_ungradeable "could not read the register section from: $ledger ($where)"
fi

registered=0
Expand Down Expand Up @@ -160,7 +216,7 @@ while IFS= read -r line; do
[[ "$line" =~ ^[[:space:]]*-[[:space:]]+[Qq][0-9]+([^0-9]|$) ]] || continue

if ! [[ "$line" =~ ^[[:space:]]*-[[:space:]]+[Qq][0-9]+[[:space:]]*\| ]]; then
die_ungradeable "malformed register row (needs 'Q<N> | status | round | question'): $line"
die_ungradeable "malformed register row (needs 'Q<N> | status | round | question'): $line ($where)"
fi

row="${line#*-}"
Expand All @@ -180,7 +236,7 @@ while IFS= read -r line; do
# reports NF as the number of `|` separators plus one.
separators="${row//[!|]/}"
if [[ "${#separators}" -lt 3 ]]; then
die_ungradeable "malformed register row (needs 'Q<N> | status | round | question'): $line"
die_ungradeable "malformed register row (needs 'Q<N> | status | round | question'): $line ($where)"
fi

# No leading zeros, and no Q0. `[[ ]]` numeric comparison evaluates its
Expand All @@ -191,13 +247,13 @@ while IFS= read -r line; do
# by the register's own contract anyway.
num="${id#[Qq]}"
if ! [[ "$num" =~ ^[1-9][0-9]*$ ]]; then
die_ungradeable "malformed question id (expected Q1, Q2, … with no leading zero): $id"
die_ungradeable "malformed question id (expected Q1, Q2, … with no leading zero): $id ($where)"
fi
# Normalize so `q3` and `Q3` collide as the same id.
id="Q$num"

case "$seen_ids" in
*" $id "*) die_ungradeable "duplicate question id: $id" ;;
*" $id "*) die_ungradeable "duplicate question id: $id ($where)" ;;
*) ;; # not seen before — fall through and register it
esac
seen_ids="$seen_ids$id "
Expand All @@ -206,7 +262,7 @@ while IFS= read -r line; do
# a gap is a row that went missing after it was written — the exact silent drop
# this gate is here to refuse. Ungradeable, never a pass.
if [[ "$num" -ne "$expected" ]]; then
die_ungradeable "non-contiguous question id: expected Q$expected, got $id"
die_ungradeable "non-contiguous question id: expected Q$expected, got $id ($where)"
fi
expected=$((expected + 1))

Expand All @@ -223,30 +279,48 @@ while IFS= read -r line; do
blocked=$((blocked + 1))
deferred_ids="$deferred_ids$id "
;;
*) die_ungradeable "unknown status '$status_field' in row: $line" ;;
*) die_ungradeable "unknown status '$status_field' in row: $line ($where)" ;;
esac
done <<<"$section"

if [[ "$registered" -eq 0 ]]; then
if [[ "$skipped_fenced_row" -eq 1 ]]; then
die_ungradeable "the register section holds no question rows in: $ledger (rows inside a fenced block are ignored by design; register rows must be unfenced)"
die_ungradeable "the register section holds no question rows in: $ledger ($where; rows inside a fenced block are ignored by design; register rows must be unfenced)"
fi
die_ungradeable "the register section holds no question rows in: $ledger"
die_ungradeable "the register section holds no question rows in: $ledger ($where)"
fi


brief_state="unchecked"
if [[ "$brief_named" -eq 1 ]]; then
deferred_section="$(extract_section 'deferred questions' "$brief")"
brief_awk=$?

if [[ "$brief_awk" -eq 3 ]]; then
brief_matches="$(heading_matches 'deferred questions' "$brief")"
brief_matches_status=$?
if [[ "$brief_matches_status" -eq 4 ]]; then
die_ungradeable "unterminated fenced block in: $brief (every heading after it is hidden; close the fence)"
Comment on lines 294 to +298

@claude claude Bot Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This heading_matches call (and its exit-4 check) runs unconditionally whenever --brief is named, before anything downstream knows whether the cross-check is even needed. If the ledger's register has zero deferred/blocked rows, deferred_ids is empty and the cross-check is a no-op — the zero-match branch two lines below (if [[ -z "$brief_matches" ]]; then ... if [[ -n "$deferred_ids" ]]) already accounts for that by skipping the "no section" refusal when nothing needs checking. But the unterminated-fence refusal on line 298 has no equivalent guard: it fires as soon as the Brief has any unclosed fence anywhere in the document, even in a section unrelated to "Deferred questions", even when there are no deferred/blocked ids to verify.

In practice a Brief is often a large planning document with its own code samples. A single unrelated stray unclosed fence in some other section would flip an otherwise-clean run (nothing deferred, ledger fully resolved) to status=ungradeable, which contradicts this gate's own stated goal of a narrowly-scoped mechanical check. Consider deferring the heading_matches/unterminated-fence check on the Brief until it's known that deferred_ids is non-empty, matching the short-circuit that already exists for the "no such heading" case right below it.

No test in check-open-questions.test.sh currently exercises "ledger has no deferred/blocked rows" + "Brief has an unrelated unterminated fence" — the existing mixed fixture (used by tests 30/31/36) always has deferred/blocked rows, so this gap isn't caught.

Fix this →

elif [[ "$brief_matches_status" -ne 0 ]]; then
die_ungradeable "could not read the headings of: $brief"
fi
brief_where=""
if [[ -z "$brief_matches" ]]; then
if [[ -n "$deferred_ids" ]]; then
die_ungradeable "no '### Deferred questions' section in: $brief (register retires:${deferred_ids% })"
fi
deferred_section=""
elif [[ "$brief_awk" -ne 0 ]]; then
die_ungradeable "could not read the deferred-questions section from: $brief"
else
# Same one-section rule as the register: two matches are a refusal, not a guess.
brief_count="$(match_count "$brief_matches")"
if [[ "$brief_count" -gt 1 ]]; then
die_ungradeable "$brief_count headings match 'deferred questions' in: $brief (lines $(match_lines "$brief_matches")); the gate reads exactly one section"
fi
brief_line="${brief_matches%%$'\t'*}"
brief_heading="${brief_matches#*$'\t'}"
brief_where=" (deferred questions '$brief_heading' at line $brief_line)"
deferred_section="$(extract_section "$brief_line" "$brief")"
brief_extract_status=$?
if [[ "$brief_extract_status" -eq 4 ]]; then
die_ungradeable "unterminated fenced block in the deferred-questions section of: $brief$brief_where"
elif [[ "$brief_extract_status" -ne 0 ]]; then
die_ungradeable "could not read the deferred-questions section from: $brief$brief_where"
fi
fi

missing=""
Expand All @@ -269,7 +343,7 @@ if [[ "$brief_named" -eq 1 ]]; then
fi
done
if [[ -n "$missing" ]]; then
die_ungradeable "deferred/blocked question(s) absent from the Brief's deferred questions: ${missing% }"
die_ungradeable "deferred/blocked question(s) absent from the Brief's deferred questions: ${missing% }$brief_where"
fi
brief_state="ok"
fi
Expand Down
Loading