feat: add plan-to-git CLI#3
Conversation
|
Warning Review limit reached
More reviews will be available in 55 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughConverts the template crate into the plan-to-git CLI: captures explicitly marked agent plans from Codex hooks, redacts secrets, persists a deduplicated Agent Plan Stack to ChangesPlan-to-Git MVP
Sequence DiagramsequenceDiagram
participant CodexHook
participant plan_to_git_CLI
participant GitCLI
participant AgentPlanState
participant gh_CLI
CodexHook->>plan_to_git_CLI: send hook JSON (stdin)
plan_to_git_CLI->>GitCLI: git rev-parse / branch / head / remote (discover)
plan_to_git_CLI->>AgentPlanState: load_state(.agent-plan.json)
plan_to_git_CLI->>AgentPlanState: add_plan / add_pending_question / answer_pending_questions
AgentPlanState-->>plan_to_git_CLI: save_state (if changed)
plan_to_git_CLI->>gh_CLI: gh pr view --json number
plan_to_git_CLI->>plan_to_git_CLI: render_plan_comment for unposted items
plan_to_git_CLI->>gh_CLI: gh api --method POST repos/{repo}/issues/{pr}/comments --input <file>
gh_CLI-->>plan_to_git_CLI: returned comment id
plan_to_git_CLI->>AgentPlanState: mark_items_commented (record comment)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
AI Session BackupCommit: 415c276
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/capture.rs (1)
124-134: 💤 Low valueFunction name overstates selectivity — it drains all pending questions.
drain_relevant_questionsusesdrain(..)and applies no relevance filter; every outstanding question set is consumed and answered by a single prompt. If indiscriminate draining is intended, consider renaming to reflect that (e.g.drain_all_pending_questions); otherwise add the relevance filtering the name implies.🤖 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 124 - 134, The function drain_relevant_questions currently consumes every PendingQuestion via pending_questions.drain(..) but applies no relevance filter, so either rename it to reflect the behavior (e.g., drain_all_pending_questions) or implement true relevance filtering: keep the name and add a predicate that inspects each PendingQuestion (or its questions) to decide relevancy before removing/collecting them (use retain or selective drain with drain_filter or manual iteration), ensure duplicates are still deduplicated as before, and update any call sites that rely on the old name/semantics (reference function drain_relevant_questions, the pending_questions.drain(..) usage, and the PendingQuestion.questions field).tests/integration/cli.rs (1)
32-56: 💤 Low valueOptional: reuse the
run_hookhelper here.This block duplicates the spawn/write/wait logic already in
run_hook(Lines 104-126). You could callrun_hook(&repo_dir, &bin_dir, &payload)and then read the state file, keeping the two tests consistent.🤖 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 `@tests/integration/cli.rs` around lines 32 - 56, The test duplicates the spawn/write/wait logic already implemented in run_hook; replace the manual Command creation block in the test with a call to run_hook(&repo_dir, &bin_dir, &payload) and then proceed to read STATE_FILE_NAME and assert on its contents (e.g., assert!(state.contains("Capture plan"))), so both tests reuse the shared helper and remain consistent.src/main.rs (1)
50-63: ⚡ Quick winHook failures won’t block Codex unless the hook exits with code 2
This
Hookerror path only logs to stderr and doesn’t set a failure exit code, so Codex won’t hit its “block” gate (Codex blocks only when the hook exits with code2; other non-zero codes are non-blocking). If hook failures should block the agent run, exit/return with2onrun_hookerrors; otherwise the current behavior matches Codex’s semantics.🤖 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/main.rs` around lines 50 - 63, The Hook arm currently only prints errors from run_hook and does not cause a blocking exit; update the Commands::Hook { source } branch so that when run_hook(*source) returns Err it exits with code 2 (e.g., call std::process::exit(2) or propagate an Err that main maps to exit code 2) instead of merely logging, so hook failures trigger Codex’s blocking behavior; locate the match arm handling Commands::Hook and the run_hook call to apply this change.
🤖 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.
Nitpick comments:
In `@src/capture.rs`:
- Around line 124-134: The function drain_relevant_questions currently consumes
every PendingQuestion via pending_questions.drain(..) but applies no relevance
filter, so either rename it to reflect the behavior (e.g.,
drain_all_pending_questions) or implement true relevance filtering: keep the
name and add a predicate that inspects each PendingQuestion (or its questions)
to decide relevancy before removing/collecting them (use retain or selective
drain with drain_filter or manual iteration), ensure duplicates are still
deduplicated as before, and update any call sites that rely on the old
name/semantics (reference function drain_relevant_questions, the
pending_questions.drain(..) usage, and the PendingQuestion.questions field).
In `@src/main.rs`:
- Around line 50-63: The Hook arm currently only prints errors from run_hook and
does not cause a blocking exit; update the Commands::Hook { source } branch so
that when run_hook(*source) returns Err it exits with code 2 (e.g., call
std::process::exit(2) or propagate an Err that main maps to exit code 2) instead
of merely logging, so hook failures trigger Codex’s blocking behavior; locate
the match arm handling Commands::Hook and the run_hook call to apply this
change.
In `@tests/integration/cli.rs`:
- Around line 32-56: The test duplicates the spawn/write/wait logic already
implemented in run_hook; replace the manual Command creation block in the test
with a call to run_hook(&repo_dir, &bin_dir, &payload) and then proceed to read
STATE_FILE_NAME and assert on its contents (e.g.,
assert!(state.contains("Capture plan"))), so both tests reuse the shared helper
and remain consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 21ed2029-89d8-4f61-8bcf-f4a429969899
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.gitignoreCargo.tomlREADME.mdchangelog.d/20260530_104500_plan_to_git_mvp.mdexamples/basic_usage.rssrc/capture.rssrc/error.rssrc/git.rssrc/github.rssrc/lib.rssrc/main.rssrc/normalize.rssrc/pr_body.rssrc/redact.rssrc/render.rssrc/store.rssrc/sum.rstests/integration/cli.rstests/integration/mod.rstests/integration/sum.rstests/unit/mod.rstests/unit/plan_capture.rstests/unit/sum.rs
💤 Files with no reviewable changes (3)
- tests/integration/sum.rs
- tests/unit/sum.rs
- src/sum.rs
AI Session BackupCommit: 94587af
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/github.rs (1)
89-101:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCleanup failure can mask a successful PR update.
If
gh api ... PATCHsucceeds butfs::remove_filefails, the earlyreturn Err(error.into())reports the sync as failed even though the PR body was updated. Treat temp-file cleanup as best-effort and decide success from the command status.🐛 Proposed fix
- let remove_result = fs::remove_file(&request_file); - let output = output?; - if let Err(error) = remove_result { - return Err(error.into()); - } - - if output.status.success() { - return Ok(()); - } + let _ = fs::remove_file(&request_file); + let output = output?; + if output.status.success() { + return Ok(()); + }🤖 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/github.rs` around lines 89 - 101, The cleanup error handling currently returns early on fs::remove_file(&request_file) (remove_result) before checking the gh command result (output.status.success()), which can report failure even when the PR update succeeded; change the control flow so you first inspect output.status.success() to determine overall success, then attempt fs::remove_file(&request_file) as a best-effort cleanup — if the command succeeded but remove_file returns Err, do not convert that into an overall Err (instead log or ignore the cleanup error), and only propagate errors from the gh command (using stderr and AppError::new) when output.status.success() is false; update the branches around remove_result, output, and the final Err(AppError::new(...)) to reflect this ordering.src/capture.rs (1)
110-114:⚠️ Potential issue | 🟠 MajorAvoid blocking synchronous hooks with untimed
ghsync callsIn
src/capture.rs(process_codex_hook, ~110-114),github::sync_state(&context, &state)is called on everyStop/UserPromptSubmithandling path. When there are current-branch items,sync_staterunsgh pr view --json number,body(and potentiallygh api ... PATCH) viaCommand::output()with no timeout, so a slow/hungghcan stall the user session.
- Gate
sync_statebehindchanged(and/or only when the rendered plan block could differ) or move it off the hook critical path.- Enforce a timeout around the
ghprocess externally (sincegh pr view/gh apidon’t provide a request-timeout flag) or implement a Rust-side process timeout.Minor:
drain_relevant_questionsdrains & deduplicates all pending questions (no “relevance” filtering), so the name is misleading.🤖 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 110 - 114, The current hook handler process_codex_hook always calls github::sync_state(&context, &state) which can block the hook because github::sync_state runs gh CLI commands without a timeout; only call github::sync_state when state actually changed or when the rendered plan block could differ (i.e., gate the call behind the existing changed boolean or a specific plan-diff check), or defer it off the critical path by spawning it as a background task/thread so the hook returns immediately. Additionally, implement a Rust-side timeout inside github::sync_state for any Command::output() calls that invoke gh (or switch to a process API that supports wait-with-timeout) to avoid hangs, and consider renaming drain_relevant_questions to a name reflecting its behavior (e.g., drain_and_dedupe_pending_questions) to match its actual logic.
🤖 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.
Outside diff comments:
In `@src/capture.rs`:
- Around line 110-114: The current hook handler process_codex_hook always calls
github::sync_state(&context, &state) which can block the hook because
github::sync_state runs gh CLI commands without a timeout; only call
github::sync_state when state actually changed or when the rendered plan block
could differ (i.e., gate the call behind the existing changed boolean or a
specific plan-diff check), or defer it off the critical path by spawning it as a
background task/thread so the hook returns immediately. Additionally, implement
a Rust-side timeout inside github::sync_state for any Command::output() calls
that invoke gh (or switch to a process API that supports wait-with-timeout) to
avoid hangs, and consider renaming drain_relevant_questions to a name reflecting
its behavior (e.g., drain_and_dedupe_pending_questions) to match its actual
logic.
In `@src/github.rs`:
- Around line 89-101: The cleanup error handling currently returns early on
fs::remove_file(&request_file) (remove_result) before checking the gh command
result (output.status.success()), which can report failure even when the PR
update succeeded; change the control flow so you first inspect
output.status.success() to determine overall success, then attempt
fs::remove_file(&request_file) as a best-effort cleanup — if the command
succeeded but remove_file returns Err, do not convert that into an overall Err
(instead log or ignore the cleanup error), and only propagate errors from the gh
command (using stderr and AppError::new) when output.status.success() is false;
update the branches around remove_result, output, and the final
Err(AppError::new(...)) to reflect this ordering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e262ddb-0c0b-4b19-8610-f15892f0cf3e
📒 Files selected for processing (11)
README.mdchangelog.d/20260530_104500_plan_to_git_mvp.mdexamples/basic_usage.rssrc/capture.rssrc/codex_history.rssrc/github.rssrc/lib.rssrc/main.rssrc/store.rstests/integration/cli.rstests/unit/plan_capture.rs
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/lib.rs
- changelog.d/20260530_104500_plan_to_git_mvp.md
- src/store.rs
- tests/unit/plan_capture.rs
AI Session BackupCommit: 4ef7872
|
AI Session BackupCommit: 283dcb7
|
AI Session BackupCommit: 979a739
|
AI Session BackupCommit: 002a9f9
|
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-05-30T10:58:24.691Z Codex Plan-to-Git MVPSummaryBuild the repo into a real Rust CLI named MVP source is Codex first. Current branch Key Changes
Test Plan
Assumptions And References
2. PlanSource: codex - Captured: 2026-05-31T13:45:16.373Z Тестовый План Для
|
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-05-31T19:55:23Z Demo PR Comment Plan
|
AI Session BackupCommit: ab2af39
|
AI Session BackupCommit: 7cb2131
|
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)
91-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly answer pending questions from the active branch/session.
drain_relevant_questions()empties the entire repo-wide queue, so a prompt on branch B can consume and clear questions that were captured on branch A. Filter by the current branch/session before draining, otherwise decisions get attached to unrelated questions.🤖 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 91 - 92, drain_relevant_questions currently empties the global state.pending_questions which lets questions from other branches/sessions be consumed; update the logic so only questions belonging to the active branch/session are drained and answered. Specifically, before calling drain_relevant_questions or within it, filter state.pending_questions by the current branch/session id (the same identifier used when capturing questions) and remove only those matching entries (e.g., collect matching questions into a temp Vec and retain the rest in state.pending_questions), then pass that filtered set to state.answer_pending_questions/NewDecision so decisions are attached only to questions from the active branch/session.src/store.rs (1)
250-255:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope pending-question dedupe by branch/session.
This hash only uses the question text, so the same question asked on another branch collapses into the existing entry and never gets recorded for that branch. That breaks the later Q/A pairing flow because the second branch no longer has its own pending question to answer.
🤖 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/store.rs` around lines 250 - 255, The dedupe currently only uses question_hash = stable_hash(&questions.join("\n")) and then checks pending_questions by comparing item_id("question", &question_hash), which collapses identical questions across branches/sessions; include branch or session scope in the identity used for dedupe by incorporating the branch/session identifier into the hash (e.g. combine branch_id or session_id when computing question_hash) or by updating the pending_questions predicate to also compare the question's branch/session field alongside question.id (referencing question_hash, pending_questions, stable_hash, and item_id to locate the logic).
🤖 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 114-117: The GitHub comment creation in
github::sync_state(&context, &mut state) is non-idempotent because
posted_comments can be modified only after the external API call; to fix, make
the persist-before-external-write: mark/append the items as "posted" in
state.posted_comments (or set a pending flag) and call save_state(&state_path,
&state) before calling github::sync_state, then perform the API call and, on
success, clear any transient flags; alternatively implement an idempotency check
inside github::sync_state that queries GitHub for an existing comment (by PR +
unique token) before creating a new one and only create if absent, and ensure
save_state is retried/checked so posted_comments is durably stored.
In `@src/store.rs`:
- Around line 224-229: The matches_current_branch method currently treats all
items as matching when self.branch is None, which can cause unrelated
branch-scoped PlanStackItem entries to be processed; change
matches_current_branch (in src/store.rs) to only return true when both branches
are Some and equal OR when item.branch is None (unscoped) — i.e., if self.branch
is None, do not match items with Some(item_branch); additionally, update
sync_state to either skip syncing when self.branch is None or ensure it relies
on the tightened matches_current_branch behavior to avoid posting branch-scoped
items to the wrong PR.
In `@tests/unit/plan_capture.rs`:
- Around line 287-300: The test pr_body_replaces_through_last_marker currently
has one START_MARKER and two END_MARKER tokens which doesn't exercise multiple
complete marker sections; update the fixture so there are two full marker blocks
(i.e., two START_MARKER ... END_MARKER pairs) before the “stale” text, then call
upsert_marker_block(&original, &block) as before and assert the old contents
from both blocks are removed and replaced by the new block; locate the test
function pr_body_replaces_through_last_marker and modify the original string to
include two complete marker sections to match the test name and intended
scenario.
---
Outside diff comments:
In `@src/capture.rs`:
- Around line 91-92: drain_relevant_questions currently empties the global
state.pending_questions which lets questions from other branches/sessions be
consumed; update the logic so only questions belonging to the active
branch/session are drained and answered. Specifically, before calling
drain_relevant_questions or within it, filter state.pending_questions by the
current branch/session id (the same identifier used when capturing questions)
and remove only those matching entries (e.g., collect matching questions into a
temp Vec and retain the rest in state.pending_questions), then pass that
filtered set to state.answer_pending_questions/NewDecision so decisions are
attached only to questions from the active branch/session.
In `@src/store.rs`:
- Around line 250-255: The dedupe currently only uses question_hash =
stable_hash(&questions.join("\n")) and then checks pending_questions by
comparing item_id("question", &question_hash), which collapses identical
questions across branches/sessions; include branch or session scope in the
identity used for dedupe by incorporating the branch/session identifier into the
hash (e.g. combine branch_id or session_id when computing question_hash) or by
updating the pending_questions predicate to also compare the question's
branch/session field alongside question.id (referencing question_hash,
pending_questions, stable_hash, and item_id to locate the logic).
🪄 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: d29013ce-9686-4bce-9d86-95d89a3bf830
📒 Files selected for processing (12)
README.mdchangelog.d/20260530_104500_plan_to_git_mvp.mdsrc/capture.rssrc/codex_history.rssrc/github.rssrc/main.rssrc/normalize.rssrc/pr_body.rssrc/render.rssrc/store.rstests/integration/cli.rstests/unit/plan_capture.rs
✅ Files skipped from review due to trivial changes (2)
- changelog.d/20260530_104500_plan_to_git_mvp.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main.rs
- src/codex_history.rs
- src/normalize.rs
| let sync_status = github::sync_state(&context, &mut state)?; | ||
| if changed || !state.items.is_empty() || !state.pending_questions.is_empty() { | ||
| save_state(&state_path, &state)?; | ||
| } |
There was a problem hiding this comment.
This comment-post path is still non-idempotent across save failures.
sync_state() can create the GitHub comment before posted_comments is persisted. If the process exits or save_state() fails after the API call, the next run will see the same items as unposted and create a duplicate PR comment. This needs a durable/idempotent guard around the external write.
🤖 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 114 - 117, The GitHub comment creation in
github::sync_state(&context, &mut state) is non-idempotent because
posted_comments can be modified only after the external API call; to fix, make
the persist-before-external-write: mark/append the items as "posted" in
state.posted_comments (or set a pending flag) and call save_state(&state_path,
&state) before calling github::sync_state, then perform the API call and, on
success, clear any transient flags; alternatively implement an idempotency check
inside github::sync_state that queries GitHub for an existing comment (by PR +
unique token) before creating a new one and only create if absent, and ensure
save_state is retried/checked so posted_comments is durably stored.
| fn matches_current_branch(&self, item: &PlanStackItem) -> bool { | ||
| match (item.branch.as_deref(), self.branch.as_deref()) { | ||
| (Some(item_branch), Some(branch)) => item_branch == branch, | ||
| (Some(_), None) | (None, _) => true, | ||
| } | ||
| } |
There was a problem hiding this comment.
Fail closed when the current branch is unknown.
When self.branch is None (detached HEAD, shallow CI checkout, etc.), this treats every stored item as current. sync_state() will then consider unrelated branch items eligible and can post them to the wrong PR. Missing branch context should only match unscoped items, or skip sync entirely.
🤖 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/store.rs` around lines 224 - 229, The matches_current_branch method
currently treats all items as matching when self.branch is None, which can cause
unrelated branch-scoped PlanStackItem entries to be processed; change
matches_current_branch (in src/store.rs) to only return true when both branches
are Some and equal OR when item.branch is None (unscoped) — i.e., if self.branch
is None, do not match items with Some(item_branch); additionally, update
sync_state to either skip syncing when self.branch is None or ensure it relies
on the tightened matches_current_branch behavior to avoid posting branch-scoped
items to the wrong PR.
| #[test] | ||
| fn pr_body_replaces_through_last_marker() { | ||
| let original = | ||
| format!("Intro\n\n{START_MARKER}\nold\n{END_MARKER}\nstale\n{END_MARKER}\nOutro"); | ||
| let block = format!("{START_MARKER}\nnew\n{END_MARKER}"); | ||
|
|
||
| let replaced = upsert_marker_block(&original, &block).expect("replace should work"); | ||
|
|
||
| assert!(replaced.contains("Intro")); | ||
| assert!(replaced.contains("new")); | ||
| assert!(replaced.contains("Outro")); | ||
| assert!(!replaced.contains("old")); | ||
| assert!(!replaced.contains("stale")); | ||
| } |
There was a problem hiding this comment.
Make this fixture match the scenario named by the test.
This body has one START_MARKER and two END_MARKERs, so it does not actually cover “multiple marker sections.” As written, it can pass while upsert_marker_block still mishandles two complete marker blocks.
Suggested test fixture
fn pr_body_replaces_through_last_marker() {
- let original =
- format!("Intro\n\n{START_MARKER}\nold\n{END_MARKER}\nstale\n{END_MARKER}\nOutro");
+ let original = format!(
+ "Intro\n\n{START_MARKER}\nold-1\n{END_MARKER}\nstale\n{START_MARKER}\nold-2\n{END_MARKER}\nOutro"
+ );
let block = format!("{START_MARKER}\nnew\n{END_MARKER}");
let replaced = upsert_marker_block(&original, &block).expect("replace should work");
assert!(replaced.contains("Intro"));
assert!(replaced.contains("new"));
assert!(replaced.contains("Outro"));
- assert!(!replaced.contains("old"));
+ assert!(!replaced.contains("old-1"));
+ assert!(!replaced.contains("old-2"));
assert!(!replaced.contains("stale"));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn pr_body_replaces_through_last_marker() { | |
| let original = | |
| format!("Intro\n\n{START_MARKER}\nold\n{END_MARKER}\nstale\n{END_MARKER}\nOutro"); | |
| let block = format!("{START_MARKER}\nnew\n{END_MARKER}"); | |
| let replaced = upsert_marker_block(&original, &block).expect("replace should work"); | |
| assert!(replaced.contains("Intro")); | |
| assert!(replaced.contains("new")); | |
| assert!(replaced.contains("Outro")); | |
| assert!(!replaced.contains("old")); | |
| assert!(!replaced.contains("stale")); | |
| } | |
| #[test] | |
| fn pr_body_replaces_through_last_marker() { | |
| let original = format!( | |
| "Intro\n\n{START_MARKER}\nold-1\n{END_MARKER}\nstale\n{START_MARKER}\nold-2\n{END_MARKER}\nOutro" | |
| ); | |
| let block = format!("{START_MARKER}\nnew\n{END_MARKER}"); | |
| let replaced = upsert_marker_block(&original, &block).expect("replace should work"); | |
| assert!(replaced.contains("Intro")); | |
| assert!(replaced.contains("new")); | |
| assert!(replaced.contains("Outro")); | |
| assert!(!replaced.contains("old-1")); | |
| assert!(!replaced.contains("old-2")); | |
| assert!(!replaced.contains("stale")); | |
| } |
🤖 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 `@tests/unit/plan_capture.rs` around lines 287 - 300, The test
pr_body_replaces_through_last_marker currently has one START_MARKER and two
END_MARKER tokens which doesn't exercise multiple complete marker sections;
update the fixture so there are two full marker blocks (i.e., two START_MARKER
... END_MARKER pairs) before the “stale” text, then call
upsert_marker_block(&original, &block) as before and assert the old contents
from both blocks are removed and replaced by the new block; locate the test
function pr_body_replaces_through_last_marker and modify the original string to
include two complete marker sections to match the test name and intended
scenario.
AI Session BackupCommit: 85b5b9e
|
AI Session BackupCommit: 0314192
|
AI Session BackupCommit: 7dce5c5
|
1 participant
Summary
Verification
Agent Plan Stack
Branch:
issue-2at979a739.1. Plan
Source: codex - Captured: 2026-05-30T10:58:24.691Z
Codex Plan-to-Git MVP
Summary
Build the repo into a real Rust CLI named
plan-to-gitthat automatically captures explicit Codex plan blocks and planning Q/A decisions, stores them as a local stack in.agent-plan.json, and syncs the full stack into the current branch PR when one exists.MVP source is Codex first. Current branch
issue-2has no PR, so the expected first behavior is local stack persistence; when a PR later exists, the next hook/sync run uploads all queued plans.Key Changes
Replace the template
sumapp withplan-to-git:plan-to-git/plan_to_git.sumcode, update README/examples/tests.serde,serde_json,chrono,sha2; addtempfileas a dev dependency.minorchangelog fragment.Public CLI:
plan-to-git hook --source codex: reads Codex hook JSON from stdin, handlesStopandUserPromptSubmit.plan-to-git sync: manually re-runs GitHub PR sync for queued stack items.plan-to-git show: prints the local stack.plan-to-git render: prints the markdown block that would be inserted into the PR.plan-to-git clear: clears local stack only with--yes.Local state:
.agent-plan.json; add it to.gitignoreas local sync state.1, withrepo,branch,head_sha,items, andpending_questions.plananddecision.Capture behavior:
Stop, inspect onlylast_assistant_message; do not parse raw~/.codextranscripts.<proposed_plan>...</proposed_plan>and explicitAccepted Planblocks.pending_questions.UserPromptSubmit, attach the user prompt as adecisionitem answering pending questions.GitHub PR sync:
gitto resolve repo root, branch, HEAD SHA, and remote; usegh pr view/gh pr edit --body-file.<!-- plan-to-git:start --> ... <!-- plan-to-git:end -->Test Plan
Unit tests:
<proposed_plan>andAccepted Plan;.agent-plan.jsonround trip and content-hash dedupe;Integration tests:
hook --source codexwith fixtureStopJSON writes a plan item.hook --source codexwithStopquestion thenUserPromptSubmitwrites a decision item.syncwith fakeghreporting no PR preserves queued items and exits success.syncwith fakeghPR body updates only the marker block.git/ghvia tempPATH.Verification commands after Rust toolchain is available:
cargo fmt --checkcargo clippy --all-targets --all-featurescargo testAssumptions And References
acceptgate in MVP.sourceextensible.codex/gh, butcargo/rustcare unavailable, so implementation verification needs a Rust toolchain first.2. Plan
Source: codex - Captured: 2026-05-31T13:45:16.373Z
Тестовый План Для
plan-to-gitSummary
Проверить PR #3: #3
Цель тестирования: подтвердить, что CLI корректно захватывает планы из Codex hooks, сохраняет локальный stack, синхронизирует plan/Q&A в PR body, не пишет в stdout hook-команды и не смешивает планы разных веток.
Automated Checks
cargo fmt --all -- --check cargo test cargo clippy --all-targets --all-features cargo package --allow-dirtyFunctional Scenarios
Stophook с<proposed_plan>:plan-to-git hook --source codex;hook_event_name: "Stop"иlast_assistant_message;.agent-plan.jsonсодержит plan item, stdout пустой.Stophook с вопросом агента:last_assistant_messageвидаShould ...?;pending_questions, stdout пустой.UserPromptSubmithook с ответом пользователя:hook_event_name: "UserPromptSubmit"иprompt;Planning Decision,pending_questionsочищается.PR sync при открытом PR:
gh pr viewнаходит PR текущей ветки;plan-to-gitобновляет PR body черезgh api PATCH;<!-- plan-to-git:start --> ... <!-- plan-to-git:end -->.PR sync без открытого PR:
gh pr viewвозвращаетno pull requests found;Regression And Safety Cases
Real PR Acceptance Criteria
Real PR Dogfood Plan;Should agent questions...;upload both...;plan-to-git:start/end.gh pr view 3 --json body --jq '.body'.playwright-mcp/не относится к тесту и не добавляется в commit.Assumptions
git/gh, чтобы не мутировать реальные PR.3. Plan
Source: codex - Captured: 2026-05-31T13:55:36.095Z
Hook Config Live Test
4. Plan
Source: codex - Captured: 2026-05-31T13:55:54.180Z
Hook JSON Test
5. Plan
Source: codex - Captured: 2026-05-31T13:56:48.365Z
Hooks JSON Live Test