diff --git a/README.md b/README.md index aff1789..65b859b 100644 --- a/README.md +++ b/README.md @@ -5,9 +5,9 @@ The MVP supports Codex and Claude Code: - reads Codex or Claude Code hook JSON from stdin; -- captures only explicit plan blocks such as `...`, `...`, or `## Accepted Plan`; +- captures explicit plan blocks such as `...`, `...`, or `## Accepted Plan`, plus structured Codex planner updates from `import-codex`; - stores captured plans and planning Q/A decisions in a per-repository state file; -- posts a new PR comment with newly captured current-branch items when a valid (open, non-draft) PR exists; +- posts a new PR comment with newly captured current-branch items when an open PR exists, including draft PRs; - leaves the local stack queued when no valid PR exists yet. ## CLI @@ -94,7 +94,7 @@ If an agent emits known XML-style plan sections (`summary`, `flow`, `test_plan`, ## Pull Request Comments -When `gh pr view` finds an open, non-draft PR for the current branch, `plan-to-git` creates a new issue comment on that PR containing items that have not been posted before: +When `gh pr view` finds an open PR for the current branch, including a draft PR, `plan-to-git` creates a new issue comment on that PR containing items that have not been posted before: ```markdown ## Agent Plan Update @@ -106,10 +106,10 @@ Use `plan-to-git sync --pr 7` to post queued current-branch items to a specific Use `--repo owner/repo` or `PLAN_TO_GIT_REPO=owner/repo` when the local `origin` remote is not the pull request target repository, for example in fork-origin workflows. The explicit repository only selects the GitHub PR/comment target; local state and history matching remain tied to the current checkout. -The PR description is not edited. Closed, merged, or still-draft pull requests are not commented on; new items stay queued until the PR is valid (open and marked ready for review). After a comment is created, the local state file records the posted item hashes and GitHub comment id so repeated `sync`, `hook`, `import-codex`, or `import-claude` runs do not post the same plan again, including on a later PR. +The PR description is not edited. Closed or merged pull requests are not commented on; new items stay queued until an open PR exists. After a comment is created, the local state file records the posted item hashes and GitHub comment id so repeated `sync`, `hook`, `import-codex`, or `import-claude` runs do not post the same plan again, including on a later PR. ## Safety -The hook path only uses stable hook payload fields, explicitly marked plan text, and Claude Plan Mode transcript artifacts. `import-codex` can backfill previous plans from `~/.codex/sessions`; `import-claude` can backfill from Claude Code transcript files under the active Claude config directory. Both importers only read sessions that match the current repository and branch when branch metadata is available, and they still import only explicit markers such as `...`, `...`, `## Accepted Plan`, or Claude Code's native Plan Mode output. +The hook path only uses stable hook payload fields, explicitly marked plan text, and Claude Plan Mode transcript artifacts. `import-codex` can backfill previous plans from `~/.codex/sessions`, including structured Codex `update_plan` planner calls; `import-claude` can backfill from Claude Code transcript files under the active Claude config directory. Both importers only read sessions that match the current repository and branch when branch metadata is available, and they still import only explicit markers such as `...`, `...`, `## Accepted Plan`, Codex planner calls, or Claude Code's native Plan Mode output. Captured content is redacted before local storage and PR sync. The local state file also acts as the sent-plan registry: content hashes prevent the same plan from being added and commented again. diff --git a/changelog.d/20260617_codex_update_plan_import.md b/changelog.d/20260617_codex_update_plan_import.md new file mode 100644 index 0000000..18fd1db --- /dev/null +++ b/changelog.d/20260617_codex_update_plan_import.md @@ -0,0 +1 @@ +- Fixed Codex history import so structured `update_plan` planner calls from `~/.codex/sessions` are captured and queued for PR sync instead of reporting matched session files with zero plans. diff --git a/changelog.d/20260617_sync_draft_prs.md b/changelog.d/20260617_sync_draft_prs.md new file mode 100644 index 0000000..36699e6 --- /dev/null +++ b/changelog.d/20260617_sync_draft_prs.md @@ -0,0 +1 @@ +- Changed plan sync to post comments to open draft pull requests instead of leaving captured plan items queued until the PR is marked ready for review. diff --git a/src/codex_history.rs b/src/codex_history.rs index d67ff8a..9b79d9d 100644 --- a/src/codex_history.rs +++ b/src/codex_history.rs @@ -9,7 +9,7 @@ use crate::history::{ collect_jsonl_files, line_turn_id, looks_like_rendered_plan_stack, session_id_from_path, HistoryImportOutcome, }; -use crate::normalize::extract_marked_plans; +use crate::normalize::{extract_marked_plans, CapturedPlan}; use crate::store::{AgentPlanState, AgentSource, NewPlanItem}; #[derive(Debug, Clone, PartialEq, Eq)] @@ -71,9 +71,10 @@ fn import_session_file( continue; } - let Some(message) = plan_message_text(&event) else { + let plans = event_plans(&event); + if plans.is_empty() { continue; - }; + } let session_id = metadata .as_ref() @@ -90,7 +91,7 @@ fn import_session_file( .and_then(Value::as_str) .map(ToOwned::to_owned); - for plan in extract_marked_plans(&message) { + for plan in plans { outcome.plans_found += 1; if looks_like_rendered_plan_stack(&plan.content) { outcome.rendered_stacks_skipped += 1; @@ -178,6 +179,84 @@ fn plan_message_text(event: &Value) -> Option { assistant_message_text(event).or_else(|| task_complete_message_text(event)) } +fn event_plans(event: &Value) -> Vec { + if let Some(message) = plan_message_text(event) { + return extract_marked_plans(&message); + } + + codex_update_plan(event).into_iter().collect() +} + +fn codex_update_plan(event: &Value) -> Option { + if event.get("type").and_then(Value::as_str) != Some("response_item") { + return None; + } + + let payload = event.get("payload")?; + if payload.get("type").and_then(Value::as_str) != Some("function_call") { + return None; + } + if payload.get("name").and_then(Value::as_str) != Some("update_plan") { + return None; + } + + let arguments = payload.get("arguments").and_then(Value::as_str)?; + let arguments = serde_json::from_str::(arguments).ok()?; + captured_plan_from_update_plan_arguments(&arguments) +} + +fn captured_plan_from_update_plan_arguments(arguments: &Value) -> Option { + let steps = arguments + .get("plan")? + .as_array()? + .iter() + .filter_map(update_plan_step) + .collect::>(); + if steps.is_empty() { + return None; + } + + let mut content = String::from("# Codex Plan\n"); + if let Some(explanation) = arguments + .get("explanation") + .and_then(Value::as_str) + .map(str::trim) + .filter(|explanation| !explanation.is_empty()) + { + content.push('\n'); + content.push_str(explanation); + content.push('\n'); + } + + content.push_str("\n## Steps\n\n"); + for (status, step) in steps { + content.push_str("- "); + content.push_str(status); + content.push_str(": "); + content.push_str(step); + content.push('\n'); + } + + Some(CapturedPlan { + title: Some(String::from("Codex Plan")), + content: content.trim_end().to_owned(), + }) +} + +fn update_plan_step(item: &Value) -> Option<(&str, &str)> { + let step = item.get("step").and_then(Value::as_str)?.trim(); + if step.is_empty() { + return None; + } + let status = item + .get("status") + .and_then(Value::as_str) + .map(str::trim) + .filter(|status| !status.is_empty()) + .unwrap_or("pending"); + Some((status, step)) +} + fn assistant_message_text(event: &Value) -> Option { if event.get("type").and_then(Value::as_str) != Some("response_item") { return None; @@ -275,6 +354,35 @@ mod tests { })) } + fn update_plan_line(timestamp: &str) -> String { + json_line(&json!({ + "timestamp": timestamp, + "type": "response_item", + "payload": { + "type": "function_call", + "name": "update_plan", + "arguments": serde_json::to_string(&json!({ + "explanation": "Structured Codex planning event.", + "plan": [ + { + "step": "Inspect failing import output", + "status": "completed" + }, + { + "step": "Import structured plan calls", + "status": "in_progress" + }, + { + "step": "Run regression tests", + "status": "pending" + } + ] + })).expect("serialize update_plan arguments"), + "call_id": "call-plan" + } + })) + } + fn write_jsonl(path: &Path, lines: &[String]) { fs::write(path, format!("{}\n", lines.join("\n"))).expect("write session"); } @@ -423,6 +531,54 @@ mod tests { assert_eq!(state.items[0].created_at, "2026-05-31T12:34:56Z"); } + #[test] + fn imports_structured_update_plan_function_calls() { + let temp_dir = tempdir().expect("temp dir"); + let repo_root = temp_dir.path().join("repo"); + let codex_home = temp_dir.path().join("codex"); + let session_dir = codex_home.join("sessions/2026/06/17"); + fs::create_dir_all(&repo_root).expect("repo root"); + fs::create_dir_all(&session_dir).expect("session dir"); + + write_jsonl( + &session_dir.join("rollout-2026-06-17T12-00-00-plan.jsonl"), + &[ + session_meta_line(&repo_root, "feature/test"), + update_plan_line("2026-06-17T12:34:56Z"), + ], + ); + + let context = GitContext { + repo_root, + repo_slug: Some("example/repo".to_owned()), + branch: Some("feature/test".to_owned()), + head_sha: Some("abcdef".to_owned()), + }; + let mut state = AgentPlanState::default(); + + let outcome = import_codex_history(&codex_home, &context, &mut state).expect("import"); + + assert_eq!(outcome.files_scanned, 1); + assert_eq!(outcome.files_matched, 1); + assert_eq!(outcome.plans_found, 1); + assert_eq!(outcome.plans_added, 1); + assert_eq!(state.items.len(), 1); + assert_eq!(state.items[0].title.as_deref(), Some("Codex Plan")); + assert!(state.items[0] + .content + .contains("Structured Codex planning event.")); + assert!(state.items[0] + .content + .contains("- completed: Inspect failing import output")); + assert!(state.items[0] + .content + .contains("- in_progress: Import structured plan calls")); + assert!(state.items[0] + .content + .contains("- pending: Run regression tests")); + assert_eq!(state.items[0].created_at, "2026-06-17T12:34:56Z"); + } + #[test] fn skips_sessions_without_positive_repo_or_cwd_match() { let temp_dir = tempdir().expect("temp dir"); diff --git a/src/github.rs b/src/github.rs index 183bada..fbc616b 100644 --- a/src/github.rs +++ b/src/github.rs @@ -18,9 +18,6 @@ pub enum SyncStatus { number: u64, state: String, }, - DraftPullRequest { - number: u64, - }, Unchanged { number: u64, }, @@ -35,8 +32,6 @@ pub enum SyncStatus { struct PullRequest { number: u64, state: String, - #[serde(default, rename = "isDraft")] - is_draft: bool, } #[derive(Debug, Deserialize)] @@ -86,12 +81,6 @@ fn sync_to_pull_request( state: pull_request.state, }); } - if pull_request.is_draft { - return Ok(SyncStatus::DraftPullRequest { - number: pull_request.number, - }); - } - let (comment_body, item_ids, item_count) = { let items = state.unposted_items_for_pr(pull_request.number); if items.is_empty() { diff --git a/src/main.rs b/src/main.rs index 6b37ca8..f55a3f0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -276,9 +276,6 @@ fn print_sync_status(status: &SyncStatus) { SyncStatus::ClosedPullRequest { number, state } => { println!("pull request #{number} is {state}; leaving plan items queued"); } - SyncStatus::DraftPullRequest { number } => { - println!("pull request #{number} is a draft; leaving plan items queued"); - } SyncStatus::Unchanged { number } => { println!("no new plan items to comment on pull request #{number}"); } diff --git a/tests/integration/cli.rs b/tests/integration/cli.rs index b8faa36..da1f296 100644 --- a/tests/integration/cli.rs +++ b/tests/integration/cli.rs @@ -656,7 +656,7 @@ mod unix { } #[test] - fn hook_leaves_plans_queued_when_pr_is_draft() { + fn hook_posts_comment_when_pr_is_draft() { let temp_dir = tempfile::tempdir().expect("temp dir"); let bin_dir = temp_dir.path().join("bin"); let repo_dir = temp_dir.path().join("repo"); @@ -675,20 +675,22 @@ mod unix { "cwd":"{}", "hook_event_name":"Stop", "turn_id":"turn", - "last_assistant_message":"\n# Queued\n\n- Wait for a valid PR\n" + "last_assistant_message":"\n# Draft\n\n- Post to a draft PR\n" }}"#, repo_dir.display() ), ); let state = fs::read_to_string(repo_dir.join(STATE_FILE_NAME)).expect("state file"); - assert!(state.contains("Wait for a valid PR")); - assert!(state.contains("\"posted_comments\": []")); - assert!(!captured_request.exists()); + assert!(state.contains("Post to a draft PR")); + assert!(state.contains("\"comment_id\": 12345")); + let request = fs::read_to_string(captured_request).expect("captured request"); + assert!(request.contains("Agent Plan Update")); + assert!(request.contains("Post to a draft PR")); } #[test] - fn sync_reports_draft_pr_and_does_not_comment() { + fn sync_posts_comment_when_pr_is_draft() { let temp_dir = tempfile::tempdir().expect("temp dir"); let bin_dir = temp_dir.path().join("bin"); let repo_dir = temp_dir.path().join("repo"); @@ -696,7 +698,7 @@ mod unix { fs::create_dir_all(&bin_dir).expect("bin dir"); fs::create_dir_all(&repo_dir).expect("repo dir"); write_fake_git(&bin_dir, &repo_dir); - write_fake_gh_draft_pr(&bin_dir, &captured_request); + write_fake_gh_no_pr(&bin_dir); run_hook( &repo_dir, @@ -713,6 +715,9 @@ mod unix { ), ); + assert!(!captured_request.exists()); + write_fake_gh_draft_pr(&bin_dir, &captured_request); + let output = Command::new(env!("CARGO_BIN_EXE_plan-to-git")) .arg("sync") .current_dir(&repo_dir) @@ -723,8 +728,10 @@ mod unix { assert!(output.status.success()); let stdout = String::from_utf8(output.stdout).expect("stdout"); - assert!(stdout.contains("pull request #17 is a draft; leaving plan items queued")); - assert!(!captured_request.exists()); + assert!(stdout.contains("posted 1 plan item(s) to pull request #17 comment #12345")); + let request = fs::read_to_string(captured_request).expect("captured request"); + assert!(request.contains("Agent Plan Update")); + assert!(request.contains("Wait for a valid PR")); } #[test] @@ -800,6 +807,122 @@ mod unix { assert!(second.contains("skipped 1 duplicate(s)")); } + #[test] + fn import_codex_backfills_structured_update_plan_calls() { + let temp_dir = tempfile::tempdir().expect("temp dir"); + let bin_dir = temp_dir.path().join("bin"); + let repo_dir = temp_dir.path().join("repo"); + let codex_home = temp_dir.path().join("codex"); + let session_dir = codex_home.join("sessions/2026/06/17"); + fs::create_dir_all(&bin_dir).expect("bin dir"); + fs::create_dir_all(&repo_dir).expect("repo dir"); + fs::create_dir_all(&session_dir).expect("session dir"); + write_fake_git(&bin_dir, &repo_dir); + + let arguments = serde_json::to_string(&serde_json::json!({ + "explanation": "Structured Codex plan from the planner tool.", + "plan": [ + { + "step": "Inspect matched history files", + "status": "completed" + }, + { + "step": "Import update_plan calls", + "status": "in_progress" + }, + { + "step": "Verify PR sync has queued items", + "status": "pending" + } + ] + })) + .expect("serialize update_plan arguments"); + let arguments_json = serde_json::to_string(&arguments).expect("quote arguments"); + + fs::write( + session_dir.join("rollout-2026-06-17T12-00-00-plan.jsonl"), + format!( + r#"{{"type":"session_meta","payload":{{"id":"session","cwd":"{}","git":{{"branch":"feature/test","repository_url":"https://github.com/example/repo.git"}}}}}} +{{"timestamp":"2026-06-17T12:34:56Z","type":"response_item","payload":{{"type":"function_call","name":"update_plan","arguments":{arguments_json},"call_id":"call-plan"}}}} +"#, + repo_dir.display() + ), + ) + .expect("write session"); + + let first = run_import_codex(&repo_dir, &bin_dir, &codex_home); + assert!(first.contains("found 1 plan(s), added 1")); + let state = fs::read_to_string(repo_dir.join(STATE_FILE_NAME)).expect("state file"); + assert!(state.contains("Codex Plan")); + assert!(state.contains("Structured Codex plan from the planner tool.")); + assert!(state.contains("completed: Inspect matched history files")); + assert!(state.contains("in_progress: Import update_plan calls")); + assert!(state.contains("pending: Verify PR sync has queued items")); + } + + #[test] + fn import_codex_syncs_structured_update_plan_calls_to_open_pr() { + let temp_dir = tempfile::tempdir().expect("temp dir"); + let bin_dir = temp_dir.path().join("bin"); + let repo_dir = temp_dir.path().join("repo"); + let codex_home = temp_dir.path().join("codex"); + let session_dir = codex_home.join("sessions/2026/06/17"); + let captured_request = temp_dir.path().join("comment.json"); + fs::create_dir_all(&bin_dir).expect("bin dir"); + fs::create_dir_all(&repo_dir).expect("repo dir"); + fs::create_dir_all(&session_dir).expect("session dir"); + write_fake_git(&bin_dir, &repo_dir); + write_fake_gh_open_pr(&bin_dir, &captured_request); + + let arguments = serde_json::to_string(&serde_json::json!({ + "explanation": "Structured Codex plan reaches PR sync.", + "plan": [ + { + "step": "Import the planner event", + "status": "completed" + }, + { + "step": "Post it to the pull request", + "status": "in_progress" + } + ] + })) + .expect("serialize update_plan arguments"); + let arguments_json = serde_json::to_string(&arguments).expect("quote arguments"); + + fs::write( + session_dir.join("rollout-2026-06-17T12-00-00-plan.jsonl"), + format!( + r#"{{"type":"session_meta","payload":{{"id":"session","cwd":"{}","git":{{"branch":"feature/test","repository_url":"https://github.com/example/repo.git"}}}}}} +{{"timestamp":"2026-06-17T12:34:56Z","type":"response_item","payload":{{"type":"function_call","name":"update_plan","arguments":{arguments_json},"call_id":"call-plan"}}}} +"#, + repo_dir.display() + ), + ) + .expect("write session"); + + let output = Command::new(env!("CARGO_BIN_EXE_plan-to-git")) + .arg("import-codex") + .arg("--codex-home") + .arg(&codex_home) + .current_dir(&repo_dir) + .env("PATH", path_with_fake_bin(&bin_dir)) + .env("PLAN_TO_GIT_STATE_PATH", state_path(&repo_dir)) + .output() + .expect("run import-codex"); + + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).expect("stdout"); + assert!(stdout.contains("found 1 plan(s), added 1")); + assert!(stdout.contains("posted 1 plan item(s) to pull request #17")); + + let request = fs::read_to_string(captured_request).expect("captured request"); + assert!(request.contains("Agent Plan Update")); + assert!(request.contains("Structured Codex plan reaches PR sync.")); + assert!(request.contains("completed: Import the planner event")); + assert!(request.contains("in_progress: Post it to the pull request")); + } + #[test] fn import_claude_backfills_history_once_from_config_dir_without_syncing() { let temp_dir = tempfile::tempdir().expect("temp dir"); diff --git a/tests/integration/support.rs b/tests/integration/support.rs index 34698d5..2a1b770 100644 --- a/tests/integration/support.rs +++ b/tests/integration/support.rs @@ -252,10 +252,10 @@ if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then printf '%s\n' '{{"number":17,"state":"OPEN","url":"https://github.com/example/repo/pull/17","isDraft":true}}' exit 0 fi -if [[ "$1" == "api" ]]; then - printf '%s\n' "$*" > "{}" - echo "comment API should not be called for draft PR" >&2 - exit 1 +if [[ "$1 $2 $3" == "api --method POST" && "$4" == "repos/example/repo/issues/17/comments" && "$5" == "--input" ]]; then + cp "$6" "{}" + printf '%s\n' '{{"id":12345}}' + exit 0 fi echo "unexpected gh args: $*" >&2 exit 1