Repository navigation
/commit stops at the commit; new issues always pass a duplicate check; #155 review fixes and doc re-sync - #163
Conversation
… duplicate check /commit ran the gates, committed, pushed, opened a pull request and merged it in one go, so PR #155 merged before Codex or Copilot had commented. It now commits only; --pr pushes and opens a pull request; merging is never a step of the skill and happens only when the owner says to merge that pull request. Issues as the project's memory: - /issues files findings as issues, lists what is open, and closes an issue with a comment on what was tried and how it was fixed. It shows every draft first. - scripts/file-issue.py is the one route for a new issue: several searches over open and closed issues, exit 3 until each candidate is reviewed (--checked), fails closed when a search cannot run, and records the check in the body. - issue-guard.py (PreToolUse, "if": "Bash(gh *)") denies a raw gh issue create, or a REST/GraphQL create, and points to the script; it costs nothing on other commands. - open-issues.py (SessionStart, opt-in with CLAUDE_ISSUES_AT_START=1) lists open issues' titles by the owner and collaborators at startup. - Battery 253 -> 296 cases; each piece qualified on a seeded fault; LEDGER rows. Fixes from the PR #155 review (Refs #156, #157, #158, #159, #160, #161): 05_figures.R checks the PDF device by opening it and stops if a figure was not written; /replication-package writes sessionInfo_stata.txt; "forked context" corrected where a fresh context is required; slide-qa exits 2 when math never typeset and reports every defect on a slide; quarto-critic cannot approve with major issues open. Documentation (Refs #162): 131 statements corrected, then a second independent pass over README, guide, CLAUDE.md and the landing page checked 865 statements and corrected 44 more; a third pass checked those corrections. /preregister now reads the paper type /interview-me actually writes; check-derived-counts counts translate-to-quarto's Phase 6.5. Guide re-rendered and stamped. Gates: ./scripts/backtest.sh exits 0 (10 gates, 296 battery cases); guide and 05_figures.R score 100/100.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
unresolved critical and moderate findings affect issue-creation safety, data disclosure, cleanup, and runtime behavior.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
This PR separates /commit from PR creation and merging, adds duplicate-checked issue workflows and hooks, and synchronizes related documentation and tooling.
Changes:
/commitnow stops after committing;--prenables pushing and opening a PR.- Adds
/issues, duplicate checking, issue guards, and opt-in startup issue listing. - Updates slide QA, R figure validation, repository conventions, and documentation.
| File | Summary | Final review notes |
|---|---|---|
TROUBLESHOOTING.md |
Updated troubleshooting guidance. | — |
templates/skill-template.md |
Updated persistence and skill-count guidance. | — |
templates/PROVENANCE.md |
Moves provenance output under quality_reports/. |
— |
templates/preregistration-template.md |
Corrects repository wording. | — |
scripts/slide-qa.py |
Reports multiple defects and fails on unrendered math. | — |
scripts/R/README.md |
Clarifies pipeline execution and verification. | — |
scripts/R/05_figures.R |
Adds graphics-device probing and artifact validation. | — |
scripts/hook-battery.sh |
Adds issue-workflow qualification cases. | — |
scripts/file-issue.py |
Adds duplicate-checked issue creation. | Critical (1 vote): raw search queries can leak restricted paths. Moderate (3 votes): temporary issue-body files are not cleaned up. |
scripts/check-repo-hygiene.py |
Allows replication-package output. | — |
scripts/check-derived-counts.py |
Corrects fractional phase counting. | — |
README.md |
Synchronizes counts, defaults, and workflow descriptions. | Nit (1 vote): the guide does not reflect the newly advertised Phase 6.5. |
Preambles/README.md |
Clarifies palette and TikZ conventions. | — |
docs/index.html |
Synchronizes landing-page counts and gate wording. | — |
CLAUDE.md |
Updates commands, issue workflow, and file-size guidance. | — |
CHANGELOG.md |
Records commit, issue, QA, and documentation changes. | — |
.render-stamp |
Updates rendered-guide fingerprints. | — |
.github/CONTRIBUTING.md |
Updates surface-sync instructions. | — |
.claude/skills/syllabus/SKILL.md |
Moves default syllabus output. | — |
.claude/skills/stata-replication/SKILL.md |
Updates monitoring instructions. | Moderate (1 vote): the required Monitor tool is absent from allowed-tools. |
.claude/skills/simulation-study/SKILL.md |
Documents log-based monitoring. | — |
.claude/skills/seven-pass-review/SKILL.md |
Adds issue-tracking guidance. | — |
.claude/skills/scaffold-exercises/SKILL.md |
Moves default outputs under quality reports. | — |
.claude/skills/review-paper/SKILL.md |
Adds issue-tracking guidance. | — |
.claude/skills/respond-to-referees/SKILL.md |
Standardizes report output paths. | — |
.claude/skills/respond-to-eval/SKILL.md |
Updates verifier terminology. | — |
.claude/skills/replication-package/SKILL.md |
Corrects the Stata environment filename. | Nit (1 vote): published guide copies still document the stale filename. |
.claude/skills/r-package-check/SKILL.md |
Updates long-running check guidance. | — |
.claude/skills/qa-quarto/SKILL.md |
Requires no major or critical findings for approval. | — |
.claude/skills/promote-memory/SKILL.md |
Clarifies native-memory inputs. | — |
.claude/skills/preregister/SKILL.md |
Corrects paper-type and verifier wording. | — |
.claude/skills/new-skill/SKILL.md |
Updates integrity and surface-sync guidance. | — |
.claude/skills/issues/SKILL.md |
Adds issue filing, listing, and closure workflow. | — |
.claude/skills/grant-proposal/SKILL.md |
Corrects paper-type extraction. | — |
.claude/skills/deep-audit/SKILL.md |
Updates orchestration and model guidance. | — |
.claude/skills/deep-audit/references/repo-infrastructure-audit.md |
Expands mechanical audit guidance. | — |
.claude/skills/data-management-plan/SKILL.md |
Clarifies delegated skill usage. | — |
.claude/skills/data-analysis/SKILL.md |
Documents log-based monitoring. | — |
.claude/skills/compress-session/SKILL.md |
Clarifies memory and hook behavior. | — |
.claude/skills/commit/SKILL.md |
Separates commit, PR, and merge operations. | — |
.claude/skills/coauthor-brief/SKILL.md |
Updates commit and pipeline guidance. | — |
.claude/skills/adjudicate-review/SKILL.md |
Adds issue-tracking guidance. | — |
.claude/settings.json |
Wires issue guard and optional issue listing. | — |
.claude/rules/verification-protocol.md |
Integrates browser-based slide QA. | — |
.claude/rules/tikz-prevention.md |
Uses the shared TikZ checker. | — |
.claude/rules/tikz-measurement.md |
References the shared checker. | — |
.claude/rules/review-fencing.md |
Expands fenced answer-key files. | — |
.claude/rules/repo-hygiene.md |
Refines draft-name detection. | — |
.claude/rules/replication-protocol.md |
Updates passport integration. | — |
.claude/rules/r-code-conventions.md |
Corrects palette and scoring wording. | — |
.claude/rules/proofreading-protocol.md |
Clarifies report ownership. | — |
.claude/rules/progress-reports.md |
Documents ignored quality-report paths. | Nit (1 vote): two phrases describing git add -f are grammatically malformed. |
.claude/rules/post-flight-verification.md |
Updates verifier and opt-out guidance. | Nit (1 vote): the named skill list is incomplete. |
.claude/rules/orchestrator-protocol.md |
Updates deep-audit terminology. | — |
.claude/rules/model-routing.md |
Corrects current agent responsibilities. | — |
.claude/rules/meta-governance.md |
Clarifies auto-memory limits. | — |
.claude/rules/issue-ledger.md |
Defines issue and closure requirements. | — |
.claude/references/verification-ladder.md |
Corrects independence terminology. | — |
.claude/references/theory-proving.md |
Updates convergence wording. | — |
.claude/references/scheduled-routines.md |
Updates routine count. | — |
.claude/references/research-agent-laws.md |
Defines two-strikes identity. | Nit (1 vote): the documented stable locus identity does not match the implemented line-based ID. |
.claude/references/provenance-and-ground-truth.md |
Moves provenance location. | — |
.claude/references/orchestration-schemas.md |
Clarifies fresh-agent contexts. | — |
.claude/references/discipline-cards.md |
Corrects consumer documentation. | — |
.claude/references/audit-pet-peeves.md |
Updates checker references. | — |
.claude/hooks/open-issues.py |
Adds opt-in startup issue listing. | — |
.claude/hooks/issue-guard.py |
Blocks unchecked issue creation. | Critical (1 vote): shell control constructs can bypass the guard despite its documented coverage. Nit (1 vote): the documented /path/to/gh limitation is inaccurate. |
.claude/hooks/git-guardrails.py |
Updates commit workflow documentation. | — |
.claude/agents/verifier.md |
Clarifies R pipeline verification. | — |
.claude/agents/quarto-critic.md |
Corrects approval criteria. | — |
.claude/agents/promote-memory-council.md |
Corrects context terminology. | — |
.claude/agents/humanize-auditor.md |
Clarifies voice-profile location. | — |
.claude/agents/claim-verifier.md |
Clarifies fresh-context terminology. | — |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0425fd64dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| joined = " ".join(args) | ||
| if len(pos) >= 2 and pos[1] == "graphql": | ||
| return "createIssue" in joined |
There was a problem hiding this comment.
A GraphQL issue mutation is allowed whenever the mutation text is outside the command line, for example gh api graphql --input payload.json or gh api graphql -F query=@mutation.graphql, because this branch only searches the argument string for createIssue. The checked gh api --help confirms that --input reads a pre-constructed request body and that a field value beginning with @ is read from a file, so either form can create an issue without running scripts/file-issue.py; conservatively deny file-backed GraphQL payloads unless they can be proven not to contain createIssue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in baef832 (#164). For gh api graphql the guard now reads the query file named by --input FILE or -F key=@file and denies when it holds createIssue; stdin (@-) and a file it cannot read are denied, since they cannot be checked. A read query from a file stays allowed. Pinned by g27–g29, control g32.
| for m in HEREDOC.finditer(line): | ||
| delim = m.group(2) | ||
| while i < len(lines) and lines[i].strip() != delim: | ||
| i += 1 | ||
| i += 1 # the closing delimiter line |
There was a problem hiding this comment.
The heredoc scan treats <<WORD inside quoted strings and comments as a real opener and removes subsequent commands from inspection. For example, gh --version # use <<EOF\ngh issue create -t x -b y\nEOF executes the issue creation in Bash, but this function strips that line and the hook emits no denial; this bypasses the duplicate check in an ordinary compound command that is guaranteed to trigger the gh hook filter. Make heredoc recognition quote- and comment-aware before discarding bodies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
…L query files; issue bodies no longer record search text PR #163 review (Copilot, Codex), Refs #164. Each finding was reproduced first. - issue-guard: skips if/then/else/while/until/do/{/! before gh; reads a GraphQL query file (--input FILE, -F key=@file) and denies stdin or an unreadable file; honours a heredoc opener only outside quotes and comments. - file-issue.py: the issue body records how many searches of each kind ran, not their text; the temp file holding the body is removed after a create. - Battery 296 -> 308 cases; the pre-fix versions turn red exactly g23-g31, i9, i10. Backtest exits 0.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baef832cd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if c == "#" and (i == 0 or command[i - 1] in " \t;&|()"): | ||
| in_comment = True | ||
| i += 1 |
There was a problem hiding this comment.
When a comment begins at column zero after the first line, command[i - 1] is "\n", so this condition does not enter comment mode. Fresh evidence beyond the earlier heredoc thread is gh --version\n# <<EOF\ngh issue create -t x\nEOF: the scanner treats the commented <<EOF as an opener, removes the real create line as heredoc data, and allows the issue creation. Include a preceding newline among the valid comment boundaries.
Useful? React with 👍 / 👎.
| elif a in GQL_FILE_FLAGS and i + 1 < len(args) and "=@" in args[i + 1]: | ||
| paths.append(args[i + 1].split("=@", 1)[1]) | ||
| elif a.startswith("--field=") and "=@" in a[len("--field="):]: | ||
| paths.append(a[len("--field="):].split("=@", 1)[1]) |
There was a problem hiding this comment.
-F values before allowing GraphQL
The new file scan recognizes -F query=@file but not the equivalent attached short-option form -Fquery=@file. Fresh evidence beyond the prior GraphQL thread is that local gh 2.96.0 accepts gh api graphql -Fquery=@mutation.graphql and proceeds to the HTTP request; gh api --help identifies this option as -F, --field key=value. Because the argument does not itself contain createIssue, a mutation loaded through this accepted form bypasses the guard.
Useful? React with 👍 / 👎.
| # Shell reserved words that can stand before a command in the same segment | ||
| # (`if true; then gh issue create; fi` splits into `then gh issue create`). | ||
| RESERVED = {"if", "then", "else", "elif", "while", "until", "do", "{", "!"} |
There was a problem hiding this comment.
When a compound command declares and invokes a function, such as function f { gh issue create -t x; }; f, the issue is created but the first segment starts with the unrecognized function reserved word, so strip_prefix never reaches gh; the later invocation segment contains only f. This leaves a normal inline Bash control construct uncovered by the new control-word handling.
Useful? React with 👍 / 👎.
| elif w in WRAPPERS: | ||
| i += 1 | ||
| while i < len(words) and words[i].startswith("-"): | ||
| i += 1 # env -i, sudo -E, ... |
| for p in list(dict.fromkeys(PATH.findall(title + "\n" + body)))[:3]: | ||
| qs.append(("file paths", f'"{p}"')) |
There was a problem hiding this comment.
Declining, with reasons. A search request is sent to the GitHub API but is not published; what is published is the issue body, and the path queries are drawn from that same body the author is about to post. The rule that matters sits one step earlier: restricted paths must not be in the draft at all (issue-ledger.md, confidential-data.md, and the /issues skill's constraints). Moving the protection to the search step would still leave the body carrying the path.
| - Create new persistent source files (`/create-lecture` → new `.tex`, `/new-diagram` → new TikZ source). | ||
| - Write a self-modifying artifact (`/learn` → new SKILL.md, `/checkpoint` → state snapshot, `/preregister` → preregistration document). | ||
| - Run a destructive cycle (`/deep-audit` → repo-wide fix loop). | ||
| - Run a long, file-writing cycle (`/simulation-study` → seeded Monte Carlo run and results, `/vaccinate` → qualification-ledger row). |
There was a problem hiding this comment.
Confirmed and fixed in 7650048: /deep-audit had lost disable-model-invocation: true when the v2.5 verification skills were merged. The flag is restored, so a vague 'audit this' cannot start its fix loop, and the skill template lists it again.
| - **Gates:** `./scripts/backtest.sh` passes all 10 gates, including the hook battery | ||
| (253 cases, seconds to run; the new case fails against the hook's old watch pattern). | ||
| (308 cases, seconds to run; the new case fails against the hook's old watch pattern). |
There was a problem hiding this comment.
Confirmed: the PR description is updated to the current count (312 cases after the second-round fixes).
… function bodies and sudo -u; /deep-audit is user-invoked again Second PR #163 review (Codex, Copilot), Refs #164. Each finding reproduced first. - issue-guard: a # at the start of a later line starts a comment; the attached -Fquery=@file form is read; 'function NAME' is skipped; wrapper options that take a value (sudo -u USER, env -u VAR) are skipped. - /deep-audit regains disable-model-invocation: true (lost in the v2.5 merge), so a vague 'audit this' cannot start its repo-wide fix loop; the skill template lists it again. - Battery 308 -> 312 cases; the previous guard turns red exactly g33-g36. Backtest exits 0.
Summary
/commitstops at the commit. It no longer pushes, opens a pull request and merges in one step (that is how v2.6.0: Opus 5.5 currency, browser slide QA, verified quotes, append-only research logs #155 merged before any reviewer commented).--prpushes and opens a PR; a merge happens only when the owner says to merge./issuesskill; newscripts/file-issue.py(the one route for creating an issue: several searches over open and closed issues, refuses until each candidate is reviewed, fails closed); newissue-guard.pyhook that denies a rawgh issue createand points to the script (spawned only forghcommands); new opt-inopen-issues.pyhook that lists open issues at startup.CLAUDE.mdand the landing page checked 865 statements and corrected 44 more; a third pass checked those corrections.Changed defaults
/commitcommits only. Pushing and opening a PR need--pr(or asking); it never merges.issue-guard.py, deniesgh issue create/gh issue new/ REST and GraphQL issue creates run by Claude, and points topython3 scripts/file-issue.py. Uses the hooksiffield (Claude Code ≥ 2.1.85); on older versions it runs on every Bash call, still correctly.open-issues.py, is wired but off unlessCLAUDE_ISSUES_AT_START=1./deep-auditis user-invoked again (disable-model-invocation: true, lost in the v2.5 merge).Type
fix/...)feat/...)docs/...)chore/...)Test plan
./scripts/validate-setup.shexits 0./scripts/backtest.shexits 0 — all 10 gates, hook battery 312 casespython3 scripts/quality_score.py—guide/workflow-guide.qmd100/100,scripts/R/05_figures.R100/100quality_reports/qualification/LEDGER.mdfile-issue.py --dry-runwith a differently worded title found 05_figures.R reports writing fig_main.pdf when the PDF device failed, and leaves Rplots.pdf behind #156;open-issues.pylisted the open issues in 0.6 s; the guard blocked a real raw create in session00_run_all.Rin a scratch copy writesfig_main.pdf, noRplots.pdf. slide-qa exits 0 when a deck's math was never typeset #159/slide-qa hides a slide's clipped content when the same slide also overflows #160:slide-qa.pyre-run in Chrome (math-off deck exits 2; two-defect slide reportsoverflow + clipped)README.mdandguide/workflow-guide.qmd; guide re-rendered,docs/synced, stampedcheck-surface-syncpasses (61 skills, 11 hooks, README skills-table row added)/deep-audit— not run as such; the three-pass documentation review above stands in for it./scripts/install-hooks.sh— not installed on this machine; the backtest was run by handGenerality (for new skills/agents/rules)
Notes for reviewer
Two bot review rounds (Copilot, Codex) raised 12 findings; 11 were reproduced and fixed (tracked in PR #163 review: the issue guard misses shell control words, file-based GraphQL and quoted heredoc openers; file-issue.py over-records #164: shell control words, file-based GraphQL, heredoc openers in quotes/comments, attached
-F,function,sudo -u, search text in issue bodies, temp body file,/deep-auditinvocation, the case count); 1 was declined with reasons on its thread.The guard is a tripwire, not a security boundary:
/path/to/gh,sh -c '…'or a script can still create an issue. It stops the form Claude usually writes. Its heredoc and"$(…)"handling are pinned by battery cases g5, g10 and g17.Issues 05_figures.R reports writing fig_main.pdf when the PDF device failed, and leaves Rplots.pdf behind #156–131 documentation statements are out of sync with the v2.6.0 repository #162 are referenced, not auto-closed; they are closed by hand with a closing comment after merge.
Windows path handling (Windows: three POSIX-path assumptions — one silently disarms root-of-trust-guard (a54), two produce false gate failures #151, PR Fix Windows path handling in the gates, two guards, and the hook battery #152) is deliberately out of scope here.