Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ The MVP is Codex-first:
- reads Codex hook JSON from stdin;
- captures only explicit plan blocks such as `<proposed_plan>...</proposed_plan>`, `<proposed_plan title="...">...</proposed_plan>`, or `## Accepted Plan`;
- stores captured plans and planning Q/A decisions in `.agent-plan.json`;
- posts a new PR comment with newly captured current-branch items when an open PR exists;
- leaves the local stack queued when no open PR exists yet.
- posts a new PR comment with newly captured current-branch items when a valid (open, non-draft) PR exists;
- leaves the local stack queued when no valid PR exists yet.

## CLI

Expand Down Expand Up @@ -46,15 +46,15 @@ If an agent emits known XML-style plan sections (`summary`, `flow`, `test_plan`,

## Pull Request Comments

When `gh pr view` finds an open 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, 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:

```markdown
## Agent Plan Update

...
```

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, `.agent-plan.json` records the posted item hashes and GitHub comment id so repeated `sync`, `hook`, or `import-codex` runs do not post the same plan again, including on a later PR.
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, `.agent-plan.json` records the posted item hashes and GitHub comment id so repeated `sync`, `hook`, or `import-codex` runs do not post the same plan again, including on a later PR.

## Safety

Expand Down
6 changes: 6 additions & 0 deletions changelog.d/20260610_000000_guard_plan_sync_draft_prs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
---

### Fixed
- Kept captured plan updates queued instead of posting comments to draft pull requests, so plans stay on the upload stack until the PR is valid (open and ready for review).
12 changes: 11 additions & 1 deletion src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ pub enum SyncStatus {
number: u64,
state: String,
},
DraftPullRequest {
number: u64,
},
Unchanged {
number: u64,
},
Expand All @@ -32,6 +35,8 @@ pub enum SyncStatus {
struct PullRequest {
number: u64,
state: String,
#[serde(default, rename = "isDraft")]
is_draft: bool,
}

#[derive(Debug, Deserialize)]
Expand All @@ -53,6 +58,11 @@ pub fn sync_state(context: &GitContext, state: &mut AgentPlanState) -> AppResult
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);
Expand All @@ -78,7 +88,7 @@ pub fn sync_state(context: &GitContext, state: &mut AgentPlanState) -> AppResult
fn view_current_pr(repo_root: &Path) -> AppResult<Option<PullRequest>> {
let output = Command::new("gh")
.current_dir(repo_root)
.args(["pr", "view", "--json", "number,state,url"])
.args(["pr", "view", "--json", "number,state,url,isDraft"])
.output()?;

if output.status.success() {
Expand Down
3 changes: 3 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ 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}");
}
Expand Down
98 changes: 95 additions & 3 deletions tests/integration/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,77 @@ mod unix {
assert!(!captured_request.exists());
}

#[test]
fn hook_leaves_plans_queued_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");
let captured_request = temp_dir.path().join("request.json");
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);

run_hook(
&repo_dir,
&bin_dir,
&format!(
r#"{{
"session_id":"session",
"cwd":"{}",
"hook_event_name":"Stop",
"turn_id":"turn",
"last_assistant_message":"<proposed_plan>\n# Queued\n\n- Wait for a valid PR\n</proposed_plan>"
}}"#,
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());
}

#[test]
fn sync_reports_draft_pr_and_does_not_comment() {
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 captured_request = temp_dir.path().join("request.json");
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);

run_hook(
&repo_dir,
&bin_dir,
&format!(
r#"{{
"session_id":"session",
"cwd":"{}",
"hook_event_name":"Stop",
"turn_id":"turn",
"last_assistant_message":"<proposed_plan>\n# Queued\n\n- Wait for a valid PR\n</proposed_plan>"
}}"#,
repo_dir.display()
),
);

let output = Command::new(env!("CARGO_BIN_EXE_plan-to-git"))
.arg("sync")
.current_dir(&repo_dir)
.env("PATH", path_with_fake_bin(&bin_dir))
.output()
.expect("run sync");

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());
}

#[test]
fn sync_reports_merged_pr_and_does_not_comment() {
let temp_dir = tempfile::tempdir().expect("temp dir");
Expand Down Expand Up @@ -441,7 +512,7 @@ esac
fn write_fake_gh_no_pr(bin_dir: &Path) {
let script = r#"#!/usr/bin/env bash
set -euo pipefail
if [[ "$*" == "pr view --json number,state,url" ]]; then
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
echo 'no pull requests found for branch "feature/test"' >&2
exit 1
fi
Expand All @@ -455,7 +526,7 @@ exit 1
let script = format!(
r#"#!/usr/bin/env bash
set -euo pipefail
if [[ "$*" == "pr view --json number,state,url" ]]; then
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
printf '%s\n' '{{"number":17,"state":"OPEN","url":"https://github.com/example/repo/pull/17"}}'
exit 0
fi
Expand All @@ -476,7 +547,7 @@ exit 1
let script = format!(
r#"#!/usr/bin/env bash
set -euo pipefail
if [[ "$*" == "pr view --json number,state,url" ]]; then
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
printf '%s\n' '{{"number":17,"state":"{state}","url":"https://github.com/example/repo/pull/17"}}'
exit 0
fi
Expand All @@ -493,6 +564,27 @@ exit 1
write_executable(&bin_dir.join("gh"), &script);
}

fn write_fake_gh_draft_pr(bin_dir: &Path, captured_request: &Path) {
let script = format!(
r#"#!/usr/bin/env bash
set -euo pipefail
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
fi
echo "unexpected gh args: $*" >&2
exit 1
"#,
captured_request.display()
);
write_executable(&bin_dir.join("gh"), &script);
}

fn write_executable(path: &Path, content: &str) {
fs::write(path, content).expect("write script");
let mut permissions = fs::metadata(path).expect("metadata").permissions();
Expand Down
Loading