release: prepare v0.3.13 headless resume - #32
Conversation
- orca exec resume <SESSION_ID> [PROMPT]... / resume --last continue a saved conversation as a first-class CLI capability (Codex-style), with shared run options as global flags and explicit rejection of conflicting --resume/--fork/--continue/--no-history combinations - session.completed carries the durable session_id when history is recorded; text-mode headless exits print the exact resume command - --resume-at <MESSAGE_ID> restores only up to a durable message boundary (new HistoryMode::ResumeAt; unknown boundaries fail closed) - budget-exhausted headless sessions persist a typed session.checkpoint (status, reason, budget consumed, last committed message id, task plan, resumable) before the terminal projection; resume owns a fresh budget scope while prior consumption records stay durable - pre-query transcript persistence and indeterminate repair of uncommitted tool calls verified on the resume path (resumable, not exactly-once) - docs: roadmap slice, harness contract, README (EN/ZH), v0.3.13 notes
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release adds headless session resumption by session ID, latest session, or message boundary. It persists typed budget checkpoints, includes durable session IDs in completion events, prints resume commands after unsuccessful headless runs, and updates documentation, tests, and release metadata. ChangesHeadless session resumption
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ExecCLI
participant Runtime
participant SessionStore
participant Session
User->>ExecCLI: Run resume command
ExecCLI->>Runtime: Send validated resume request
Runtime->>SessionStore: Load session through boundary
SessionStore->>Session: Restore bounded transcript
Session->>Runtime: Continue with fresh budget scope
Runtime->>SessionStore: Append checkpoint if budget is exhausted
Runtime->>ExecCLI: Emit session ID and resume command
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@crates/orca-runtime/src/session.rs`:
- Around line 312-315: Update the HistoryMode::ResumeAt branch around
SessionWriter::append_to_existing so reopening the writer preserves the selected
resume_at boundary in durable storage. Either pass and enforce the boundary when
appending, or truncate/rewrite transcript.path to remove records after resume_at
before creating the writer; keep plain Resume behavior unchanged and ensure
later resumes cannot replay the excluded tail.
In `@docs/production-roadmap.md`:
- Around line 25-26: Update the persisted-transcript compatibility statement
near the session.checkpoint roadmap entry to describe the change as an additive
record-type change, while explicitly stating that existing persisted records
remain compatible.
In `@docs/releases/v0.3.13.md`:
- Around line 40-45: Update the CLI compatibility statement in the release notes
to clarify that existing CLI arguments remain compatible, while `orca exec
resume` and `--resume-at` are additive additions.
In `@README.md`:
- Around line 91-92: Update the resume-command wording to state that only
non-success headless exits print the exact resume command: revise README.md
lines 91-92, the English changelog summary in site/src/changelog/Changelog.tsx
lines 79-80, and the equivalent Chinese summary at lines 587-588; no other
behavior changes are needed.
In `@src/cli.rs`:
- Around line 117-119: Update the resume-at argument wiring so the parent
ExecArgs::resume_at value is available to the resume subcommand: mark the parent
argument global and configure ExecResumeArgs::resume_at with
#[arg(from_global)]. Ensure exec resume builds its request from this shared
value without allowing conflicting duplicate values.
In `@tests/history_contract.rs`:
- Around line 753-788: Extend the test exec_resume_at_rejects_unknown_boundary
to parse resumed.stdout as JSONL and assert it contains no
assistant.message.delta event with mock_history_echo output, confirming
rejection occurs before provider execution while preserving the existing status
and stderr assertions.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df443209-e6df-4db5-9a59-3c9759974b67
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
Cargo.tomlREADME.mdREADME.zh-CN.mdcrates/orca-core/src/config/mod.rscrates/orca-core/src/event_schema.rscrates/orca-runtime/src/command/exec.rscrates/orca-runtime/src/command/launch.rscrates/orca-runtime/src/controller.rscrates/orca-runtime/src/runtime_host.rscrates/orca-runtime/src/server.rscrates/orca-runtime/src/session.rscrates/orca-runtime/src/thread.rscrates/orca-runtime/src/thread_store.rscrates/orca-runtime/src/thread_store/local.rscrates/orca-runtime/src/thread_store/types.rscrates/orca-runtime/src/thread_store/writer.rscrates/orca-tui/src/app.rsdocs/harness-contract.mddocs/production-roadmap.mddocs/releases/v0.3.13.mdnpm/orca/package.jsonsite/src/changelog/Changelog.tsxsite/src/shared.tssrc/cli.rstests/exec_jsonl.rstests/history_contract.rs
| HistoryMode::Resume(_) | HistoryMode::ResumeAt { .. } => match loaded_transcript { | ||
| Some(transcript) => { | ||
| let thread_session_id = transcript.meta.session_id.clone(); | ||
| match SessionWriter::append_to_existing(transcript.path) { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve the ResumeAt boundary when reopening the session writer.
SessionWriter::append_to_existing reloads every record from transcript.path. It then appends the continuation after the original tail. The in-memory conversation excludes records after resume_at, but the durable session does not.
A later plain resume can replay the excluded tail and the new continuation. Persist and honor the selected boundary when continuing a ResumeAt session, or rewrite the durable transcript before reopening its writer.
🤖 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 `@crates/orca-runtime/src/session.rs` around lines 312 - 315, Update the
HistoryMode::ResumeAt branch around SessionWriter::append_to_existing so
reopening the writer preserves the selected resume_at boundary in durable
storage. Either pass and enforce the boundary when appending, or
truncate/rewrite transcript.path to remove records after resume_at before
creating the writer; keep plain Resume behavior unchanged and ensure later
resumes cannot replay the excluded tail.
| the caller exactly how to continue. This changes no persisted transcript | ||
| schema and no server protocol. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Correct the persisted-transcript compatibility claim.
Line 25 states that the persisted transcript schema does not change. Lines 34-37 introduce a persisted session.checkpoint record. Describe this as an additive record-type change, and state that existing records remain compatible.
🤖 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 `@docs/production-roadmap.md` around lines 25 - 26, Update the
persisted-transcript compatibility statement near the session.checkpoint roadmap
entry to describe the change as an additive record-type change, while explicitly
stating that existing persisted records remain compatible.
| CLI arguments, TUI workflows, server/JSONL and ACP protocols, and SQLite | ||
| schemas are unchanged. The persisted transcript format gains one optional, | ||
| additive record type (`session.checkpoint`) written only on budget exhaustion; | ||
| older readers ignore unknown record types and existing transcripts remain | ||
| readable. `HistoryMode` gains a `ResumeAt` variant used only by the new | ||
| `--resume-at` path. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the CLI compatibility statement.
Line 40 says that CLI arguments are unchanged. This release adds orca exec resume and --resume-at. State that existing CLI arguments remain compatible and that the new arguments are additive.
🤖 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 `@docs/releases/v0.3.13.md` around lines 40 - 45, Update the CLI compatibility
statement in the release notes to clarify that existing CLI arguments remain
compatible, while `orca exec resume` and `--resume-at` are additive additions.
| branching; `orca exec resume <SESSION_ID>` restores a headless session with a | ||
| fresh budget scope, and headless exits print the exact resume command. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify that only non-success headless exits print a resume command.
Successful text-mode runs do not print this hint. The current wording conflicts with the release contract and its integration test.
README.md#L91-L92: qualify the resume-command statement with “non-success.”site/src/changelog/Changelog.tsx#L79-L80: qualify the English changelog summary with “non-success.”site/src/changelog/Changelog.tsx#L587-L588: qualify the Chinese changelog summary with the equivalent condition.
📍 Affects 2 files
README.md#L91-L92(this comment)site/src/changelog/Changelog.tsx#L79-L80site/src/changelog/Changelog.tsx#L587-L588
🤖 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 `@README.md` around lines 91 - 92, Update the resume-command wording to state
that only non-success headless exits print the exact resume command: revise
README.md lines 91-92, the English changelog summary in
site/src/changelog/Changelog.tsx lines 79-80, and the equivalent Chinese summary
at lines 587-588; no other behavior changes are needed.
| /// Restore the resumed conversation only up to this persisted message id. | ||
| #[arg(long = "resume-at", value_name = "MESSAGE_ID")] | ||
| resume_at: Option<String>, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files matching cli.rs =="
fd -a 'cli\.rs$' . || true
echo "== src/cli.rs outline =="
ast-grep outline src/cli.rs --view compact || true
echo "== relevant src/cli.rs ranges =="
wc -l src/cli.rs
sed -n '1,220p' src/cli.rs
sed -n '400,500p' src/cli.rs
echo "== search resume_at and ExecResumeArgs usage =="
rg -n "resume_at|ExecResumeArgs|ExecCommand::Resume|resume_args" src/cli.rs || true
echo "== git diff stat/name-only if available =="
git diff --stat || true
git diff --name-only || trueRepository: echoVic/orca-agent
Length of output: 11939
🌐 Web query:
clap subcommand argument parser outer positional/parent subcommand arg behavior
💡 Result:
In clap, the parser treats command lines as context-sensitive grammars [1]. When a subcommand is detected, the parser shifts focus to that subcommand's command definition, and all subsequent arguments are interpreted according to that subcommand's rules [2][1]. Positional arguments defined on an "outer" (parent) command behave according to their index and presence in the argument stream, but their interaction with subcommands is specific: 1. Parser Behavior: When the clap parser encounters a valid subcommand name, it stops parsing the parent command's arguments and begins parsing the subcommand [2]. If a positional argument for the parent command was expected but not yet provided, the parser may error depending on configuration [2]. 2. Subcommand Nesting: If you intend for a positional argument to be available to a subcommand, it must typically be defined within that subcommand's definition [3]. 3. Global Arguments: If you need a value to be accessible across the parent and its subcommands, you can mark the argument with global = true [4][5]. A global argument can be provided anywhere in the command line (e.g., before or after the subcommand), and its value will be propagated [6][5]. 4. Accessing Values: In the derive API, the recommended approach is to define your arguments within the relevant structs [7]. If an argument is shared across subcommands, you can define it as a global argument in the parent struct or create a shared "context" struct to hold common state [8][5]. For complex cases where positional arguments need to coexist with subcommands or be "passed through," consider settings like: - Arg::last: Forces an argument to be the final positional one, only accessible after the -- separator [4]. - Command::allow_external_subcommands: Allows the parser to capture any unknown arguments as a list, which can be useful for passing arguments to a child process [2][1]. For more information, see the official clap documentation on concepts and argument parsing [2][1].
Citations:
- 1: https://docs.rs/clap/latest/aarch64-apple-darwin/clap/_concepts/index.html
- 2: https://docs.rs/clap/latest/src/clap/_concepts.rs.html
- 3: Positional arguments order in clap 3.x clap-rs/clap#2260
- 4: https://docs.rs/clap/latest/clap/struct.Arg.html
- 5: https://stackoverflow.com/questions/75514455/how-to-parse-common-subcommand-arguments-with-clap-in-rust
- 6: https://docs.rs/clap/2/clap/struct.Arg.html
- 7: https://docs.rs/clap/latest/clap/_derive/_tutorial/chapter_2/index.html
- 8: Best practice to access arguments throughout an application clap-rs/clap#5258
🌐 Web query:
Clap v4 AllowExternalSubcommands subcommand argument before subcommand parent parser stops
💡 Result:
In clap v4, the parser does not support placing subcommand-specific arguments before the subcommand token [1][2]. When you use allow_external_subcommands(true), clap adopts a strict positional parsing strategy: once it encounters a token that it identifies as a subcommand, it stops parsing the parent command's arguments and begins parsing the remainder of the input according to the subcommand's definition (or treats them as external arguments) [1][3][4]. Clap's architecture is designed around scoped arguments, where flags and positional arguments are expected to follow the command they belong to [2]. Parsing ahead to determine if a token is a subcommand or a parent-level argument is intentionally avoided because it introduces significant complexity, ambiguity (e.g., if a flag takes a value that happens to be the same string as a subcommand name), and potential conflicts with positional argument support [2]. If you need to support passing arguments before a subcommand, consider these standard patterns: 1. Global Arguments: Use Arg::global(true) for arguments that should be recognized regardless of where they appear in the command line [2]. This is typically used for universal flags like --verbose or --version. 2. Argument Forwarding: If you are building a wrapper tool, use trailing_var_arg(true) and allow_hyphen_values(true) on a positional argument to capture all remaining input as a list of values, which you can then pass to the underlying command [5]. 3. Restructuring: Follow standard CLI conventions by scoping flags strictly to the command they modify. If a subcommand has specific arguments, they should be defined as part of that subcommand and placed after it on the command line [2]. There is no built-in "ambivalent" parsing mode in clap v4 that allows parent-level and subcommand-level arguments to be interleaved freely [2]. Attempts to work around this by manually managing external subcommands or ignoring errors are generally discouraged in favor of aligning with standard POSIX-style CLI structures [6][2].
Citations:
- 1: https://docs.rs/clap/latest/src/clap/_concepts.rs.html
- 2: Parse arguments before or after subcommands ambivalently clap-rs/clap#3056
- 3: https://docs.rs/clap/3.0.7/clap/enum.AppSettings.html
- 4: https://docs.rs/clap/4.1.8/clap/struct.Command.html
- 5: What's the canonical way to do "argument forwarding" (wrapping exec)? clap-rs/clap#5423
- 6: Document external subcommands clap-rs/clap#3887
🌐 Web query:
"clap" "[derive(Parser)]" subcommand "after subcommand" before subcommand argument
💡 Result:
In the clap derive API, subcommands are inherently context-sensitive, and clap generally enforces that subcommands appear after positional arguments defined at the same level [1]. It is not currently possible to define a CLI structure where a subcommand is required to appear before a positional argument that is defined in the same parent struct [1][2]. If you are attempting to design a CLI where certain flags or arguments should be available regardless of the subcommand's position, you have the following options: 1. Global Arguments: Use the #[arg(global = true)] attribute on your arguments [3]. This allows those arguments to be placed either before or after any subcommand (e.g., myapp --color subcommand) [3][2]. This is the standard way to define options that apply across the entire application [3]. 2. Move Arguments to Subcommands: If an argument is specific to a subcommand or must logically follow it, define that argument within the Subcommand enum variants themselves [4][3][2]. When using the derive API, the arguments for each subcommand are defined within the struct-variant or unit-variant of the subcommand enum [5][3]. 3. Default Subcommands: If you want to accept arguments without explicitly specifying a subcommand, you can make the subcommand field an Option and potentially use flatten to include default arguments [6][7]. However, clap will generally still treat the subcommand name as a distinct trigger [1]. Clap maintainers have noted that allowing subcommands to be positioned before required positional arguments in the same scope introduces significant complexity in parsing and can lead to confusing error messages [1]. If your specific use case requires a subcommand to precede other inputs, consider restructuring your CLI design to favor global flags or subcommand-specific arguments rather than attempting to force a specific order of positional arguments [1][2].
Citations:
- 1: Add a way to put subcommands first, before required positional arguments clap-rs/clap#6026
- 2: Positional arguments order in clap 3.x clap-rs/clap#2260
- 3: https://rust-lang-nursery.github.io/rust-cookbook/cli/clap-subcommand.html
- 4: https://www.reddit.com/r/rust/comments/yxxjte/clap_40_how_to_make_the_subcommand_selection_be/
- 5: https://docs.rs/clap/latest/clap/_derive/_tutorial/index.html
- 6: Built-in default subcommand support clap-rs/clap#4442
- 7: https://rs4ts.dev/18-cli-tools/02-subcommands/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if command -v git >/dev/null 2>&1; then
git rev-parse --show-toplevel >/dev/null
echo "== deps/version indicators =="
fd -a '^(Cargo\.toml|Cargo\.lock)$' . | sort | while read -r f; do
echo "-- $f"
rg -n 'clap|name = "clap"|\[dependencies\]|\[package\]' "$f" || true
done
echo "== cargo tree clap if local cargo and lockfile =="
cargo tree -e normal -i clap 2>/dev/null | head -80 || true
fi
echo "== lockfile clap snippet, if present =="
(fed Cargo.lock && rg -n 'name = "clap"|version = ' Cargo.lock | rg -A 1 'name = "clap"|version = ' || true)
echo "== test area around cli resume_at tests =="
sed -n '700,760p' src/cli.rsRepository: echoVic/orca-agent
Length of output: 9554
🌐 Web query:
Clap derive API global subcommand attributes global = true arguments
💡 Result:
In the clap derive API, the global = true attribute is used to allow an argument defined at a parent level to be available and usable within all child subcommands [1][2][3]. When you define an argument with #[arg(global = true)] in a parent struct, clap propagates that argument down, making it accessible to any subcommand [4][3]. Key technical points regarding global arguments in clap: 1. Definition: Use the attribute #[arg(global = true)] on the field within your top-level struct [1][3]. 2. Functionality: This ensures the argument can be passed before or after subcommands, and it will be captured correctly regardless of the active subcommand [4][3]. 3. Retrieving Values: When using the derive API, a common challenge is accessing this global value if the subcommand struct does not explicitly include it. To handle this, clap provides the #[arg(from_global)] attribute [1][2]. By adding a field to your subcommand struct with this attribute, clap will automatically populate that field with the value of the corresponding global argument defined in the parent [1][5]. Example pattern: #[derive(Parser)] struct Cli { #[arg(long, global = true)] global_option: bool, #[command(subcommand)] command: Commands, } #[derive(Subcommand)] enum Commands { Sub(SubArgs), } #[derive(Args)] struct SubArgs { #[arg(from_global)] global_option: bool, } This mechanism avoids the need to pass the parent object manually to subcommands [5]. Note that global arguments must typically be defined at the level they are intended to propagate from (usually the top level) [4]. If nested subcommands are involved, they inherit from their immediate parent or the top level depending on the configuration, though direct propagation to all levels is intended [6].
Citations:
- 1: https://docs.rs/clap/latest/clap/_derive/
- 2: https://github.com/clap-rs/clap/blob/master/src/_derive/mod.rs
- 3: https://stackoverflow.com/questions/75514455/how-to-parse-common-subcommand-arguments-with-clap-in-rust
- 4: https://docs.rs/clap/latest/clap/struct.Arg.html
- 5: feat(derive): Add support from inheriting global options in subcommands clap-rs/clap#2026
- 6: [Clap v3] Nested sub-commands don't propagate
globalarguments clap-rs/clap#2053
🌐 Web query:
clap_derive Arg from_global global subcommand before subcommand
💡 Result:
In clap's derive API, the global argument system is managed through two primary attributes: global = true (to define) and from_global (to consume) [1][2]. 1. global = true: Use this on an argument field in your top-level struct to specify that the argument should be available to all child subcommands [3][4]. Although defined at the top level, clap allows users to supply the argument anywhere (parent or subcommands) [3]. 2. from_global: Use this attribute on a field within a subcommand to explicitly read a value from an argument that was marked as global at a higher level, regardless of the current subcommand's position in the hierarchy [1][2]. In practice, this mechanism allows you to define an argument once at the root level—avoiding code duplication across multiple subcommands—while still allowing individual subcommands to access that value [3][4]. Because from_global is a raw attribute, it is applied as: #[arg(from_global)] my_field: Option, Note that from_global effectively instructs clap to look up the global argument's value that was matched elsewhere in the command-line arguments [1][5]. It is commonly used when you need to specifically bind a global argument to a field within a subcommand's struct [6]. If you simply need the argument available for help/parsing but do not need to access it inside the subcommand's struct logic, defining it as global = true at the root is often sufficient [3][4].
Citations:
- 1: https://docs.rs/clap/latest/clap/_derive/
- 2: https://github.com/clap-rs/clap/blob/master/src/_derive/mod.rs
- 3: https://docs.rs/clap/latest/clap/struct.Arg.html
- 4: https://stackoverflow.com/questions/75514455/how-to-parse-common-subcommand-arguments-with-clap-in-rust
- 5: https://docs.rs/clap/latest/clap/_derive/index.html
- 6:
#[arg(from_global)]with#[command(flatten)]clap-rs/clap#5525
Use from_global for the parent --resume-at value in exec resume.
--resume-at appears before resume, so ExecArgs::resume_at receives it while ExecCommand::Resume still builds the request from ExecResumeArgs::resume_at. Mark the parent argument global and fill the subcommand field with #[arg(from_global)], or merge the duplicate fields and reject both values being set.
🤖 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/cli.rs` around lines 117 - 119, Update the resume-at argument wiring so
the parent ExecArgs::resume_at value is available to the resume subcommand: mark
the parent argument global and configure ExecResumeArgs::resume_at with
#[arg(from_global)]. Ensure exec resume builds its request from this shared
value without allowing conflicting duplicate values.
| #[test] | ||
| fn exec_resume_at_rejects_unknown_boundary() { | ||
| let home = TempDir::new().expect("temp home"); | ||
|
|
||
| let first = Command::new(env!("CARGO_BIN_EXE_orca")) | ||
| .env("ORCA_HOME", home.path()) | ||
| .args(["exec", "--provider", "mock", "first prompt"]) | ||
| .output() | ||
| .expect("run first orca"); | ||
| assert_eq!(first.status.code(), Some(0)); | ||
| let session_id = session_id_from_home(home.path()); | ||
|
|
||
| let resumed = Command::new(env!("CARGO_BIN_EXE_orca")) | ||
| .env("ORCA_HOME", home.path()) | ||
| .args([ | ||
| "exec", | ||
| "--output-format", | ||
| "jsonl", | ||
| "--provider", | ||
| "mock", | ||
| "resume", | ||
| &session_id, | ||
| "--resume-at", | ||
| "item_00000000-0000-0000-0000-000000000000", | ||
| "mock_history_echo", | ||
| ]) | ||
| .output() | ||
| .expect("resume at unknown boundary"); | ||
|
|
||
| assert_eq!(resumed.status.code(), Some(1)); | ||
| let stderr = String::from_utf8_lossy(&resumed.stderr); | ||
| assert!( | ||
| stderr.contains("no saved message matches"), | ||
| "unknown boundary must fail closed: {stderr}" | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that boundary rejection occurs before provider execution.
The test verifies the error status and message. It does not verify the fail-closed ordering. Parse resumed.stdout and assert that mock_history_echo produced no assistant.message.delta event.
Proposed test addition
assert_eq!(resumed.status.code(), Some(1));
+ let events = parse_jsonl(&resumed.stdout);
+ assert!(
+ !events
+ .iter()
+ .any(|event| event["type"] == "assistant.message.delta"),
+ "the provider must not run for an unknown boundary"
+ );
let stderr = String::from_utf8_lossy(&resumed.stderr);📝 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 exec_resume_at_rejects_unknown_boundary() { | |
| let home = TempDir::new().expect("temp home"); | |
| let first = Command::new(env!("CARGO_BIN_EXE_orca")) | |
| .env("ORCA_HOME", home.path()) | |
| .args(["exec", "--provider", "mock", "first prompt"]) | |
| .output() | |
| .expect("run first orca"); | |
| assert_eq!(first.status.code(), Some(0)); | |
| let session_id = session_id_from_home(home.path()); | |
| let resumed = Command::new(env!("CARGO_BIN_EXE_orca")) | |
| .env("ORCA_HOME", home.path()) | |
| .args([ | |
| "exec", | |
| "--output-format", | |
| "jsonl", | |
| "--provider", | |
| "mock", | |
| "resume", | |
| &session_id, | |
| "--resume-at", | |
| "item_00000000-0000-0000-0000-000000000000", | |
| "mock_history_echo", | |
| ]) | |
| .output() | |
| .expect("resume at unknown boundary"); | |
| assert_eq!(resumed.status.code(), Some(1)); | |
| let stderr = String::from_utf8_lossy(&resumed.stderr); | |
| assert!( | |
| stderr.contains("no saved message matches"), | |
| "unknown boundary must fail closed: {stderr}" | |
| ); | |
| } | |
| #[test] | |
| fn exec_resume_at_rejects_unknown_boundary() { | |
| let home = TempDir::new().expect("temp home"); | |
| let first = Command::new(env!("CARGO_BIN_EXE_orca")) | |
| .env("ORCA_HOME", home.path()) | |
| .args(["exec", "--provider", "mock", "first prompt"]) | |
| .output() | |
| .expect("run first orca"); | |
| assert_eq!(first.status.code(), Some(0)); | |
| let session_id = session_id_from_home(home.path()); | |
| let resumed = Command::new(env!("CARGO_BIN_EXE_orca")) | |
| .env("ORCA_HOME", home.path()) | |
| .args([ | |
| "exec", | |
| "--output-format", | |
| "jsonl", | |
| "--provider", | |
| "mock", | |
| "resume", | |
| &session_id, | |
| "--resume-at", | |
| "item_00000000-0000-0000-0000-000000000000", | |
| "mock_history_echo", | |
| ]) | |
| .output() | |
| .expect("resume at unknown boundary"); | |
| assert_eq!(resumed.status.code(), Some(1)); | |
| let events = parse_jsonl(&resumed.stdout); | |
| assert!( | |
| !events | |
| .iter() | |
| .any(|event| event["type"] == "assistant.message.delta"), | |
| "the provider must not run for an unknown boundary" | |
| ); | |
| let stderr = String::from_utf8_lossy(&resumed.stderr); | |
| assert!( | |
| stderr.contains("no saved message matches"), | |
| "unknown boundary must fail closed: {stderr}" | |
| ); | |
| } |
🤖 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/history_contract.rs` around lines 753 - 788, Extend the test
exec_resume_at_rejects_unknown_boundary to parse resumed.stdout as JSONL and
assert it contains no assistant.message.delta event with mock_history_echo
output, confirming rejection occurs before provider execution while preserving
the existing status and stderr assertions.
The text-mode budget-exhaustion hint test used the 128-turn mock trajectory, which exceeded the CI slow-test timeout on Windows debug runners. Switch it to the one-turn cost-budget fixture (same budget_exhausted terminal and resume hint, with the max-inner-turns path still covered by the trajectory contract).
1 participant
Prepares Orca v0.3.13: headless resume as a first-class CLI capability.
orca exec resume <SESSION_ID> [PROMPT].../resume --lastwith global run options; conflicts with--resume/--fork/--continuerejectedsession.completedcarries the durablesession_id; text-mode exits print the exact resume command--resume-at <MESSAGE_ID>message-boundary restore (fail-closed on unknown boundaries)session.checkpointtyped record on budget exhaustion; resume owns a fresh budget scopeVerification: workspace tests, clippy 0 errors, fmt, contract validators, version sync, site build + SEO.
Summary by CodeRabbit
New Features
Documentation
Release