Skip to content

Commit 43f5213

Browse files
schicklingclaude
andcommitted
fix(pi): settle-edge idle, terminal-record precedence, and runtime-id sessions
Review-pass fixes: idle emits on agent_settled instead of agent_end — measured against the repo's own pi captures, ctx.isIdle() is still false through agent_end and a queued follow-up turn starts exactly there, so the old edge blipped a spurious idle before it; the channel drops queued live frames once the wrapper's terminal record is on disk (the flock serializes but does not order two processes); and ptySession records the wrapper's runtime ID, delivered to the channel via ST2_PI_CHANNEL_RUNTIME_ID beside the existing three env vars. checks.pi-extension-types stays green against pinned pi, proving the settle event exists on that surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c341ebe commit 43f5213

3 files changed

Lines changed: 71 additions & 11 deletions

File tree

hooks/pi-channel.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,11 @@ export default function (pi: ExtensionAPI) {
197197
});
198198
};
199199

200-
// Observed harness state, extension side. pi's own turn boundaries are the positive signal:
201-
// `ctx.isIdle()` is false for exactly the `agent_start`..`agent_end` span, so these two events
202-
// carry the working/idle edge without inspecting anything. The frame is observational — st2
200+
// Observed harness state, extension side. pi's own turn boundaries are the positive signal,
201+
// and the idle edge is `agent_settled`, not `agent_end`: measured against the repo's own pi
202+
// captures, `ctx.isIdle()` is still false through `agent_end`, and a queued follow-up turn
203+
// starts exactly at that boundary — an `agent_end` emit would blip a spurious idle before it.
204+
// `agent_settled` is the first point pi is provably idle. The frame is observational — st2
203205
// decides what becomes of it — and a closed channel drops it silently, matching the fail-open
204206
// rule this file already follows. pi 0.84.2 exposes no typed waiting-on-a-human event, so no
205207
// frame here ever claims one.
@@ -209,7 +211,7 @@ export default function (pi: ExtensionAPI) {
209211
child.stdin.write(JSON.stringify({ type: "state", state: word }) + "\n");
210212
};
211213
pi.on("agent_start", async () => sendState("active"));
212-
pi.on("agent_end", async () => sendState("idle"));
214+
pi.on("agent_settled", async () => sendState("idle"));
213215

214216
pi.on("session_start", async (_event, ctx) => {
215217
// Awaited before the session's first turn, which is what makes restored context reach the boot

src/pi_channel.rs

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,13 @@ pub fn run(catalog_root: &Path, identity: &str) -> Result<()> {
9292
// pi's own turn events, and its stdio connection to the extension is the evidence that those
9393
// events are still being watched. The terminal half belongs to the outer session wrapper,
9494
// which alone sees the provider die.
95-
let mut writer =
96-
harness_state::Writer::new(&agent_dir, identity, "pi", Some(identity.to_string()));
95+
// The pty session vouching for the record is the wrapper's task: its runtime ID arrives in
96+
// the channel environment, and only aliases the identity on driver-expanded seats.
97+
let pty_session = std::env::var(crate::pi_session::CHANNEL_RUNTIME_ID)
98+
.ok()
99+
.filter(|value| !value.is_empty())
100+
.unwrap_or_else(|| identity.to_string());
101+
let mut writer = harness_state::Writer::new(&agent_dir, identity, "pi", Some(pty_session));
97102
channel_loop(
98103
&input_rx,
99104
&mut stdout,
@@ -135,7 +140,10 @@ fn channel_loop(
135140
.ok()
136141
.as_ref()
137142
.and_then(state_observation)
138-
&& let Err(error) = writer.observe(observation)
143+
// A queued live frame must never overwrite the wrapper's terminal record:
144+
// the channel and the wrapper are separate processes, so the flock alone
145+
// serializes but does not order their writes.
146+
&& let Err(error) = writer.observe_unless_ended(observation)
139147
{
140148
eprintln!("st2 pi channel: recording observed state failed: {error}");
141149
}
@@ -312,6 +320,40 @@ mod tests {
312320
);
313321
}
314322

323+
/// The wrapper's terminal record is the incarnation's last word: a live frame the extension
324+
/// queued before dying must not resurrect the session after the wrapper reaped it.
325+
#[test]
326+
fn a_queued_live_frame_never_overwrites_the_wrappers_terminal_record() {
327+
let tmp = tempfile::tempdir().unwrap();
328+
let agent_dir = tmp.path();
329+
std::fs::create_dir_all(message::inbox_dir(agent_dir)).unwrap();
330+
let record = harness_state::harness_state_path(agent_dir);
331+
let mut channel_writer =
332+
harness_state::Writer::new(agent_dir, "h.worker", "pi", Some("h.worker".into()));
333+
let mut wrapper_writer =
334+
harness_state::Writer::new(agent_dir, "h.worker", "pi", Some("h.worker".into()));
335+
wrapper_writer.ended("signal 9").unwrap();
336+
let terminal = std::fs::read(&record).unwrap();
337+
338+
let (tx, rx) = mpsc::channel();
339+
tx.send(Ok(r#"{"type":"state","state":"idle"}"#.to_string()))
340+
.unwrap();
341+
drop(tx);
342+
let mut out = Vec::new();
343+
channel_loop(
344+
&rx,
345+
&mut out,
346+
&message::inbox_dir(agent_dir),
347+
&mut channel_writer,
348+
"h.worker",
349+
Duration::from_millis(2),
350+
Duration::from_millis(5),
351+
)
352+
.unwrap();
353+
354+
assert_eq!(std::fs::read(&record).unwrap(), terminal);
355+
}
356+
315357
#[test]
316358
fn channel_content_reuses_the_claude_channel_envelope() {
317359
assert_eq!(

src/pi_session.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ pub const CHANNEL_BIN: &str = "ST2_PI_CHANNEL_BIN";
3131
pub const CHANNEL_CATALOG: &str = "ST2_PI_CHANNEL_CATALOG";
3232
/// The host-qualified bus identity the channel binds.
3333
pub const CHANNEL_IDENTITY: &str = "ST2_PI_CHANNEL_IDENTITY";
34+
/// The wrapper's runtime/task ID — the pty session whose liveness vouches for observed state.
35+
pub const CHANNEL_RUNTIME_ID: &str = "ST2_PI_CHANNEL_RUNTIME_ID";
3436

3537
/// pi's startup network work, which a supervised seat should not be doing.
3638
///
@@ -55,7 +57,7 @@ pub fn run(
5557
);
5658
let executable =
5759
std::env::current_exe().context("resolving st2 executable for the pi channel")?;
58-
let mut env = channel_env(&executable, catalog_root, &identity)?;
60+
let mut env = channel_env(&executable, catalog_root, &identity, &runtime_id)?;
5961
env.extend(offline_defaults(|key| std::env::var_os(key).is_some()));
6062
let set = hooks::verify_required_set().with_context(|| {
6163
format!(
@@ -74,7 +76,7 @@ pub fn run(
7476
&STOP,
7577
)
7678
.with_context(|| format!("running pi driver '{runtime_id}'"))?;
77-
record_session_end(&agent_dir, &identity, &outcome);
79+
record_session_end(&agent_dir, &identity, &runtime_id, &outcome);
7880
match outcome {
7981
ProviderOutcome::Exited(exit) => {
8082
anyhow::ensure!(exit.success(), "pi provider exited with {exit}");
@@ -90,13 +92,18 @@ pub fn run(
9092
/// one fact it records. The `Writer` is constructed at the terminal edge on purpose: it re-reads
9193
/// whatever the channel last wrote and continues its transition counter, and by the time the
9294
/// wrapper has reaped pi the extension (and with it the channel) is already gone.
93-
fn record_session_end(agent_dir: &Path, identity: &str, outcome: &ProviderOutcome) {
95+
fn record_session_end(
96+
agent_dir: &Path,
97+
identity: &str,
98+
runtime_id: &str,
99+
outcome: &ProviderOutcome,
100+
) {
94101
let label = match outcome {
95102
ProviderOutcome::Exited(exit) | ProviderOutcome::Stopped(Some(exit)) => exit_label(*exit),
96103
ProviderOutcome::Stopped(None) => "stopped".to_string(),
97104
};
98105
let mut writer =
99-
harness_state::Writer::new(agent_dir, identity, "pi", Some(identity.to_string()));
106+
harness_state::Writer::new(agent_dir, identity, "pi", Some(runtime_id.to_string()));
100107
if let Err(error) = writer.ended(label) {
101108
eprintln!("st2 pi driver: recording session end failed: {error}");
102109
}
@@ -141,6 +148,7 @@ fn channel_env(
141148
executable: &Path,
142149
catalog_root: &Path,
143150
identity: &str,
151+
runtime_id: &str,
144152
) -> Result<Vec<(String, String)>> {
145153
let executable = executable
146154
.to_str()
@@ -150,6 +158,7 @@ fn channel_env(
150158
(CHANNEL_BIN.to_string(), executable.to_string()),
151159
(CHANNEL_CATALOG.to_string(), catalog_root.to_string()),
152160
(CHANNEL_IDENTITY.to_string(), identity.to_string()),
161+
(CHANNEL_RUNTIME_ID.to_string(), runtime_id.to_string()),
153162
])
154163
}
155164

@@ -213,6 +222,7 @@ mod tests {
213222
record_session_end(
214223
agent_dir,
215224
"h.worker",
225+
"h.worker",
216226
&ProviderOutcome::Exited(ExitStatus::from_raw(3 << 8)),
217227
);
218228

@@ -229,6 +239,7 @@ mod tests {
229239
record_session_end(
230240
agent_dir,
231241
"h.worker",
242+
"h.worker",
232243
&ProviderOutcome::Stopped(Some(ExitStatus::from_raw(9))),
233244
);
234245
let observed = crate::harness_state::read(&record, None).unwrap();
@@ -309,6 +320,7 @@ mod tests {
309320
&PathBuf::from("/opt/st2/bin/st2"),
310321
&PathBuf::from("/catalog"),
311322
"host.worker",
323+
"host.worker-task",
312324
)
313325
.unwrap();
314326

@@ -318,6 +330,10 @@ mod tests {
318330
(CHANNEL_BIN.to_string(), "/opt/st2/bin/st2".to_string()),
319331
(CHANNEL_CATALOG.to_string(), "/catalog".to_string()),
320332
(CHANNEL_IDENTITY.to_string(), "host.worker".to_string()),
333+
(
334+
CHANNEL_RUNTIME_ID.to_string(),
335+
"host.worker-task".to_string()
336+
),
321337
]
322338
);
323339
}

0 commit comments

Comments
 (0)