Conversation
Add a pre-exit analysis phase that spawns an Opus agent to review development records from a methodology perspective. The agent produces a sanitized report (no project-specific info) and optionally helps the user file a GitHub issue with improvement suggestions. New --privacy flag disables the feature; legacy loops default to privacy=true (opt-in only for new loops). The phase integrates into all three non-manual exit paths (complete, stop, maxiter) using the established Finalize Phase pattern with state file renaming and a completion artifact gate.
Address Codex review findings: - Require methodology-analysis-report.md to exist before allowing completion (prevents silent no-op when Opus agent does not run) - Fail closed when .methodology-exit-reason marker is missing or invalid instead of defaulting to "complete" (prevents misreporting stop/maxiter loops as successful)
…y analysis The Opus analysis agent needs to read all round-*-summary.md and round-*-review-result.md files, but the read validator was blocking access to summaries from non-current rounds. Bypass the round number check when methodology-analysis-state.md is the active state file, while still requiring files to be within the active loop directory.
…d allowlist Address Codex review findings: - Use realpath to canonicalize paths before prefix check, preventing directory traversal attacks (e.g., LOOP_DIR/../sensitive-file) - Restrict allowed reads to an explicit allowlist of files the analysis agent actually needs: round summaries, review results, and its own artifacts. This prevents exposing plan.md, prompt files, and other project-specific loop metadata that would undercut sanitization.
Add methodology analysis restrictions to all four validators: - Read validator: restrict loop dir reads to allowlisted artifacts only - Write validator: block all writes except methodology report/done marker - Edit validator: block all edits except methodology report/done marker - Bash validator: block git write commands and in-place file editing tools This prevents source code modifications after Codex has signed off and prevents project-specific information from leaking into the analysis report.
All four validators now try unfiltered loop search when session-filtered search returns empty, so spawned agents (with different session_id) are also subject to methodology analysis restrictions. Bash validator now blocks: touch, mv, cp, rm, dd, truncate, chmod, chown, output redirection to non-/dev/ paths, and all git write commands.
Handle BSD/macOS where realpath fails for non-existent files by resolving the parent directory and appending the basename. This allows the initial Write to methodology-analysis-report.md to succeed. Narrow the bash gh allowlist from all gh commands to only gh issue subcommands, preventing workspace mutations via gh pr checkout, gh repo clone, or mutating gh api calls.
Read validator now blocks reads of files within the project root (not just loop dir) during methodology analysis, while still allowing system files outside the project (CLAUDE.md, configs). This prevents the analysis agent from accessing source code that could leak into the report. Bash validator now blocks common interpreter commands (python, ruby, node, perl, php) during methodology analysis as defense-in-depth against file write bypasses.
…ology analysis Add find_methodology_analysis_loop() that scans all loop directories for methodology-analysis-state.md instead of using the unfiltered find_active_loop fallback which only returns the newest active loop. This prevents spawned agents from binding to a wrong concurrent session during methodology analysis. Block shell script entry points (bash/sh/zsh, build tools, source/dot commands, direct script execution) in the bash validator during methodology analysis to prevent bypassing file modification restrictions via wrapper binaries.
…y exit Remove the gh issue early exit that short-circuited all subsequent methodology analysis checks. Commands like 'gh issue create; rm file' were bypassing blockers. Now all commands go through the full blocklist; pure gh issue commands pass naturally since they match no blocker. Add cancel-rlcr-loop.sh to the allowlist so the cancel command works during the methodology analysis phase. Document concurrent methodology analysis limitation in find_methodology_analysis_loop.
…bcommands Only fall back to find_methodology_analysis_loop when NO session-matched loop was found (spawned agent case). If a session has its own active loop, do NOT search for another session's methodology analysis state -- that would apply restrictions to an unrelated concurrent session. Add git restore, clean, rm, mv to the methodology analysis git command blocklist to prevent working tree modifications after Codex signoff.
…back Remove unfiltered find_methodology_analysis_loop fallback from all validators. The fallback incorrectly applied methodology analysis restrictions to unrelated sessions opened in the same repo. Now only the originating session (matched by session_id) gets restricted. Spawned agents rely on their prompt for guidance. Add raw path fallback when realpath is unavailable (older macOS/BSD) to prevent deadlock where the originating session cannot write completion artifacts.
…tor status Add git switch/pull/clone/submodule/worktree and mkdir/rmdir/ln/mktemp to the methodology analysis bash command blocklist. Require methodology-analysis-report.md to have content (not just exist) before allowing the methodology analysis phase to complete. Add methodology-analysis-state.md to statusline state file resolution and show "Analyzing" status. Treat methodology-analysis as active phase in monitor color.
…ead code Require cancel-rlcr-loop.sh to be a standalone command (no shell operators) to prevent chained commands from bypassing methodology analysis restrictions. Document that spawned agents are not restricted by hooks due to session_id mismatch -- their sanitization is enforced by the analysis prompt. This is an inherent limitation of the hook architecture. Remove unused find_methodology_analysis_loop function.
Remove round-*-summary.md and round-*-review-result.md from the methodology analysis Read allowlist. The originating session should only read the sanitized methodology-analysis-report.md, not raw development records that contain project-specific information. The spawned agent reads raw records directly (not restricted by hooks due to different session_id). Document that read-only bash commands are intentionally not blocked during methodology analysis -- blocking them would break basic operations. The analysis prompt is the primary enforcement mechanism for sanitization.
Update blocked-read messages to match actual allowlist (methodology-analysis-* files only). Previous messages incorrectly listed round-*-summary.md and round-*-review-result.md which were removed from the allowlist.
The template was referenced in loop-common.sh but never created, causing the CI template-references test to fail.
The cancel-rlcr-loop.sh allowlist in the bash validator matched when the script name appeared as an argument to another command (e.g. cp). Anchor the regex to the start of the command string so only direct invocations are allowed. Move the methodology analysis completion handler in the stop hook to run before the git-clean check. Writing methodology artifacts can make the working tree appear dirty when .humanize is tracked, which would block exit before the handler ever ran.
Replace hardcoded #!/bin/bash with #!/usr/bin/env bash for better portability across different Unix systems where bash may be installed in non-standard locations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove spurious blank stderr line from load_template on missing files - Add awk exit code propagation in render_template to surface failures - Extract _emit_fallback helper to deduplicate load_and_render_safe - Prevent append_template from appending empty content on missing templates - Extend validate_template_dir to check all required subdirs (plan, pr-loop) - Bump version to 1.14.1 Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com>
- add native Codex hook config and installer, wire RLCR/PR stop hooks, and document Codex setup and usage - fix BitLesson selector routing and update related skills/docs - simplify install-skill target handling and harden RLCR test mock argument parsing
![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: dbb88bcade
ℹ️ 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".
hooks/loop-codex-stop-hook.sh
Outdated
| # portable-timeout.sh already sourced above | ||
|
|
||
| # Disable native hooks for nested Codex reviewer calls to prevent Stop-hook recursion. | ||
| CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks) |
There was a problem hiding this comment.
This new flag is unconditionally added to nested Codex invocations, but the hook only validates that codex exists and never checks whether the installed CLI supports --disable codex_hooks; on older Codex builds, the nested codex call exits with an unknown-argument error, which causes RLCR stop handling to treat review as failed and repeatedly block loop/finalize exits. Add a feature/version probe (or a fallback path without this flag) before injecting it.
Useful? React with 👍 / 👎.
scripts/install-codex-hooks.sh
Outdated
| if not isinstance(stop_groups, list): | ||
| raise SystemExit(f"existing hooks config has invalid Stop array: {hooks_file}") | ||
|
|
||
| managed_pattern = re.compile(r"(^|/)humanize/hooks/(loop-codex-stop-hook\.sh|pr-loop-stop-hook\.sh)$") |
There was a problem hiding this comment.
The stale-hook cleanup regex hardcodes /humanize/hooks/, so installs that use the supported --runtime-root option with any other path (for example /opt/hz/...) will never match existing managed commands; rerunning the installer then appends duplicate loop/pr Stop hooks instead of replacing them, breaking idempotency for custom layouts.
Useful? React with 👍 / 👎.
Replace #!/bin/bash with #!/usr/bin/env bash for better portability across different Unix systems (e.g. NixOS) where bash may be installed in non-standard locations. Also fix remaining shebangs in files added after the original PR was created. Includes prepare_runtime_bin() helper for test PATH isolation.
…ation - Remove spurious blank stderr line from load_template - Add awk exit code propagation in render_template - Extract _emit_fallback helper to eliminate duplicated fallback logic - Guard append_template against appending empty content - Extend validate_template_dir to check all 5 required subdirectories
Add a pre-exit analysis that runs an independent agent to analyze development methodology before the RLCR loop fully exits. The analysis is sanitized of project-specific information and can optionally help file improvement suggestions. Feature is enabled by default; disable with --privacy flag on loop start. Includes validator enforcement during the analysis and monitor/statusline support for the analyzing status.
|
@codex review this PR as an entirety |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95a6b7b3ee
ℹ️ 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".
| local verdict_line | ||
| local verdict_value | ||
|
|
||
| verdict_line=$(printf '%s\n' "$review_content" | grep -Ei 'Mainline Progress Verdict:[[:space:]]*(ADVANCED|STALLED|REGRESSED)([^A-Za-z]|$)' | tail -1 || true) |
There was a problem hiding this comment.
extract_mainline_progress_verdict currently accepts any line containing one of the verdict tokens, so the instructional placeholder Mainline Progress Verdict: ADVANCED / STALLED / REGRESSED is parsed as advanced instead of being treated as invalid. In implementation rounds this can silently reset drift state (mainline_stall_count/drift_status) and bypass the intended replan/circuit-breaker flow whenever the reviewer echoes the format template rather than choosing a single verdict.
Useful? React with 👍 / 👎.
scripts/ask-gemini.sh
Outdated
| if ! command -v gemini &>/dev/null; then | ||
| echo "Error: 'gemini' command is not installed or not in PATH" >&2 | ||
| echo "" >&2 | ||
| echo "Please install Gemini CLI: npm install -g @anthropic-ai/gemini-cli or https://github.com/anthropics/gemini-cli" >&2 |
There was a problem hiding this comment.
When gemini is missing, this message points users to @anthropic-ai/gemini-cli and an anthropics GitHub path, which are not the official Gemini CLI distribution. Users who follow this instruction will still fail the prerequisite check and cannot use /humanize:ask-gemini until they discover the correct package manually.
Useful? React with 👍 / 👎.
…ll path Mainline verdict parser now rejects lines containing multiple verdict keywords (e.g. template placeholders listing all options) as unknown, preventing silent false-positive ADVANCED readings that reset drift counters. Fixed Gemini CLI install guidance to point to the correct package (@google/gemini-cli) and repository (google-gemini/gemini-cli).
|
@codex review this PR as an entirety |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd506f2233
ℹ️ 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".
| "hooks": [ | ||
| { | ||
| "type": "command", | ||
| "command": "{{HUMANIZE_RUNTIME_ROOT}}/hooks/loop-codex-stop-hook.sh", |
There was a problem hiding this comment.
The template emits the hook command as a bare path, so when {{HUMANIZE_RUNTIME_ROOT}} contains spaces (for example a custom CODEX_HOME path like /Users/John Doe/.codex), the command is split at runtime and Codex cannot execute the stop hook script. That breaks RLCR/PR exit gating in those installations; the command value should be shell-quoted or otherwise encoded as a single executable path.
Useful? React with 👍 / 👎.
| elif codex --help 2>&1 | grep -q -- '--disable'; then | ||
| CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks) | ||
| echo "yes" > "$_CODEX_FEATURE_CACHE" 2>/dev/null | ||
| else | ||
| echo "no" > "$_CODEX_FEATURE_CACHE" 2>/dev/null |
There was a problem hiding this comment.
The stop hook permanently caches no when codex --help does not contain --disable, which also happens when codex --help fails (wrapper CLIs, constrained test binaries, transient probe errors). In that case nested reviewer calls run without --disable codex_hooks, re-enabling recursive Stop-hook execution for the rest of the loop. Feature detection should avoid negative caching on probe failure (or probe a command path that reliably exposes exec flags).
Useful? React with 👍 / 👎.
Hook installer now shell-quotes command paths via shlex.quote() so runtime roots containing spaces do not split the command at execution. Mainline verdict parser rejects lines with multiple verdict keywords as unknown to prevent template placeholders from being parsed as a valid verdict. Corrected Gemini CLI install guidance to the actual package and repo.
|
@codex review this PR as an entirety |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
The PR loop workflow is superseded by the /loop command combined with GitHub PR review polling. This removes all PR loop implementation, tests, documentation, templates, and supporting scripts. Deleted PR-loop-only files: commands (start/cancel-pr-loop), hooks (pr-loop-stop-hook), scripts (setup/cancel-pr-loop, check-bot-reactions, check-pr-reviewer-status, fetch-pr-comments, poll-pr-reviews), prompt templates (pr-loop/), test fixtures (setup-fixture-mock-gh), and all PR loop test files. Cleaned PR loop references from mixed files: hook validators, loop library, template loader, bash validator, humanize.sh monitor, monitor-common.sh, codex hooks config, install script, RLCR setup mutual exclusion, SKILL.md, usage docs, and test harnesses. The install script retains pr-loop-stop-hook.sh in its managed pattern so upgrading users get stale hooks cleaned from their hooks.json.
… write failure When CLAUDE_PLUGIN_ROOT has a trailing slash, the command template produces double slashes (e.g. "humania//scripts/setup-rlcr-loop.sh"). The setup script normalizes its own path via cd+pwd (single slash), but tool_input.command preserves the original double-slash string. This causes the boundary-aware string match in loop-post-bash-hook.sh to always fail, so the .pending-session-id signal is never consumed and session_id is never written to state.md. With an empty session_id, find_active_loop() backward-compat logic matches any session, causing cross-instance stop hook hijacking when multiple Claude Code sessions run concurrently. Fix: normalize consecutive slashes with tr -s '/' before comparison. Fixes: #67 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ZenusZhang
force-pushed
the
dev
branch
2 times, most recently
from
16dbf50 to
a1f3614
Compare
Extends comment parsing to support three formats: - Classic: CMT:/ENDCMT (existing) - Short tag: <cmt></cmt> (new) - Long tag: <comment></comment> (new) All formats support inline and multi-line usage and can be mixed within the same file. Updated documentation and error messages to be format-agnostic. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
fix: normalize path slashes in PostToolUse hook to prevent session_id write failure
…T: marker
The find_comment_markers awk function used ":" as the delimiter to return
"marker:position" pairs. Since the CMT: marker itself contains a colon,
split("CMT::1", parts, ":") would parse incorrectly, producing
found_marker="CMT" instead of "CMT:" and an empty marker_pos. This caused
pos to never advance, resulting in an infinite loop that hung the CI for
nearly 5 hours.
Switch the internal delimiter from ":" to "|" which does not appear in any
comment marker string.
The PR renamed CMT-specific terminology to generic "comment" terminology in both refine-plan.md and validate-refine-plan-io.sh, but the test assertions in test-refine-plan.sh still referenced the old text. Update all 9 affected assertions to match the current documentation and error messages.
Add support for XML-style <cmt>comment</cmt> <comment>comment</comment> formats in refine-plan
|
@codex review this PR as an entirety |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e61b570532
ℹ️ 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".
scripts/install-codex-hooks.sh
Outdated
| if not isinstance(stop_groups, list): | ||
| raise SystemExit(f"existing hooks config has invalid Stop array: {hooks_file}") | ||
|
|
||
| managed_pattern = re.compile(r"(^|/)hooks/(loop-codex-stop-hook\.sh|pr-loop-stop-hook\.sh)(\s|$)") |
There was a problem hiding this comment.
The managed-hook cleanup regex only matches commands that end with whitespace or end-of-string after loop-codex-stop-hook.sh, so it misses commands that were shell-quoted (for example when runtime_root contains spaces and shlex.quote wraps the command in single quotes). In that case rerunning the installer keeps the old managed hook and appends a new one, resulting in duplicate Stop hooks and multiple stop-hook executions per session.
Useful? React with 👍 / 👎.
| if [[ "$FILE_PATH" == /* ]]; then | ||
| _ma_real_path="$FILE_PATH" | ||
| else | ||
| _ma_real_path="$PROJECT_ROOT/$FILE_PATH" |
There was a problem hiding this comment.
When realpath is unavailable, the fallback path check uses the raw requested path and then allows reads outside PROJECT_ROOT; this can be bypassed with a symlink located outside the project that points back into project files. During methodology-analysis phase that lets restricted project content be read despite the privacy guard, because the prefix check runs on the symlink path rather than the resolved target.
Useful? React with 👍 / 👎.
install-codex-hooks.sh: managed_pattern only matched commands ending with whitespace or end-of-string after the .sh filename, missing shell-quoted forms produced by shlex.quote when runtime_root contains spaces. Updated the trailing anchor to also accept quote characters so duplicate Stop hooks are not inserted on reinstall. loop-read-validator.sh: the realpath-unavailable fallback used the raw unresolved path for prefix checks, allowing a symlink outside the project root pointing to a project file to bypass the methodology analysis read restriction. Added a symlink check that blocks the read when the path cannot be canonicalized (fail closed).
Feed accumulated commit history and recent round summaries/reviews to Codex during each review round, completing the PID feedback model: - P (proportional): goal-tracker vs acceptance criteria - I (integral): accumulated commits + last 3 rounds history - D (derivative): current round summary vs prompt Validate BASE_COMMIT with cat-file -e before git log to prevent set -e crash on corrupted/stale state files. Add test-commit-history-section.sh covering all edge cases (9/9 pass). Signed-off-by: Chao Liu <chao.liu.zevorn@gmail.com>
feat: add integral context to RLCR review prompts
|
@codex review this PR as an entirety |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
7 participants
Summary
#!/usr/bin/env bash) across all shell scripts for NixOS and non-standard bash locations (PR Use portable shebang across all shell scripts #49)_emit_fallbackdedup,validate_template_direxpanded to all 5 subdirs (PR Harden template-loader error handling and reduce duplication #50)install-codex-hooks.sh) with idempotent managed-hook cleanupgen-planBug Fixes (post-merge review)
--disable codex_hooksbehind a feature probe for older Codex buildsdrift_statuspresence for legacy loop compatsed /Iwith portablegrep -oEiin verdict extractionupsert_state_fieldsawk to split on first=only (prevent value truncation)STATE_PRIVACY_MODEdefault toparse_state_file_strictcodex --helpfeature probe per loop to reduce latencyruntime_rootbefore template substitution in hook installerIMMUTABLE SECTIONheader.humanize/in global gitignoreTest Status
Included PRs