diff --git a/AGENTS.md b/AGENTS.md index 41700f4f..90f6c952 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -305,7 +305,7 @@ Three location classes decide what survives: | Path | Class | Holds | Written by | Read by | Removed by | |---|---|---|---|---|---| -| `.orca/runs/.progress.json` | committed | `ProgressLog`: header (branches, `branchMode`, `startingCommit`, `userPrompt`, `flow`), one `StageEntry` per completed stage (`id` as `StagePath` segments, `resultJson`), `published` | `ProgressStore` (`FlowLifecycle.freshRun`, `stage.scala`'s `recordAndCommit`, `recordOpenedPr`) | `stage.scala`'s `resumeFrom`, `RecoveryCheck`, `FlowLifecycle`, shell `ResumeDetector` (header) | success teardown, in a final commit | +| `.orca/runs/.progress.json` | committed | `ProgressLog`: header (branches, `branchMode`, `startingCommit`, `userPrompt`, `flow`, `prReadiness`), one `StageEntry` per completed stage (`id` as `StagePath` segments, `resultJson`), `published` | `ProgressStore` (`FlowLifecycle.freshRun`, `stage.scala`'s `recordAndCommit`, `recordOpenedPr`) | `stage.scala`'s `resumeFrom`, `RecoveryCheck`, `FlowLifecycle`, shell `ResumeDetector` (header) | success teardown, in a final commit | | `.orca/cache/runs//events.jsonl` | cache | the run's append-only `RunEvent` log, one JSON line per event: `AttemptStarted`, `BranchBound`, `StageStarted`/`StageEnded`, `SessionMinted`/`SessionWireId` (session records), `SessionCommitted`, `Turn` (token usage and cost), `RunSucceeded`, `AttemptFinished`; a public format, documented in docs/using/output-and-files.md | `RunEventLog`: the run's `SessionStore` and an `OrcaListener` | `RunEventLog` at attempt start (session records after the last `RunSucceeded`), shell `ContinuableAttempts` → session picker / `orca continue` (attempts with no session are left out), people and scripts | pruning at attempt start, by run directory: never the current run or one with a `SessionMinted` after its last `RunSucceeded`; else newest 20 runs with a session ∪ newest 20 of any kind | | `.orca/cache/runs//.trace.log` (+ `.trace.1.log`) | cache | DEBUG trace of logger `orca`: prompts, agent output, tool calls; 4 MB roll | `OrcaLog` | people (path in the banner and in `AttemptStarted.trace`) | pruned with its run directory; across all run directories, all but the newest 40 attempts' traces at attempt start | | `.orca/cache/flow.lock` | cache | an OS file lock, held for the run; holder pid | `FlowLock` | `FlowLock` on contention | never; the OS releases the lock when the holder exits | diff --git a/docs/api/tools.md b/docs/api/tools.md index 9cb549fa..f20e6f1e 100644 --- a/docs/api/tools.md +++ b/docs/api/tools.md @@ -62,7 +62,7 @@ and `issue` an `IssueHandle`. |---|---| | `availability()` | probes whether a PR can be opened from this checkout | | `prHandle(ref)` | a `PrHandle` from a PR URL or `owner/repo#N`; `Either[String, PrHandle]` | -| `createPr(title, body)` | opens a PR; `Either[PrCreateFailed, PrHandle]` | +| `createPr(title, body, readiness?)` | opens a PR, as a draft with `Some(PrReadiness.Draft)` or, when omitted, under `--draft`; `Either[PrCreateFailed, PrHandle]` | | `updatePr(pr, title, body)` | replaces a PR's title and body | | `readIssue(issue)`, `readIssueComments(issue)`, `readPrComments(pr)` | read an issue, its comments, or a PR's comments | | `writeComment(pr, body)`, `writeComment(issue, body)` | posts a comment | @@ -74,10 +74,11 @@ and `issue` an `IssueHandle`. returns `Either[BuildWaitFailed, BuildStatus]`. `createPr` is idempotent by branch: if a PR is already open for the branch, it -returns that one. `upsertComment` finds a prior comment carrying `marker` and -edits it in place, posting a new one only when there is none. `updatePr` is -harmless to re-run. This idempotency is what makes a resumed stage safe, see -[Pull requests](../authoring/pull-requests.md). +returns that one as it is: `readiness` applies only to a PR this call opens, and +never changes the state of an existing one. `upsertComment` finds a prior +comment carrying `marker` and edits it in place, posting a new one only when +there is none. `updatePr` is harmless to re-run. This idempotency is what makes +a resumed stage safe, see [Pull requests](../authoring/pull-requests.md). ## `fs` diff --git a/docs/authoring/pull-requests.md b/docs/authoring/pull-requests.md index da7d49a4..cf87b919 100644 --- a/docs/authoring/pull-requests.md +++ b/docs/authoring/pull-requests.md @@ -9,7 +9,7 @@ There are two calls that open a PR, and one that only writes its description. ### `openPrIfGitHub` -`openPrIfGitHub(summarisingAgent, openFindings, title?, body?, context?, instructions?): Option[PrHandle]` +`openPrIfGitHub(summarisingAgent, openFindings, title?, body?, context?, instructions?, readiness?): Option[PrHandle]` pushes the branch, summarises the change and opens the PR, provided that `gh.availability` says the checkout is on a reachable GitHub. Otherwise it emits one step saying why and returns `None`; the run then finishes. This is @@ -23,7 +23,7 @@ included. ### `openPrFromBranch` -`openPrFromBranch(summarisingAgent, openFindings, title?, body?, context?, instructions?): PrHandle` +`openPrFromBranch(summarisingAgent, openFindings, title?, body?, context?, instructions?, readiness?): PrHandle` runs the same three stages, but fails the run when there is no GitHub remote or `gh` login. @@ -39,6 +39,13 @@ Both open calls create stages, so they cannot be called inside a `stage(...)` body. Call them at the top level of the flow. ``` +### Draft PRs + +Both open calls, like `gh.createPr`, open a draft when the run gets `--draft`, +and a ready PR otherwise. A flow can override this with +`readiness = Some(PrReadiness.Draft)` or `Some(PrReadiness.Ready)`. It applies only to a PR the run opens: a PR +already open for the branch is reused with its state unchanged. + ### Findings, title, body and context `openFindings` is the final review's diff --git a/docs/using/run-lifecycle.md b/docs/using/run-lifecycle.md index f5832f0f..c55a08d4 100644 --- a/docs/using/run-lifecycle.md +++ b/docs/using/run-lifecycle.md @@ -88,7 +88,8 @@ A few things are worth knowing: A re-run with the same prompt finds the progress log and resumes from the first incomplete stage. If `--branch` names a different branch than the one in the -log, the run is refused. On resume, Orca prints which branch the run is bound +log, the run is refused. A resume keeps the first attempt's `--draft`, whatever +this attempt passes. On resume, Orca prints which branch the run is bound to, how many stages are already done, and that the interrupted stage's uncommitted work was dropped. Each [durable session](../authoring/talking-to-agents.md) that is resumed gets the diff --git a/docs/using/shell.md b/docs/using/shell.md index 4c3cfad8..55772526 100644 --- a/docs/using/shell.md +++ b/docs/using/shell.md @@ -84,6 +84,7 @@ job. The commands are: | `--skip-branch` | continue on the current branch instead of creating one | | `--keep-changes` | leave uncommitted files in place instead of stashing them | | `--worktree` | run in a git worktree of this repository instead of the checkout | +| `--draft` | open the run's PR as a draft | | `--honor-pin` | use the flow's own pinned Orca version | | `--verbose` | print a stack trace on abort | diff --git a/flow/src/main/scala/orca/pr/openPrFromBranch.scala b/flow/src/main/scala/orca/pr/openPrFromBranch.scala index dd57eda3..fbe3fa83 100644 --- a/flow/src/main/scala/orca/pr/openPrFromBranch.scala +++ b/flow/src/main/scala/orca/pr/openPrFromBranch.scala @@ -3,7 +3,7 @@ package orca.pr import orca.{FlowContext, FlowControl, OutsideStage, fail, gh, git, tracedStage} import orca.agents.Agent import orca.review.OpenFindings -import orca.tools.PrHandle +import orca.tools.{PrHandle, PrReadiness} import ox.either.orThrow @@ -37,10 +37,13 @@ import ox.either.orThrow * findings it left unfixed. The same section is printed to the run output, * also when opening the PR fails. * + * `readiness` says whether the PR opens as a draft; when omitted, the run's + * `--draft` decides. + * * `gh.createPr` is idempotent by head branch: a re-run that already opened the - * PR gets the existing handle back rather than failing. Returns that handle, - * recorded as the run's published work through [[recordOpenedPr]] inside the - * create stage. + * PR gets the existing handle back, its draft state untouched, rather than + * failing. Returns that handle, recorded as the run's published work through + * [[recordOpenedPr]] inside the create stage. * * Refused inside a stage — at compile time where the call sits in a stage * body, at run time where it is reached through a helper: opening the PR is a @@ -52,7 +55,8 @@ def openPrFromBranch( title: PrSummary => String = _.title, body: PrSummary => String = _.body, context: Option[String] = None, - instructions: String = PrPrompts.Summarise + instructions: String = PrPrompts.Summarise, + readiness: Option[PrReadiness] = None )(using FlowContext, FlowControl, OutsideStage): PrHandle = summon[FlowControl].assertAtFlowBody("openPrFromBranch(...)") reportOpenFindings(openFindings) @@ -72,7 +76,8 @@ def openPrFromBranch( val pr = gh .createPr( title = title(summary), - body = bodyWithOpenFindings(body(summary), openFindings) + body = bodyWithOpenFindings(body(summary), openFindings), + readiness = readiness ) .orThrow recordOpened(pr) diff --git a/flow/src/main/scala/orca/pr/openPrIfGitHub.scala b/flow/src/main/scala/orca/pr/openPrIfGitHub.scala index 56881c2c..cc9b82d7 100644 --- a/flow/src/main/scala/orca/pr/openPrIfGitHub.scala +++ b/flow/src/main/scala/orca/pr/openPrIfGitHub.scala @@ -15,7 +15,7 @@ import orca.agents.Agent import orca.events.OrcaEvent import orca.progress.ThrowawayBranch import orca.review.OpenFindings -import orca.tools.{GitHubAvailability, NoDefaultBase, PrHandle} +import orca.tools.{GitHubAvailability, NoDefaultBase, PrHandle, PrReadiness} import orca.util.TextUtil import scala.util.control.NonFatal @@ -45,7 +45,8 @@ def openPrIfGitHub( title: PrSummary => String = _.title, body: PrSummary => String = _.body, context: Option[String] = None, - instructions: String = PrPrompts.Summarise + instructions: String = PrPrompts.Summarise, + readiness: Option[PrReadiness] = None )(using ctx: FlowContext, control: FlowControl, @@ -62,7 +63,8 @@ def openPrIfGitHub( // threaded through the legs below. body = summary => bodyWithOpenFindings(body(summary), openFindings), context = context, - instructions = instructions + instructions = instructions, + readiness = readiness ).fold(skipped, Some(_)) /** [[openPrFromBranch]]'s three stages, the two remote-facing legs under @@ -82,7 +84,8 @@ private def pushThenCreate( title: PrSummary => String, body: PrSummary => String, context: Option[String], - instructions: String + instructions: String, + readiness: Option[PrReadiness] )(using FlowContext, FlowControl): Either[String, PrHandle] = for push <- gatedStage(PushStage)(preFlight(base))(_ => pushBestEffort()) @@ -90,7 +93,7 @@ private def pushThenCreate( summary <- summarise(summarisingAgent, base, context, instructions).left .map(baseStopReason) create = tracedStage(CreateStage)( - createBestEffort(title(summary), body(summary)) + createBestEffort(title(summary), body(summary), readiness) ) pr <- create.value.outcome.left.map(refusalLine(_, create)) yield pr @@ -147,7 +150,11 @@ private def pushBestEffort()(using FlowContext, WorkspaceWrite): PushResult = /** [[openPrFromBranch]]'s create with the `gh pr create` under [[bestEffort]], * as [[pushBestEffort]] is for the push. */ -private def createBestEffort(title: String, body: String)(using +private def createBestEffort( + title: String, + body: String, + readiness: Option[PrReadiness] +)(using FlowContext, FlowControl, WorkspaceWrite @@ -155,7 +162,7 @@ private def createBestEffort(title: String, body: String)(using bestEffort( "could not open a PR", "open it yourself from the pushed branch" - )(gh.createPr(title = title, body = body)) match + )(gh.createPr(title = title, body = body, readiness = readiness)) match case Left(reason) => CreateResult.Refused(reason) // Outside `bestEffort`, so a log that cannot be written fails the run. case Right(pr) => recordOpened(pr) diff --git a/flow/src/main/scala/orca/progress/ProgressLog.scala b/flow/src/main/scala/orca/progress/ProgressLog.scala index 68cf12b4..fbf80785 100644 --- a/flow/src/main/scala/orca/progress/ProgressLog.scala +++ b/flow/src/main/scala/orca/progress/ProgressLog.scala @@ -4,6 +4,7 @@ import com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec import orca.StagePath import orca.gitref.{BranchName, CommitHash, Head} import orca.json.{JsonData, RawJson, given} +import orca.tools.PrReadiness /** Whether orca minted [[ProgressHeader.branch]] itself or bound to a * pre-existing one. Gates the throwaway-branch auto-delete @@ -25,7 +26,8 @@ enum BranchMode derives JsonData: * `userPrompt` is the full prompt: the shell's "Resume interrupted run" offer * relaunches the run byte-identically (ADR 0021 §3 amendment), and * `RecoveryCheck` refuses a log whose prompt is not the current one. `flow` is - * `None` for a run started outside the shell. + * `None` for a run started outside the shell. `prReadiness` is the first + * attempt's `--draft`, which every resume keeps. * * `startingCommit` is the commit HEAD pointed at when the run bound its branch * — the diff base for a review of everything the whole run changed. @@ -38,7 +40,8 @@ case class ProgressHeader( branchMode: BranchMode, userPrompt: String, flow: Option[FlowSource], - startingCommit: CommitHash + startingCommit: CommitHash, + prReadiness: PrReadiness ) derives JsonData: /** Where the run started, which a successful run may hand HEAD back to. */ def startingHead: Head = diff --git a/flow/src/test/scala/orca/FlowSessionTest.scala b/flow/src/test/scala/orca/FlowSessionTest.scala index 784b3ddb..659500e7 100644 --- a/flow/src/test/scala/orca/FlowSessionTest.scala +++ b/flow/src/test/scala/orca/FlowSessionTest.scala @@ -1,5 +1,7 @@ package orca +import orca.tools.PrReadiness + import munit.FunSuite import orca.backend.{ AgentResult, @@ -232,7 +234,8 @@ class FlowSessionTest extends FunSuite: BranchMode.Created, userPrompt = "p", flow = None, - startingCommit = orca.gitref.CommitHash.from("0" * 40).get + startingCommit = orca.gitref.CommitHash.from("0" * 40).get, + prReadiness = PrReadiness.Ready ) ) for record <- sessions do sessionStore.upsert(record) @@ -754,7 +757,8 @@ class FlowSessionTest extends FunSuite: BranchMode.Created, userPrompt = "p", flow = None, - startingCommit = orca.gitref.CommitHash.from("0" * 40).get + startingCommit = orca.gitref.CommitHash.from("0" * 40).get, + prReadiness = PrReadiness.Ready ) ) sessionStore.upsert( diff --git a/flow/src/test/scala/orca/TestRun.scala b/flow/src/test/scala/orca/TestRun.scala index d8a6491d..990f6036 100644 --- a/flow/src/test/scala/orca/TestRun.scala +++ b/flow/src/test/scala/orca/TestRun.scala @@ -1,5 +1,7 @@ package orca +import orca.tools.PrReadiness + import orca.agents.{Agent, BackendTag} import orca.events.EventDispatcher import orca.progress.{BranchMode, ProgressHeader, ProgressStore} @@ -47,7 +49,8 @@ object TestRun: BranchMode.Created, userPrompt = userPrompt, flow = None, - startingCommit = headCommit + startingCommit = headCommit, + prReadiness = PrReadiness.Ready ) ) val context = new TestFlowContext( diff --git a/flow/src/test/scala/orca/pr/OpenPrFromBranchTest.scala b/flow/src/test/scala/orca/pr/OpenPrFromBranchTest.scala index 20d2e27a..705fb6a4 100644 --- a/flow/src/test/scala/orca/pr/OpenPrFromBranchTest.scala +++ b/flow/src/test/scala/orca/pr/OpenPrFromBranchTest.scala @@ -1,7 +1,15 @@ package orca.pr import munit.FunSuite -import orca.{BoundedDiff, OrcaFlowException, OutsideStage, interceptReported} +import orca.{ + BoundedDiff, + FlowContext, + FlowControl, + OrcaFlowException, + OutsideStage, + interceptReported +} +import orca.agents.Agent import orca.plan.Title import orca.progress.PublishedWork import orca.review.{FindingId, OpenFinding, OpenFindings, OpenReason} @@ -10,6 +18,7 @@ import orca.tools.{ GitHubAvailability, PrCreateFailed, PrHandle, + PrReadiness, PushFailure } import orca.events.{OrcaEvent, OrcaListener} @@ -74,6 +83,26 @@ class OpenPrFromBranchTest extends FunSuite: steps.asScala.toList ) + /** The readiness each `gh.createPr` got while `open` ran over a fresh seeded + * repo, given a summariser to pass on. + */ + private def createPrReadinesses( + open: Agent[?] => (FlowContext, FlowControl) ?=> Any + ): List[Option[PrReadiness]] = + val (dir, store) = seededPrRepo() + val readinesses = new ConcurrentLinkedQueue[Option[PrReadiness]]() + val run = prRun( + dir, + store, + _ => (), + new ConcurrentLinkedQueue[String](), + availability = GitHubAvailability.Available("github.com", "acme", "w"), + prReadinesses = readinesses + ) + import run.given + val _ = open(new StubSummariser().agent) + readinesses.asScala.toList + private val oneOpen = OpenFindings( List( OpenFinding( @@ -101,6 +130,48 @@ class OpenPrFromBranchTest extends FunSuite: List("Push branch", "Generate PR title and description", "Open PR") ) + test("both open calls leave the readiness to gh by default"): + assertEquals( + createPrReadinesses(agent => + openPrFromBranch( + summarisingAgent = agent, + openFindings = OpenFindings.empty + ) + ), + List(None) + ) + assertEquals( + createPrReadinesses(agent => + openPrIfGitHub( + summarisingAgent = agent, + openFindings = OpenFindings.empty + ) + ), + List(None) + ) + + test("both open calls pass an explicit readiness on"): + assertEquals( + createPrReadinesses(agent => + openPrFromBranch( + summarisingAgent = agent, + openFindings = OpenFindings.empty, + readiness = Some(PrReadiness.Draft) + ) + ), + List(Some(PrReadiness.Draft)) + ) + assertEquals( + createPrReadinesses(agent => + openPrIfGitHub( + summarisingAgent = agent, + openFindings = OpenFindings.empty, + readiness = Some(PrReadiness.Draft) + ) + ), + List(Some(PrReadiness.Draft)) + ) + test("openPrFromBranch reached inside a stage is refused before it pushes"): // A helper taking only FlowControl carries OutsideStage past the compile // check; the stage open around it is caught at run time. diff --git a/flow/src/test/scala/orca/pr/PrFakes.scala b/flow/src/test/scala/orca/pr/PrFakes.scala index 6bdab0ea..e7780c87 100644 --- a/flow/src/test/scala/orca/pr/PrFakes.scala +++ b/flow/src/test/scala/orca/pr/PrFakes.scala @@ -10,6 +10,7 @@ import orca.tools.{ OsGitTool, PrCreateFailed, PrHandle, + PrReadiness, PushFailure, RuntimeGit } @@ -62,13 +63,19 @@ private[pr] class RecordingGh( calls: ConcurrentLinkedQueue[String], availabilityAnswer: => GitHubAvailability, createPrAnswer: => Either[PrCreateFailed, PrHandle] = Right(samplePr), - bodies: ConcurrentLinkedQueue[String] + bodies: ConcurrentLinkedQueue[String], + readinesses: ConcurrentLinkedQueue[Option[PrReadiness]] ) extends StubGitHubTool: override def availability(): GitHubAvailability = calls.add("availability"): Unit availabilityAnswer - override def createPr(title: String, body: String)(using WorkspaceWrite) = + override def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite) = calls.add("createPr"): Unit + readinesses.add(readiness): Unit bodies.add(body): Unit createPrAnswer @@ -133,7 +140,8 @@ private[pr] def seededPrRepo( branchMode, userPrompt = "p", flow = None, - startingCommit = startedAt + startingCommit = startedAt, + prReadiness = PrReadiness.Ready ) ) (dir, store) @@ -154,7 +162,9 @@ private[pr] def prRun( push: => Either[PushFailure, Unit] = Right(()), base: => Either[NoDefaultBase, String] = Right("main"), prBodies: ConcurrentLinkedQueue[String] = - new ConcurrentLinkedQueue[String]() + new ConcurrentLinkedQueue[String](), + prReadinesses: ConcurrentLinkedQueue[Option[PrReadiness]] = + new ConcurrentLinkedQueue[Option[PrReadiness]]() ): TestRun = TestRun( new TestFlowControl( @@ -168,7 +178,9 @@ private[pr] def prRun( wiredGit = Some( new RecordingGit(new OsGitTool(dir), calls, branchDiff, push, base) ), - wiredGh = Some(new RecordingGh(calls, availability, createPr, prBodies)) + wiredGh = Some( + new RecordingGh(calls, availability, createPr, prBodies, prReadinesses) + ) ), dir ) diff --git a/flow/src/test/scala/orca/progress/ProgressLogTest.scala b/flow/src/test/scala/orca/progress/ProgressLogTest.scala index 79af7d0d..71128e95 100644 --- a/flow/src/test/scala/orca/progress/ProgressLogTest.scala +++ b/flow/src/test/scala/orca/progress/ProgressLogTest.scala @@ -1,5 +1,7 @@ package orca.progress +import orca.tools.PrReadiness + import com.github.plokhotnyuk.jsoniter_scala.core.{ JsonReaderException, readFromString, @@ -28,7 +30,8 @@ class ProgressLogTest extends FunSuite: userPrompt = "fix the flaky test", flow = flow, startingCommit = - CommitHash.from("0badc0ffee0ddf00d1234567890abcdef1234567").get + CommitHash.from("0badc0ffee0ddf00d1234567890abcdef1234567").get, + prReadiness = PrReadiness.Ready ) test("ProgressLog round-trips through JsonData codec"): diff --git a/flow/src/test/scala/orca/progress/ProgressScanTest.scala b/flow/src/test/scala/orca/progress/ProgressScanTest.scala index ecb40faf..4f142fea 100644 --- a/flow/src/test/scala/orca/progress/ProgressScanTest.scala +++ b/flow/src/test/scala/orca/progress/ProgressScanTest.scala @@ -1,5 +1,7 @@ package orca.progress +import orca.tools.PrReadiness + import munit.FunSuite import orca.{RunKey, WorkspaceWrite} import orca.gitref.CommitHash @@ -17,7 +19,8 @@ class ProgressScanTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = "my prompt", flow = None, - startingCommit = CommitHash.from("0" * 40).get + startingCommit = CommitHash.from("0" * 40).get, + prReadiness = PrReadiness.Ready ) test("progressLogPaths is empty when .orca doesn't exist"): diff --git a/flow/src/test/scala/orca/progress/ProgressStoreTest.scala b/flow/src/test/scala/orca/progress/ProgressStoreTest.scala index 50f7bc4a..c4a97321 100644 --- a/flow/src/test/scala/orca/progress/ProgressStoreTest.scala +++ b/flow/src/test/scala/orca/progress/ProgressStoreTest.scala @@ -1,5 +1,7 @@ package orca.progress +import orca.tools.PrReadiness + import munit.FunSuite import orca.{RunKey, StagePath, WorkspaceWrite} import orca.gitref.CommitHash @@ -17,7 +19,8 @@ class ProgressStoreTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = "my prompt", flow = None, - startingCommit = CommitHash.from("0" * 40).get + startingCommit = CommitHash.from("0" * 40).get, + prReadiness = PrReadiness.Ready ) test("writeHeader then load returns the header with empty entries"): diff --git a/flow/src/test/scala/orca/progress/RecoveryCheckTest.scala b/flow/src/test/scala/orca/progress/RecoveryCheckTest.scala index 94709677..2d73b820 100644 --- a/flow/src/test/scala/orca/progress/RecoveryCheckTest.scala +++ b/flow/src/test/scala/orca/progress/RecoveryCheckTest.scala @@ -1,5 +1,7 @@ package orca.progress +import orca.tools.PrReadiness + import munit.FunSuite import orca.gitref.CommitHash import orca.testkit.branchName @@ -17,7 +19,8 @@ class RecoveryCheckTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = testCommit + startingCommit = testCommit, + prReadiness = PrReadiness.Ready ) assert( RecoveryCheck.validateHeader(header, prompt, Set.empty).isLeft, @@ -34,7 +37,8 @@ class RecoveryCheckTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = testCommit + startingCommit = testCommit, + prReadiness = PrReadiness.Ready ) val rejected = RecoveryCheck.validateHeader(header, prompt, Set("trunk")) assert( @@ -64,7 +68,8 @@ class RecoveryCheckTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = testCommit + startingCommit = testCommit, + prReadiness = PrReadiness.Ready ) assertEquals( RecoveryCheck.validateHeader(header, prompt, Set.empty).map(_.value), @@ -78,7 +83,8 @@ class RecoveryCheckTest extends FunSuite: branchMode = BranchMode.Created, userPrompt = "a different prompt", flow = None, - startingCommit = testCommit + startingCommit = testCommit, + prReadiness = PrReadiness.Ready ) assert( RecoveryCheck.validateHeader(header, "do the thing", Set.empty).isLeft diff --git a/runner/src/main/scala/orca/OrcaArgs.scala b/runner/src/main/scala/orca/OrcaArgs.scala index 75b911ad..fc8ea9e4 100644 --- a/runner/src/main/scala/orca/OrcaArgs.scala +++ b/runner/src/main/scala/orca/OrcaArgs.scala @@ -2,13 +2,15 @@ package orca import mainargs.ParserForClass import orca.gitref.BranchName +import orca.tools.PrReadiness /** Parsed command-line arguments for the `orca` entry point. */ case class OrcaArgs( userPrompt: String = "", verbose: Boolean = false, target: RunTarget = RunTarget.NewBranch(Uncommitted.Stash), - branch: Option[BranchName] = None + branch: Option[BranchName] = None, + prReadiness: PrReadiness = PrReadiness.Ready ): /** The argv [[OrcaArgs.parse]] reads back as this value; the shell passes it * to a flow child. A `branch` with a [[RunTarget.CurrentBranch]] target @@ -32,7 +34,10 @@ case class OrcaArgs( Seq("--skip-branch", "--keep-changes") case RunTarget.Worktree => Seq("--worktree") val branchArgv = branch.toList.flatMap(name => Seq("--branch", name.value)) - promptArgv ++ verboseArgv ++ targetArgv ++ branchArgv + val draftArgv = prReadiness match + case PrReadiness.Draft => Seq("--draft") + case PrReadiness.Ready => Nil + promptArgv ++ verboseArgv ++ targetArgv ++ branchArgv ++ draftArgv object OrcaArgs: diff --git a/runner/src/main/scala/orca/RawArgs.scala b/runner/src/main/scala/orca/RawArgs.scala index 38ad5c66..8135653b 100644 --- a/runner/src/main/scala/orca/RawArgs.scala +++ b/runner/src/main/scala/orca/RawArgs.scala @@ -3,6 +3,7 @@ package orca import mainargs.{Flag, ParserForClass, arg} import orca.gitref.BranchName import orca.progress.FeatureBranch +import orca.tools.PrReadiness /** The argv shape mainargs parses, shared by a flow's own argv * ([[OrcaArgs.parse]]) and `orca run`, so each option is declared once. It @@ -29,7 +30,9 @@ private[orca] case class RawArgs( @arg(doc = "name of the branch to create for this run (default: derived from the prompt); not with --skip-branch" ) - branch: Option[String] + branch: Option[String], + @arg(doc = "open the run's pull request as a draft") + draft: Flag ): /** Refuses a prompt given both positionally and with `--prompt`, an invalid * `--branch`, and a refused flag pair. @@ -51,7 +54,15 @@ private[orca] case class RawArgs( keepChanges = keepChanges.value, branch = branchName ) - yield CheckedArgs(givenPrompt, verbose.value, target, branchName) + prReadiness = + if draft.value then PrReadiness.Draft else PrReadiness.Ready + yield CheckedArgs( + givenPrompt, + verbose.value, + target, + branchName, + prReadiness + ) private[orca] object RawArgs: given ParserForClass[RawArgs] = ParserForClass[RawArgs] @@ -63,12 +74,14 @@ private[orca] case class CheckedArgs( givenPrompt: Option[String], verbose: Boolean, target: RunTarget, - branch: Option[BranchName] + branch: Option[BranchName], + prReadiness: PrReadiness ): def withPrompt(userPrompt: String): OrcaArgs = OrcaArgs( userPrompt = userPrompt, verbose = verbose, target = target, - branch = branch + branch = branch, + prReadiness = prReadiness ) diff --git a/runner/src/main/scala/orca/exports.scala b/runner/src/main/scala/orca/exports.scala index d9ecb574..e103799e 100644 --- a/runner/src/main/scala/orca/exports.scala +++ b/runner/src/main/scala/orca/exports.scala @@ -119,6 +119,7 @@ export orca.tools.{ IssueHandle, NoDefaultBase, PrHandle, + PrReadiness, PushFailure } // Agent-override surface: the wiring an override factory receives, plus each diff --git a/runner/src/main/scala/orca/flowEntry.scala b/runner/src/main/scala/orca/flowEntry.scala index 6fc51f47..53519092 100644 --- a/runner/src/main/scala/orca/flowEntry.scala +++ b/runner/src/main/scala/orca/flowEntry.scala @@ -34,6 +34,7 @@ import orca.runner.{ OrcaLog, RoleAgents, RoleOverrides, + RunGitHubTool, RunLogStarter, RunRequest, SetupOptions, @@ -460,7 +461,7 @@ private def runInContext( reviewAgent = resolvedRoles.review, wired = agents, runtimeGit = runtimeGit, - gh = ghTool, + gh = RunGitHubTool(ghTool, flowSetup.prReadiness), fs = fsTool, stackSettings = flowSetup.stackSettings, reviewerCatalog = reviewerCatalog diff --git a/runner/src/main/scala/orca/runner/FlowLifecycle.scala b/runner/src/main/scala/orca/runner/FlowLifecycle.scala index 4d8aeda5..bf0ba95c 100644 --- a/runner/src/main/scala/orca/runner/FlowLifecycle.scala +++ b/runner/src/main/scala/orca/runner/FlowLifecycle.scala @@ -33,7 +33,7 @@ import orca.progress.{ } import orca.settings.{AgentSettings, SettingsFile, SettingsScope} import orca.subprocess.TtyProbe -import orca.tools.{RuntimeGit, UntrackedFiles} +import orca.tools.{PrReadiness, RuntimeGit, UntrackedFiles} import org.slf4j.LoggerFactory import ox.either.orThrow @@ -137,22 +137,27 @@ object FlowLifecycle: * `--worktree`, or a resume the shell relaunched into one without the * flag. The closing summary names it. */ - worktree: Option[os.Path] + worktree: Option[os.Path], + /** What a `createPr` without a readiness opens: this run's `--draft`, or + * on resume the first attempt's. + */ + prReadiness: PrReadiness ) /** The branch half of [[FlowSetup]], resolved by whichever of * [[SetupSession.bindBranch]]'s arms runs (corrupt log, absent log, or * resumed) before the stack-settings half is folded in. * - * `startingCommit` comes from the same arm: a fresh run records the commit - * it bound at, a resumed one reads back what its header recorded, so both - * name the state the RUN started from rather than this attempt's. + * `startingCommit` and `prReadiness` come from the same arm: a fresh run + * takes this attempt's, a resumed one reads back what its header recorded, + * so both describe the RUN rather than this attempt. */ private[orca] case class BranchBinding( featureBranch: FeatureBranch, startingHead: Head, branchMode: BranchMode, - startingCommit: Option[CommitHash] + startingCommit: Option[CommitHash], + prReadiness: PrReadiness ) /** Bind the run to a branch + progress log before the body runs (ADR 0018 @@ -245,7 +250,8 @@ object FlowLifecycle: // From where the run IS, not from the flag: the shell relaunches a resume // inside the worktree its log was found in WITHOUT `--worktree` (the flag // would re-derive a path), and that run has to name the tree too. - Option.when(WorktreeRun.isWorktreeRun(workDir))(workDir) + Option.when(WorktreeRun.isWorktreeRun(workDir))(workDir), + binding.prReadiness ) /** [[setup]]'s fixed inputs — the coding-role agent, git, the progress store, @@ -377,7 +383,8 @@ object FlowLifecycle: startingHead, if args.target.skipBranch then BranchMode.Reused else BranchMode.Created, - Some(headAtBinding) + Some(headAtBinding), + args.prReadiness ) /** Fresh run: resolve + create the branch, then commit the header as the @@ -434,7 +441,8 @@ object FlowLifecycle: else BranchMode.Created, userPrompt = args.userPrompt, flow = flowSource, - startingCommit = headAtBinding + startingCommit = headAtBinding, + prReadiness = args.prReadiness ) ) git.forceCommitOnly(store.path, "orca: progress log") @@ -494,7 +502,8 @@ object FlowLifecycle: featureBranch, header.startingHead, header.branchMode, - startingCommit + startingCommit, + header.prReadiness ) /** What a resumed run says about itself, once. `createBranch` and diff --git a/runner/src/main/scala/orca/runner/RunGitHubTool.scala b/runner/src/main/scala/orca/runner/RunGitHubTool.scala new file mode 100644 index 00000000..d208de25 --- /dev/null +++ b/runner/src/main/scala/orca/runner/RunGitHubTool.scala @@ -0,0 +1,24 @@ +package orca.runner + +import orca.WorkspaceWrite +import orca.tools.{GitHubTool, PrCreateFailed, PrHandle, PrReadiness} + +/** The `gh` a run's flow sees: `underlying` with a `createPr` that opens + * `runReadiness` when its caller passes no readiness. + */ +private[orca] class RunGitHubTool( + underlying: GitHubTool, + runReadiness: PrReadiness +) extends GitHubTool: + export underlying.{createPr as _, *} + + // `export` skips members the trait already implements. + override def prHandle(ref: String): Either[String, PrHandle] = + underlying.prHandle(ref) + + def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite): Either[PrCreateFailed, PrHandle] = + underlying.createPr(title, body, readiness.orElse(Some(runReadiness))) diff --git a/runner/src/test/scala/flowtests/FlowCompilesTest.scala b/runner/src/test/scala/flowtests/FlowCompilesTest.scala index 5b8b9a79..26789439 100644 --- a/runner/src/test/scala/flowtests/FlowCompilesTest.scala +++ b/runner/src/test/scala/flowtests/FlowCompilesTest.scala @@ -353,6 +353,22 @@ object FlowCanary: case Left(e) => throw e case Right(_) => () + /** Draft PRs: `PrReadiness` resolves from `import orca.*` alone and both open + * calls take it, overriding `--draft`. + */ + def draftPrSurface(): Unit = + flow(OrcaArgs()): + val _: Option[PrHandle] = openPrIfGitHub( + summarisingAgent = claude.haiku, + openFindings = OpenFindings.empty, + readiness = Some(PrReadiness.Draft) + ) + val _: PrHandle = openPrFromBranch( + summarisingAgent = claude.haiku, + openFindings = OpenFindings.empty, + readiness = Some(PrReadiness.Draft) + ) + /** Planning grid surface; exercised across `flows/`. Pins the full `mode × * operation` grid: every cell returns `WithChat[]` where the result * is `Plan` (`from`) or `Triage` (`triage`). diff --git a/runner/src/test/scala/orca/OrcaArgsPropertyTest.scala b/runner/src/test/scala/orca/OrcaArgsPropertyTest.scala index a41fc80c..29e15390 100644 --- a/runner/src/test/scala/orca/OrcaArgsPropertyTest.scala +++ b/runner/src/test/scala/orca/OrcaArgsPropertyTest.scala @@ -35,7 +35,8 @@ class OrcaArgsPropertyTest extends ScalaCheckSuite: target <- targetGen branch <- if target.skipBranch then Gen.const(None) else Gen.option(branchGen) - yield OrcaArgs(prompt, verbose, target, branch) + prReadiness <- Gen.oneOf(PrReadiness.values.toSeq) + yield OrcaArgs(prompt, verbose, target, branch, prReadiness) property("parse reads toArgv back as the same args"): forAll(argsGen): args => diff --git a/runner/src/test/scala/orca/OrcaArgsTest.scala b/runner/src/test/scala/orca/OrcaArgsTest.scala index a96a8e43..ff236876 100644 --- a/runner/src/test/scala/orca/OrcaArgsTest.scala +++ b/runner/src/test/scala/orca/OrcaArgsTest.scala @@ -103,6 +103,16 @@ class OrcaArgsTest extends munit.FunSuite: case Left(msg) => assert(msg.nonEmpty) case Right(r) => fail(s"expected parse failure, got $r") + test("--draft makes the run's PR a draft; absent, a ready one"): + assertEquals( + OrcaArgs.parse(Seq("--draft", "x")).map(_.prReadiness), + Right(PrReadiness.Draft) + ) + assertEquals( + OrcaArgs.parse(Seq("x")).map(_.prReadiness), + Right(PrReadiness.Ready) + ) + test("--prompt carries a prompt that starts with '-'"): assertEquals( OrcaArgs.parse(Seq("--prompt", "- add X")).map(_.userPrompt), diff --git a/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala b/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala index 3e6bc42d..721f1a40 100644 --- a/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala +++ b/runner/src/test/scala/orca/runner/FlowLifecycleTest.scala @@ -36,6 +36,7 @@ import orca.tools.{ GitHubAvailability, GitHubTool, OsGitTool, + PrReadiness, RuntimeGit, UntrackedFiles, Worktrees @@ -46,6 +47,8 @@ import ox.either.orThrow import java.io.{ByteArrayOutputStream, PrintStream} import java.util.concurrent.atomic.{AtomicInteger, AtomicReference} +import java.util.concurrent.ConcurrentLinkedQueue +import scala.jdk.CollectionConverters.* import orca.testkit.{ GitRepo, PushlessGit, @@ -249,7 +252,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -322,7 +326,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = GitRepo.headCommit(workDir) + startingCommit = GitRepo.headCommit(workDir), + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -605,7 +610,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -739,7 +745,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = userPrompt, flow = flow, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) store @@ -911,7 +918,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) val _ = writeForeignLog(workDir, branch = "feat/resume-me") @@ -996,7 +1004,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, startingCommit = unreachableCommit, userPrompt = prompt, - flow = None + flow = None, + prReadiness = PrReadiness.Ready ) ) val setup = setupForSettings(workDir, Some(StackSettings.empty), prompt) @@ -1114,6 +1123,38 @@ class FlowLifecycleTest extends munit.FunSuite: "feat/x" ) + /** [[setupWithBranch]]'s twin for `--draft`: one prompt, so a second call + * resumes the first. + */ + private def setupWithReadiness( + workDir: os.Path, + prReadiness: PrReadiness + ): FlowLifecycle.FlowSetup = + val prompt = "a task with a chosen readiness" + setupForSettings( + workDir, + Some(StackSettings.empty), + prompt, + args = Some(OrcaArgs(prompt, prReadiness = prReadiness)) + ) + + test("setup: a fresh run records its --draft in the header"): + val workDir = GitRepo.seeded() + val setup = setupWithReadiness(workDir, PrReadiness.Draft) + assertEquals(setup.prReadiness, PrReadiness.Draft) + assertEquals( + setup.store.load().map(_.header.prReadiness), + Some(PrReadiness.Draft) + ) + + test("setup: a resume keeps the first attempt's readiness over its own"): + val workDir = GitRepo.seeded() + val _ = setupWithReadiness(workDir, PrReadiness.Ready) + assertEquals( + setupWithReadiness(workDir, PrReadiness.Draft).prReadiness, + PrReadiness.Ready + ) + test("setup: --branch naming the detected default branch is refused"): val workDir = GitRepo.seeded() val git = new OsGitTool(workDir): @@ -1597,7 +1638,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -1778,7 +1820,8 @@ class FlowLifecycleTest extends munit.FunSuite: claude: ClaudeAgent = StubAgent.claude, gh: Option[GitHubTool] = None, git: Option[RuntimeGit] = None, - target: RunTarget = RunTarget.NewBranch(Uncommitted.Stash) + target: RunTarget = RunTarget.NewBranch(Uncommitted.Stash), + prReadiness: PrReadiness = PrReadiness.Ready )(body: (orca.FlowContext, orca.FlowControl) ?=> Unit): Unit = supervised: val interaction = TerminalInteraction.start( @@ -1788,7 +1831,7 @@ class FlowLifecycleTest extends munit.FunSuite: ) runFlow( FlowHarness.request( - args = OrcaArgs(prompt, target = target), + args = OrcaArgs(prompt, target = target, prReadiness = prReadiness), stackSettings = Some(StackSettings.empty), wiring = FlowWiring(claude = Some(_ => claude), gh = gh, git = git), workDir = workDir, @@ -1798,6 +1841,33 @@ class FlowLifecycleTest extends munit.FunSuite: ) )(body) + test("a resumed run's createPr without a readiness opens its first --draft"): + val workDir = GitRepo.seeded() + val prompt = "draft-gh" + val readinesses = new ConcurrentLinkedQueue[Option[PrReadiness]]() + val gh = new StubGitHubTool: + override def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite) = + readinesses.add(readiness): Unit + Right(handoffPr) + val _ = intercept[ReportedFailure]: + runFlowForTest(workDir, prompt, prReadiness = PrReadiness.Draft): + val _ = stage("stage-one")("one-done") + val _ = stage[String]("stage-two"): + throw new RuntimeException("boom") + runFlowForTest( + workDir, + prompt, + gh = Some(gh), + prReadiness = PrReadiness.Ready + ): + given WorkspaceWrite = WorkspaceWrite.unsafe + val _ = summon[orca.FlowContext].gh.createPr("t", "b") + assertEquals(readinesses.asScala.toList, List(Some(PrReadiness.Draft))) + test( "teardownSuccess is best-effort: a non-missing-file removal error does not fail an otherwise-successful run" ): @@ -1955,7 +2025,11 @@ class FlowLifecycleTest extends munit.FunSuite: private class StubGh extends StubGitHubTool: override def availability(): GitHubAvailability = GitHubAvailability.Available(host = "github.com", owner = "a", repo = "w") - override def createPr(title: String, body: String)(using WorkspaceWrite) = + override def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite) = Right(handoffPr) /** A claude whose structured call answers with a fixed [[PrSummary]] — what @@ -2758,7 +2832,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Reused, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -2883,7 +2958,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -3185,7 +3261,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Reused, startingTree = StartingTree.Clean, startingCommit = startedAt, - worktree = None + worktree = None, + prReadiness = PrReadiness.Ready ) FlowLifecycle.teardownSuccess( git, @@ -3225,7 +3302,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, startingTree = StartingTree.Clean, startingCommit = startedAt, - worktree = None + worktree = None, + prReadiness = PrReadiness.Ready ) (git, workDir, setup) @@ -3351,7 +3429,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Reused, startingTree = StartingTree.Clean, startingCommit = None, - worktree = None + worktree = None, + prReadiness = PrReadiness.Ready ) TeardownPushRepo(git, remote, setup, store.path.subRelativeTo(workDir)) @@ -3452,7 +3531,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = branchMode, startingTree = StartingTree.Clean, startingCommit = startingCommit, - worktree = None + worktree = None, + prReadiness = PrReadiness.Ready ) test( @@ -3570,7 +3650,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, startingCommit = base, userPrompt = prompt, - flow = None + flow = None, + prReadiness = PrReadiness.Ready ) ) store.upsertEntry( @@ -3633,7 +3714,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = GitRepo.headCommit(workDir) + startingCommit = GitRepo.headCommit(workDir), + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -3697,7 +3779,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -3759,7 +3842,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -3817,7 +3901,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = "a different prompt", flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) @@ -3862,7 +3947,8 @@ class FlowLifecycleTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = prompt, flow = None, - startingCommit = unreachableCommit + startingCommit = unreachableCommit, + prReadiness = PrReadiness.Ready ) ) git.forceAdd(store.path) diff --git a/runner/src/test/scala/orca/runner/RunGitHubToolTest.scala b/runner/src/test/scala/orca/runner/RunGitHubToolTest.scala new file mode 100644 index 00000000..6cd52742 --- /dev/null +++ b/runner/src/test/scala/orca/runner/RunGitHubToolTest.scala @@ -0,0 +1,36 @@ +package orca.runner + +import orca.WorkspaceWrite +import orca.testkit.StubGitHubTool +import orca.tools.{PrHandle, PrReadiness} + +import java.util.concurrent.ConcurrentLinkedQueue +import scala.jdk.CollectionConverters.* + +class RunGitHubToolTest extends munit.FunSuite: + given WorkspaceWrite = WorkspaceWrite.unsafe + + private val pr = + PrHandle.fromUrl("https://github.com/acme/widgets/pull/42").get + + test("createPr with a readiness keeps it over the run's"): + val seen = new ConcurrentLinkedQueue[Option[PrReadiness]]() + val underlying = new StubGitHubTool: + override def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite) = + seen.add(readiness): Unit + Right(pr) + val _ = RunGitHubTool(underlying, PrReadiness.Draft) + .createPr("t", "b", Some(PrReadiness.Ready)) + assertEquals(seen.asScala.toList, List(Some(PrReadiness.Ready))) + + test("prHandle is the wrapped tool's"): + val underlying = new StubGitHubTool: + override def prHandle(ref: String): Either[String, PrHandle] = Right(pr) + assertEquals( + RunGitHubTool(underlying, PrReadiness.Draft).prHandle("x"), + Right(pr) + ) diff --git a/shell/src/test/scala/orca/shell/menu/ResumeDetectorTest.scala b/shell/src/test/scala/orca/shell/menu/ResumeDetectorTest.scala index bbfb94fc..d925c3af 100644 --- a/shell/src/test/scala/orca/shell/menu/ResumeDetectorTest.scala +++ b/shell/src/test/scala/orca/shell/menu/ResumeDetectorTest.scala @@ -1,5 +1,7 @@ package orca.shell.menu +import orca.tools.PrReadiness + import orca.{OrcaDir, RunKey, WorkspaceWrite} import orca.gitref.CommitHash import orca.progress.{BranchMode, FlowSource, ProgressHeader, ProgressStore} @@ -21,7 +23,8 @@ class ResumeDetectorTest extends munit.FunSuite: branchMode = BranchMode.Created, userPrompt = userPrompt, flow = flow, - startingCommit = CommitHash.from("0" * 40).get + startingCommit = CommitHash.from("0" * 40).get, + prReadiness = PrReadiness.Ready ) // The scan's own guards (symlinked log file, symlinked or missing `.orca`) diff --git a/skills/orca/SKILL.md b/skills/orca/SKILL.md index dd852b43..5eb7f340 100644 --- a/skills/orca/SKILL.md +++ b/skills/orca/SKILL.md @@ -107,6 +107,7 @@ Add these flags only if the user asks: this shell's. - `--branch ` — create the run's branch under this name. Refused with `--skip-branch`, and when the branch already exists. +- `--draft` — open the run's PR as a draft. ## After it runs diff --git a/tools/src/main/scala/orca/tools/GitHubTool.scala b/tools/src/main/scala/orca/tools/GitHubTool.scala index e839c295..e3e7db69 100644 --- a/tools/src/main/scala/orca/tools/GitHubTool.scala +++ b/tools/src/main/scala/orca/tools/GitHubTool.scala @@ -217,8 +217,17 @@ trait GitHubTool: * included. Refusals with a recovery path come back as `Left`s; throws on * detached HEAD, on gh output carrying no PR URL, and on system failures * (auth, network). - */ - def createPr(title: String, body: String)(using + * + * `readiness` applies only to a PR this call opens (`gh pr create --draft`); + * when omitted, the run's `--draft` decides (applied by the run's `gh`). + * When an open PR already exists for the branch, it is returned as it is: + * neither draft nor ready state is changed. + */ + def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] = None + )(using WorkspaceWrite ): Either[PrCreateFailed, PrHandle] @@ -296,6 +305,8 @@ trait GitHubTool: * * `events` lets the tool publish a [[OrcaEvent.Step]] when a PR is opened so * the URL surfaces in the event log. Defaults to `OrcaListener.noop`. + * + * A `createPr` without a readiness opens a ready PR. */ private[orca] class OsGitHubTool( cli: CliRunner, @@ -466,8 +477,8 @@ private[orca] class OsGitHubTool( ) catch case NonFatal(e) => Left(cannotRunGh(e)) - def createPr(title: String, body: String)(using - ws: WorkspaceWrite + def createPr(title: String, body: String, readiness: Option[PrReadiness])( + using ws: WorkspaceWrite ): Either[PrCreateFailed, PrHandle] = ws.check("gh.createPr") // Inspect exit code + stderr ourselves to split the recoverable failures @@ -480,15 +491,12 @@ private[orca] class OsGitHubTool( // its progress log would be misread as "branch not pushed". Bare branch // name, no `owner:` prefix, matching `findOpenPr`. val head = headBranchForPr() + val draftFlag = readiness.getOrElse(PrReadiness.Ready) match + case PrReadiness.Draft => Seq("--draft") + case PrReadiness.Ready => Nil val result = runGhResult( - "pr", - "create", - "--title", - title, - "--body", - body, - "--head", - head + (Seq("pr", "create", "--title", title, "--body", body, "--head", head) ++ + draftFlag)* ) if result.exitCode == 0 then val output = result.stdout.trim diff --git a/tools/src/main/scala/orca/tools/PrReadiness.scala b/tools/src/main/scala/orca/tools/PrReadiness.scala new file mode 100644 index 00000000..00d542d8 --- /dev/null +++ b/tools/src/main/scala/orca/tools/PrReadiness.scala @@ -0,0 +1,9 @@ +package orca.tools + +import orca.json.JsonData + +/** Whether [[GitHubTool.createPr]] opens a PR ready for review or as a draft. + */ +enum PrReadiness derives JsonData: + case Ready + case Draft diff --git a/tools/src/test/scala/orca/testkit/StubGitHubTool.scala b/tools/src/test/scala/orca/testkit/StubGitHubTool.scala index 2c6cca52..590bf9c1 100644 --- a/tools/src/test/scala/orca/testkit/StubGitHubTool.scala +++ b/tools/src/test/scala/orca/testkit/StubGitHubTool.scala @@ -10,7 +10,8 @@ import orca.tools.{ IssueHandle, PrCreateFailed, BuildWaitFailed, - PrHandle + PrHandle, + PrReadiness } import scala.concurrent.duration.FiniteDuration @@ -24,7 +25,11 @@ class StubGitHubTool extends GitHubTool: throw new NotImplementedError(s"$endpoint is not wired in this stub") def availability(): GitHubAvailability = nyi("availability") - def createPr(title: String, body: String)(using + def createPr( + title: String, + body: String, + readiness: Option[PrReadiness] + )(using WorkspaceWrite ): Either[PrCreateFailed, PrHandle] = nyi("createPr") def updatePr(pr: PrHandle, title: String, body: String)(using diff --git a/tools/src/test/scala/orca/tools/OsGitHubToolTest.scala b/tools/src/test/scala/orca/tools/OsGitHubToolTest.scala index 016cce2c..5705877b 100644 --- a/tools/src/test/scala/orca/tools/OsGitHubToolTest.scala +++ b/tools/src/test/scala/orca/tools/OsGitHubToolTest.scala @@ -403,6 +403,42 @@ class OsGitHubToolTest extends munit.FunSuite: assert(args.containsSlice(Seq("--body", "hello"))) assert(args.containsSlice(Seq("--head", "feat"))) + test("createPr passes --draft only for a draft PR"): + def createArgs(readiness: Option[PrReadiness]): Seq[String] = + val cli = createPrRunner( + CliResult(0, "https://github.com/acme/widgets/pull/42\n", "") + ) + val _ = new OsGitHubTool(cli) + .createPr("feat: hi", "hello", readiness) + .orThrow + cli.calls.last.args + assert(createArgs(Some(PrReadiness.Draft)).contains("--draft")) + assert(!createArgs(Some(PrReadiness.Ready)).contains("--draft")) + assert(!createArgs(None).contains("--draft")) + + test("createPr leaves an existing PR alone, whatever the readiness"): + val cli = createPrRunner( + CliResult(1, "", "a pull request for branch \"feat\" already exists"), + CliResult( + 0, + "[{\"number\":42,\"url\":\"https://github.com/acme/widgets/pull/42\"}]", + "" + ) + ) + val pr = new OsGitHubTool(cli) + .createPr("feat: hi", "hello", Some(PrReadiness.Draft)) + .orThrow + assertEquals(pr, samplePr) + // Only the create and the lookup reach gh: nothing edits the found PR. + assertEquals( + cli.calls.map(_.args.take(3)), + List( + Seq("git", "symbolic-ref", "--quiet"), + Seq("gh", "pr", "create"), + Seq("gh", "pr", "list") + ) + ) + test("createPr parses the PR URL returned by gh"): val cli = createPrRunner( CliResult(0, "https://github.com/acme/widgets/pull/42\n", "")