feat: add Claude plan capture and explicit PR sync#8
Merged
Conversation
Member
Author
📝 WalkthroughWalkthroughThis PR extends plan-to-git to support Claude Code hooks, external state storage, and PR-targeted syncing. It adds Claude transcript parsing and backfill, refactors shared history infrastructure, and introduces environment-variable configuration for state file paths. ChangesClaude Code Hook Support and External State Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/capture.rs (1)
119-161:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrack “plan found” separately from “plan added.”
captured_plansonly increments whenstate.add_plan(...)returnstrue. On a duplicate Stop event, a marked plan still looks like “no plan captured”, so this branch falls through into Claude transcript plan-mode recovery and then question extraction. That makes hook retries non-idempotent and can persist the wrong follow-up state.Suggested fix
- if let Some(message) = message.as_deref() { + let mut found_plan = false; + + if let Some(message) = message.as_deref() { for plan in extract_marked_plans(message) { + found_plan = true; let added = state.add_plan(NewPlanItem { source: hook_input.source, title: plan.title, @@ - if hook_input.source == AgentSource::Claude && captured_plans == 0 { + if hook_input.source == AgentSource::Claude && !found_plan { if let Some(plan) = hook_input .transcript_path .as_deref() .and_then(last_claude_plan_mode_plan_from_transcript) { + found_plan = true; let added = state.add_plan(NewPlanItem { source: hook_input.source, title: plan.title, @@ - if captured_plans == 0 { + if !found_plan { if let Some(message) = message.as_deref() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/capture.rs` around lines 119 - 161, The code currently only increments captured_plans when state.add_plan(...) returns true, conflating "plan found in input" with "plan actually added", which lets duplicate Stop events bypass idempotent logic; update the logic to track plan_found (or similar) separately from add_result: set plan_found = true whenever extract_marked_plans(message) yields a plan or when last_claude_plan_mode_plan_from_transcript(...) returns Some, then still call state.add_plan(NewPlanItem { ... }) and increment captured_plans or changed only if add_plan returned true; use plan_found (not captured_plans) to decide whether to skip Claude transcript recovery and question extraction so retries remain idempotent.src/codex_history.rs (1)
59-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCount
files_matchedonce per session file.
files_matchedis now part of the shared import outcome, but this branch increments it for every matchingsession_metarecord. That can overcount a single file and diverge from the Claude importer’s first-match-only behavior.Suggested fix
if event.get("type").and_then(Value::as_str) == Some("session_meta") { metadata = Some(parse_session_metadata(&event)); - file_matches = metadata + let matches = metadata .as_ref() .is_some_and(|session| session_matches_context(session, context)); - if file_matches { + if matches && !file_matches { outcome.files_matched += 1; } + file_matches = matches; continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex_history.rs` around lines 59 - 66, The current branch increments outcome.files_matched for every matching session_meta record, causing duplicate counts per session file; change the logic in the session_meta handling (around metadata, parse_session_metadata, session_matches_context and file_matches) to only increment outcome.files_matched when a session file transitions from not-matched to matched (i.e., detect the previous file_matches state and only increment when previous was false and current file_matches becomes true) so each session file is counted at most once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/capture.rs`:
- Around line 180-183: drain_relevant_questions currently empties the entire
state.pending_questions queue and allows a decision created under NewDecision {
source: hook_input.source, questions } to include questions from other agents;
change the flow so only questions matching the current source (and preferably
session/turn id) are drained/answered: either update drain_relevant_questions to
accept a filter parameter (e.g., source and optional session_id/turn_id) and
return only matching questions while leaving others in state.pending_questions,
or filter state.pending_questions in-place before calling
state.answer_pending_questions so you build NewDecision with questions that have
question.source == hook_input.source (and matching session/turn when available).
Ensure the same change is applied at the other location mentioned (around lines
314-323) to prevent cross-agent consumption.
In `@src/claude_history.rs`:
- Around line 241-248: The function claude_plan_mode_file currently uses
Path::starts_with (lexical) on the input path which can be fooled by .. segments
or symlinks; instead canonicalize both the input path and claude_home (using
fs::canonicalize) and only proceed if the canonicalized target starts_with the
canonicalized claude_home; on any canonicalization error return None, and then
read the file contents from the canonicalized path and pass them to
captured_plan_from_markdown (update references to path and claude_home in
claude_plan_mode_file accordingly).
In `@src/history.rs`:
- Around line 58-61: The current looks_like_rendered_plan_stack function returns
true if the content merely contains the three substrings, which can
false-positive on texts that mention those literals; change it to verify
ordering and proximity: locate the index of "## Agent Plan Stack" first, then
find START_MARKER after that index and END_MARKER after the START_MARKER index
(and optionally require a minimum separation or newline boundaries) so that
START_MARKER occurs after the header and END_MARKER occurs after START_MARKER;
update looks_like_rendered_plan_stack to use substring/index checks (using the
START_MARKER and END_MARKER symbols) rather than simple contains().
---
Outside diff comments:
In `@src/capture.rs`:
- Around line 119-161: The code currently only increments captured_plans when
state.add_plan(...) returns true, conflating "plan found in input" with "plan
actually added", which lets duplicate Stop events bypass idempotent logic;
update the logic to track plan_found (or similar) separately from add_result:
set plan_found = true whenever extract_marked_plans(message) yields a plan or
when last_claude_plan_mode_plan_from_transcript(...) returns Some, then still
call state.add_plan(NewPlanItem { ... }) and increment captured_plans or changed
only if add_plan returned true; use plan_found (not captured_plans) to decide
whether to skip Claude transcript recovery and question extraction so retries
remain idempotent.
In `@src/codex_history.rs`:
- Around line 59-66: The current branch increments outcome.files_matched for
every matching session_meta record, causing duplicate counts per session file;
change the logic in the session_meta handling (around metadata,
parse_session_metadata, session_matches_context and file_matches) to only
increment outcome.files_matched when a session file transitions from not-matched
to matched (i.e., detect the previous file_matches state and only increment when
previous was false and current file_matches becomes true) so each session file
is counted at most once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 01a88f37-222c-4caa-bf2a-b7c26b2146cf
📒 Files selected for processing (12)
README.mdchangelog.d/20260611_000000_claude_code_plan_capture.mdsrc/capture.rssrc/claude_history.rssrc/codex_history.rssrc/github.rssrc/history.rssrc/lib.rssrc/main.rssrc/state_path.rssrc/store.rstests/integration/cli.rs
Comment on lines
180
to
183
| let questions = drain_relevant_questions(&mut state.pending_questions); | ||
| if state.answer_pending_questions(NewDecision { | ||
| source: AgentSource::Codex, | ||
| source: hook_input.source, | ||
| questions, |
There was a problem hiding this comment.
Don’t let one agent answer another agent’s pending questions.
Now that Claude and Codex share the same state file, drain_relevant_questions empties the entire queue and UserPromptSubmit stores one decision under the current hook_input.source. A Claude prompt can therefore consume Codex questions, and vice versa, permanently mis-associating persisted decisions. Filter by source at minimum, and ideally by session/turn as well.
Also applies to: 314-323
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/capture.rs` around lines 180 - 183, drain_relevant_questions currently
empties the entire state.pending_questions queue and allows a decision created
under NewDecision { source: hook_input.source, questions } to include questions
from other agents; change the flow so only questions matching the current source
(and preferably session/turn id) are drained/answered: either update
drain_relevant_questions to accept a filter parameter (e.g., source and optional
session_id/turn_id) and return only matching questions while leaving others in
state.pending_questions, or filter state.pending_questions in-place before
calling state.answer_pending_questions so you build NewDecision with questions
that have question.source == hook_input.source (and matching session/turn when
available). Ensure the same change is applied at the other location mentioned
(around lines 314-323) to prevent cross-agent consumption.
Comment on lines
+241
to
+248
| fn claude_plan_mode_file(path: &Path, claude_home: &Path) -> Option<CapturedPlan> { | ||
| if !path.starts_with(claude_home) { | ||
| return None; | ||
| } | ||
|
|
||
| fs::read_to_string(path) | ||
| .ok() | ||
| .and_then(|content| captured_plan_from_markdown(&content)) |
There was a problem hiding this comment.
Canonicalize the artifact path before reading it.
Path::starts_with(claude_home) is only a lexical check. A crafted transcript can use .../.claude/../... segments or a symlink under claude_home to make import-claude read arbitrary local files, which then get persisted and potentially synced to GitHub. Resolve both paths first and verify the canonicalized target stays under the canonicalized Claude home.
Suggested fix
fn claude_plan_mode_file(path: &Path, claude_home: &Path) -> Option<CapturedPlan> {
- if !path.starts_with(claude_home) {
+ let canonical_home = claude_home.canonicalize().ok()?;
+ let canonical_path = path.canonicalize().ok()?;
+
+ if !canonical_path.starts_with(&canonical_home) {
return None;
}
- fs::read_to_string(path)
+ fs::read_to_string(canonical_path)
.ok()
.and_then(|content| captured_plan_from_markdown(&content))
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/claude_history.rs` around lines 241 - 248, The function
claude_plan_mode_file currently uses Path::starts_with (lexical) on the input
path which can be fooled by .. segments or symlinks; instead canonicalize both
the input path and claude_home (using fs::canonicalize) and only proceed if the
canonicalized target starts_with the canonicalized claude_home; on any
canonicalization error return None, and then read the file contents from the
canonicalized path and pass them to captured_plan_from_markdown (update
references to path and claude_home in claude_plan_mode_file accordingly).
Comment on lines
+58
to
+61
| pub fn looks_like_rendered_plan_stack(content: &str) -> bool { | ||
| content.contains("## Agent Plan Stack") | ||
| && content.contains(START_MARKER) | ||
| && content.contains(END_MARKER) |
There was a problem hiding this comment.
Tighten rendered-stack detection before skipping imports.
This helper skips any plan that merely mentions the stack header plus the start/end markers, so a legitimate plan describing those literals will be dropped by both importers.
Suggested fix
pub fn looks_like_rendered_plan_stack(content: &str) -> bool {
- content.contains("## Agent Plan Stack")
- && content.contains(START_MARKER)
- && content.contains(END_MARKER)
+ let trimmed = content.trim();
+ trimmed.starts_with(START_MARKER)
+ && trimmed.contains("\n## Agent Plan Stack")
+ && trimmed.ends_with(END_MARKER)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/history.rs` around lines 58 - 61, The current
looks_like_rendered_plan_stack function returns true if the content merely
contains the three substrings, which can false-positive on texts that mention
those literals; change it to verify ordering and proximity: locate the index of
"## Agent Plan Stack" first, then find START_MARKER after that index and
END_MARKER after the START_MARKER index (and optionally require a minimum
separation or newline boundaries) so that START_MARKER occurs after the header
and END_MARKER occurs after START_MARKER; update looks_like_rendered_plan_stack
to use substring/index checks (using the START_MARKER and END_MARKER symbols)
rather than simple contains().
skulidropek
changed the title
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
1 participant
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
import-claude, and Claude Plan Mode transcript backfill./tmp/plan-to-git/<repo-key>/.agent-plan.jsonwith env overrides.plan-to-git sync --pr <number>so callers can sync queued plans to an explicit PR when branch discovery is not enough.Dependency for ProverCoderAI/docker-git#398.
Verification
cargo fmt --checkcargo testcargo clippy --all-targets --all-features -- -D warningscargo build