-
Notifications
You must be signed in to change notification settings - Fork 1
feat(session): configurable daemon wait timeout + retry hint #536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bac37c8
122f833
b759891
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -43,10 +43,11 @@ fn read_daemon_pid(session_dir: &std::path::Path) -> Option<u32> { | |||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||
| /// Exits 0 when result.toml appears (streams stdout.log), exits 124 on timeout, | ||||||||||||||||||||||||||||||||||||
| /// exits 1 if the daemon process died without producing a result. | ||||||||||||||||||||||||||||||||||||
| /// Hardcoded wait timeout in seconds. | ||||||||||||||||||||||||||||||||||||
| const WAIT_TIMEOUT_SECS: u64 = 250; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| pub(crate) fn handle_session_wait(session: String, cd: Option<String>) -> Result<i32> { | ||||||||||||||||||||||||||||||||||||
| pub(crate) fn handle_session_wait( | ||||||||||||||||||||||||||||||||||||
| session: String, | ||||||||||||||||||||||||||||||||||||
| cd: Option<String>, | ||||||||||||||||||||||||||||||||||||
| wait_timeout_secs: u64, | ||||||||||||||||||||||||||||||||||||
| ) -> Result<i32> { | ||||||||||||||||||||||||||||||||||||
| let project_root = crate::pipeline::determine_project_root(cd.as_deref())?; | ||||||||||||||||||||||||||||||||||||
| let resolved = resolve_session_prefix_with_fallback(&project_root, &session)?; | ||||||||||||||||||||||||||||||||||||
| let session_dir = get_session_dir(&project_root, &resolved.session_id)?; | ||||||||||||||||||||||||||||||||||||
|
|
@@ -84,10 +85,20 @@ pub(crate) fn handle_session_wait(session: String, cd: Option<String>) -> Result | |||||||||||||||||||||||||||||||||||
| return Ok(1); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if start.elapsed().as_secs() >= WAIT_TIMEOUT_SECS { | ||||||||||||||||||||||||||||||||||||
| let elapsed = start.elapsed().as_secs(); | ||||||||||||||||||||||||||||||||||||
| if elapsed >= wait_timeout_secs { | ||||||||||||||||||||||||||||||||||||
| eprintln!( | ||||||||||||||||||||||||||||||||||||
| "Timeout: session {} did not complete within {}s", | ||||||||||||||||||||||||||||||||||||
| resolved.session_id, WAIT_TIMEOUT_SECS | ||||||||||||||||||||||||||||||||||||
| resolved.session_id, wait_timeout_secs, | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| // Emit structured retry hint for orchestrators / agents. | ||||||||||||||||||||||||||||||||||||
| let cd_arg = cd | ||||||||||||||||||||||||||||||||||||
| .as_ref() | ||||||||||||||||||||||||||||||||||||
| .map(|path| format!(" --cd '{}'", path)) | ||||||||||||||||||||||||||||||||||||
| .unwrap_or_default(); | ||||||||||||||||||||||||||||||||||||
| eprintln!( | ||||||||||||||||||||||||||||||||||||
| "<!-- CSA:SESSION_WAIT_TIMEOUT session={} elapsed={}s cmd=\"csa session wait --session {}{}\" -->", | ||||||||||||||||||||||||||||||||||||
| resolved.session_id, elapsed, resolved.session_id, cd_arg, | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The structured retry hint contains nested double quotes in the Additionally, using single quotes for the path in the command hint is generally safer and avoids conflict with the outer double quotes of the attribute.
Suggested change
Comment on lines
+95
to
102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The structured retry hint does not escape the It is recommended to escape single quotes for the shell command and XML-sensitive characters for the attribute value, and to quote all attribute values for better machine readability. let cd_arg = cd
.as_ref()
.map(|path| format!(" --cd '{}'", path.replace('\'', "'\\''")))
.unwrap_or_default();
let cmd = format!("csa session wait --session {}{}", resolved.session_id, cd_arg);
eprintln!(
"<!-- CSA:SESSION_WAIT_TIMEOUT session=\"{}\" elapsed=\"{}s\" cmd=\"{}\" -->",
resolved.session_id,
elapsed,
cmd.replace('&', "&").replace('<', "<").replace('>', ">").replace('"', """)
); |
||||||||||||||||||||||||||||||||||||
| return Ok(124); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -51,6 +51,13 @@ pub struct SessionConfig { | |||||||||||||||||||||
| /// Only effective when `tool_output_compression` is enabled. | ||||||||||||||||||||||
| #[serde(default = "default_tool_output_threshold_bytes")] | ||||||||||||||||||||||
| pub tool_output_threshold_bytes: u64, | ||||||||||||||||||||||
| /// Timeout (seconds) for `csa session wait` polling loop. | ||||||||||||||||||||||
| /// | ||||||||||||||||||||||
| /// The default of 250s is intentional: it lets the daemon's KV cache stay | ||||||||||||||||||||||
| /// warm while periodically returning control to the calling orchestrator. | ||||||||||||||||||||||
| /// The caller is expected to re-invoke `csa session wait` in a loop. | ||||||||||||||||||||||
| #[serde(default = "default_daemon_wait_seconds")] | ||||||||||||||||||||||
| pub daemon_wait_seconds: u64, | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| fn default_seed_max_age_secs() -> u64 { | ||||||||||||||||||||||
|
|
@@ -65,6 +72,13 @@ fn default_tool_output_threshold_bytes() -> u64 { | |||||||||||||||||||||
| 8192 | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /// Default daemon wait timeout: 250s for KV cache warmth. | ||||||||||||||||||||||
| pub const DEFAULT_DAEMON_WAIT_SECS: u64 = 250; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| fn default_daemon_wait_seconds() -> u64 { | ||||||||||||||||||||||
| DEFAULT_DAEMON_WAIT_SECS | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+75
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default timeout value of
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const DEFAULT_SPOOL_MAX_MB: u32 = 32; | ||||||||||||||||||||||
| const DEFAULT_SPOOL_KEEP_ROTATED: bool = true; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -82,6 +96,7 @@ impl Default for SessionConfig { | |||||||||||||||||||||
| spool_keep_rotated: None, | ||||||||||||||||||||||
| tool_output_compression: false, | ||||||||||||||||||||||
| tool_output_threshold_bytes: default_tool_output_threshold_bytes(), | ||||||||||||||||||||||
| daemon_wait_seconds: default_daemon_wait_seconds(), | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -99,6 +114,7 @@ impl SessionConfig { | |||||||||||||||||||||
| && self.spool_keep_rotated.is_none() | ||||||||||||||||||||||
| && !self.tool_output_compression | ||||||||||||||||||||||
| && self.tool_output_threshold_bytes == default_tool_output_threshold_bytes() | ||||||||||||||||||||||
| && self.daemon_wait_seconds == default_daemon_wait_seconds() | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| pub fn resolved_spool_max_mb(&self) -> u32 { | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structured retry hint does not include the
--cdargument if it was provided to the original command. If an orchestrator or agent relies on this hint to re-invoke the command from a different working directory, the retry might fail because it won't be able to locate the session or project root.