Add methodology analysis phase for RLCR loop exit - #42
Merged
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.
![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: 2cba950a86
ℹ️ 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".
Comment on lines
+90
to
+91
| if echo "$COMMAND_LOWER" | grep -qE '(^|[[:space:]])([^[:space:]]*/)?cancel-rlcr-loop\.sh' && \ | ||
| ! echo "$COMMAND_LOWER" | grep -qE '[;|&]'; then |
There was a problem hiding this comment.
The methodology-phase allowlist returns early whenever the command string merely contains a cancel-rlcr-loop.sh token, so non-cancel write commands can bypass every later restriction. For example, cp /path/cancel-rlcr-loop.sh /tmp/x matches this regex and exits 0 before the file-modification/redirection blocks run, which defeats the “no modifications after sign-off” guarantee in this phase.
Useful? React with 👍 / 👎.
Comment on lines
+693
to
+696
| if [[ "$IS_METHODOLOGY_ANALYSIS_PHASE" == "true" ]]; then | ||
| if complete_methodology_analysis; then | ||
| # Analysis complete, allow exit | ||
| exit 0 |
There was a problem hiding this comment.
The new methodology completion handler is placed after the git-clean/push checks, so methodology-phase exits can still be blocked by git status before complete_methodology_analysis runs. In repos where .humanize files are tracked, writing methodology-analysis-report.md / methodology-analysis-done.md makes the tree dirty, and the session can get stuck in repeated “please commit first” blocks instead of progressing through the analysis-completion path.
Useful? React with 👍 / 👎.
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.
Contributor
|
We'd better allow users to use agents from different vendors that is configured in config system. |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--privacyflag on loop startChanges
hooks/lib/methodology-analysis.sh,prompt-template/claude/methodology-analysis-prompt.md--privacyflag support, cancel during methodology analysis phaseTest plan
--privacy, verify all 3 exit paths skip methodology analysis--privacy, verify methodology analysis phase is entered after finalize