Skip to content

Commit a00a830

Browse files
authored
Merge pull request #7 from konard/issue-6-9b8d4b256f20
fix: guard plan sync for draft pull requests
2 parents 06fe8bd + 2e29caf commit a00a830

5 files changed

Lines changed: 119 additions & 8 deletions

File tree

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ The MVP is Codex-first:
77
- reads Codex hook JSON from stdin;
88
- captures only explicit plan blocks such as `<proposed_plan>...</proposed_plan>`, `<proposed_plan title="...">...</proposed_plan>`, or `## Accepted Plan`;
99
- stores captured plans and planning Q/A decisions in `.agent-plan.json`;
10-
- posts a new PR comment with newly captured current-branch items when an open PR exists;
11-
- leaves the local stack queued when no open PR exists yet.
10+
- posts a new PR comment with newly captured current-branch items when a valid (open, non-draft) PR exists;
11+
- leaves the local stack queued when no valid PR exists yet.
1212

1313
## CLI
1414

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

4747
## Pull Request Comments
4848

49-
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:
49+
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:
5050

5151
```markdown
5252
## Agent Plan Update
5353

5454
...
5555
```
5656

57-
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.
57+
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.
5858

5959
## Safety
6060

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
bump: patch
3+
---
4+
5+
### Fixed
6+
- 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).

src/github.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ pub enum SyncStatus {
1818
number: u64,
1919
state: String,
2020
},
21+
DraftPullRequest {
22+
number: u64,
23+
},
2124
Unchanged {
2225
number: u64,
2326
},
@@ -32,6 +35,8 @@ pub enum SyncStatus {
3235
struct PullRequest {
3336
number: u64,
3437
state: String,
38+
#[serde(default, rename = "isDraft")]
39+
is_draft: bool,
3540
}
3641

3742
#[derive(Debug, Deserialize)]
@@ -53,6 +58,11 @@ pub fn sync_state(context: &GitContext, state: &mut AgentPlanState) -> AppResult
5358
state: pull_request.state,
5459
});
5560
}
61+
if pull_request.is_draft {
62+
return Ok(SyncStatus::DraftPullRequest {
63+
number: pull_request.number,
64+
});
65+
}
5666

5767
let (comment_body, item_ids, item_count) = {
5868
let items = state.unposted_items_for_pr(pull_request.number);
@@ -78,7 +88,7 @@ pub fn sync_state(context: &GitContext, state: &mut AgentPlanState) -> AppResult
7888
fn view_current_pr(repo_root: &Path) -> AppResult<Option<PullRequest>> {
7989
let output = Command::new("gh")
8090
.current_dir(repo_root)
81-
.args(["pr", "view", "--json", "number,state,url"])
91+
.args(["pr", "view", "--json", "number,state,url,isDraft"])
8292
.output()?;
8393

8494
if output.status.success() {

src/main.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ fn print_sync_status(status: &SyncStatus) {
190190
SyncStatus::ClosedPullRequest { number, state } => {
191191
println!("pull request #{number} is {state}; leaving plan items queued");
192192
}
193+
SyncStatus::DraftPullRequest { number } => {
194+
println!("pull request #{number} is a draft; leaving plan items queued");
195+
}
193196
SyncStatus::Unchanged { number } => {
194197
println!("no new plan items to comment on pull request #{number}");
195198
}

tests/integration/cli.rs

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,77 @@ mod unix {
296296
assert!(!captured_request.exists());
297297
}
298298

299+
#[test]
300+
fn hook_leaves_plans_queued_when_pr_is_draft() {
301+
let temp_dir = tempfile::tempdir().expect("temp dir");
302+
let bin_dir = temp_dir.path().join("bin");
303+
let repo_dir = temp_dir.path().join("repo");
304+
let captured_request = temp_dir.path().join("request.json");
305+
fs::create_dir_all(&bin_dir).expect("bin dir");
306+
fs::create_dir_all(&repo_dir).expect("repo dir");
307+
write_fake_git(&bin_dir, &repo_dir);
308+
write_fake_gh_draft_pr(&bin_dir, &captured_request);
309+
310+
run_hook(
311+
&repo_dir,
312+
&bin_dir,
313+
&format!(
314+
r#"{{
315+
"session_id":"session",
316+
"cwd":"{}",
317+
"hook_event_name":"Stop",
318+
"turn_id":"turn",
319+
"last_assistant_message":"<proposed_plan>\n# Queued\n\n- Wait for a valid PR\n</proposed_plan>"
320+
}}"#,
321+
repo_dir.display()
322+
),
323+
);
324+
325+
let state = fs::read_to_string(repo_dir.join(STATE_FILE_NAME)).expect("state file");
326+
assert!(state.contains("Wait for a valid PR"));
327+
assert!(state.contains("\"posted_comments\": []"));
328+
assert!(!captured_request.exists());
329+
}
330+
331+
#[test]
332+
fn sync_reports_draft_pr_and_does_not_comment() {
333+
let temp_dir = tempfile::tempdir().expect("temp dir");
334+
let bin_dir = temp_dir.path().join("bin");
335+
let repo_dir = temp_dir.path().join("repo");
336+
let captured_request = temp_dir.path().join("request.json");
337+
fs::create_dir_all(&bin_dir).expect("bin dir");
338+
fs::create_dir_all(&repo_dir).expect("repo dir");
339+
write_fake_git(&bin_dir, &repo_dir);
340+
write_fake_gh_draft_pr(&bin_dir, &captured_request);
341+
342+
run_hook(
343+
&repo_dir,
344+
&bin_dir,
345+
&format!(
346+
r#"{{
347+
"session_id":"session",
348+
"cwd":"{}",
349+
"hook_event_name":"Stop",
350+
"turn_id":"turn",
351+
"last_assistant_message":"<proposed_plan>\n# Queued\n\n- Wait for a valid PR\n</proposed_plan>"
352+
}}"#,
353+
repo_dir.display()
354+
),
355+
);
356+
357+
let output = Command::new(env!("CARGO_BIN_EXE_plan-to-git"))
358+
.arg("sync")
359+
.current_dir(&repo_dir)
360+
.env("PATH", path_with_fake_bin(&bin_dir))
361+
.output()
362+
.expect("run sync");
363+
364+
assert!(output.status.success());
365+
let stdout = String::from_utf8(output.stdout).expect("stdout");
366+
assert!(stdout.contains("pull request #17 is a draft; leaving plan items queued"));
367+
assert!(!captured_request.exists());
368+
}
369+
299370
#[test]
300371
fn sync_reports_merged_pr_and_does_not_comment() {
301372
let temp_dir = tempfile::tempdir().expect("temp dir");
@@ -441,7 +512,7 @@ esac
441512
fn write_fake_gh_no_pr(bin_dir: &Path) {
442513
let script = r#"#!/usr/bin/env bash
443514
set -euo pipefail
444-
if [[ "$*" == "pr view --json number,state,url" ]]; then
515+
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
445516
echo 'no pull requests found for branch "feature/test"' >&2
446517
exit 1
447518
fi
@@ -455,7 +526,7 @@ exit 1
455526
let script = format!(
456527
r#"#!/usr/bin/env bash
457528
set -euo pipefail
458-
if [[ "$*" == "pr view --json number,state,url" ]]; then
529+
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
459530
printf '%s\n' '{{"number":17,"state":"OPEN","url":"https://github.com/example/repo/pull/17"}}'
460531
exit 0
461532
fi
@@ -476,7 +547,7 @@ exit 1
476547
let script = format!(
477548
r#"#!/usr/bin/env bash
478549
set -euo pipefail
479-
if [[ "$*" == "pr view --json number,state,url" ]]; then
550+
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
480551
printf '%s\n' '{{"number":17,"state":"{state}","url":"https://github.com/example/repo/pull/17"}}'
481552
exit 0
482553
fi
@@ -493,6 +564,27 @@ exit 1
493564
write_executable(&bin_dir.join("gh"), &script);
494565
}
495566

567+
fn write_fake_gh_draft_pr(bin_dir: &Path, captured_request: &Path) {
568+
let script = format!(
569+
r#"#!/usr/bin/env bash
570+
set -euo pipefail
571+
if [[ "$*" == "pr view --json number,state,url,isDraft" ]]; then
572+
printf '%s\n' '{{"number":17,"state":"OPEN","url":"https://github.com/example/repo/pull/17","isDraft":true}}'
573+
exit 0
574+
fi
575+
if [[ "$1" == "api" ]]; then
576+
printf '%s\n' "$*" > "{}"
577+
echo "comment API should not be called for draft PR" >&2
578+
exit 1
579+
fi
580+
echo "unexpected gh args: $*" >&2
581+
exit 1
582+
"#,
583+
captured_request.display()
584+
);
585+
write_executable(&bin_dir.join("gh"), &script);
586+
}
587+
496588
fn write_executable(path: &Path, content: &str) {
497589
fs::write(path, content).expect("write script");
498590
let mut permissions = fs::metadata(path).expect("metadata").permissions();

0 commit comments

Comments
 (0)