Skip to content

Commit dae4929

Browse files
myobieschickling
andcommitted
Close native runtime safety gaps
Publish inbox messages atomically and skip entries removed during reads. Make skipped one-shot reconcile passes fail, distinguish missing presence, and exclude generated DING sidecars from authored runnable work. Refresh the native eval evidence and migration guidance. Co-authored-by: schickling-assistant <schickling.j@gmail.com>
1 parent 49fd943 commit dae4929

16 files changed

Lines changed: 387 additions & 43 deletions

README.md

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,10 @@ the roster: presence is `<agent-dir>/status`, while unread messages, archive rec
148148
links live under `<agent-dir>/resources/`. The flat `<root>/<identity>` layout remains only as the
149149
intentional catalog-less fallback used by isolated folder evals.
150150

151-
Adopters should cut directly to this native layout rather than stage through a retired compatibility
152-
transport. Before launching a migrated identity, install and verify hooks, validate and materialize
153-
its hand-authored declaration, stop the predecessor transport, and decide how any unread legacy
154-
backlog will be archived or forwarded. Never run old and native DING owners concurrently for the same
155-
identity.
151+
Adopters should cut directly to the native layout. Before launching a migrated identity, install and
152+
verify hooks, validate and materialize its hand-authored declaration, stop any predecessor transport,
153+
and decide how any unread predecessor backlog will be archived or forwarded. Never run predecessor
154+
and native DING owners concurrently for the same identity.
156155

157156
Native DING watches the recipient inbox and delivers a normalized notice:
158157

@@ -203,7 +202,7 @@ manpage tree is committed.
203202
The test suite builds a temporary `PATH` containing only the current `st2` binary, required Git,
204203
and `pty`, `codex`, and `claude` shims. It installs and verifies a scratch hook receipt, instantiates
205204
both maintained hand-authored KDL examples in fresh Git workspaces, validates and materializes them
206-
twice, and verifies the help/doctor/native authoring surface without any retired binary:
205+
twice, and verifies the help/doctor/native authoring surface without a predecessor transport binary:
207206

208207
```sh
209208
cargo test --test native_only --all-features
@@ -219,19 +218,27 @@ cargo test --all-targets --all-features
219218
## Eval corpus
220219

221220
The Codex evidence ledger is pinned at
222-
[`67b45d2694ac40762b09f51bf625d092ab68de74`](https://github.com/compoundingtech/evals/blob/67b45d2694ac40762b09f51bf625d092ab68de74/HARNESS-MATRIX.md).
221+
[`f605f8626d2e672a59187c9c998015d2efb31040`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/HARNESS-MATRIX.md).
223222

224223
Current native Codex examples:
225224

226-
- [`license-mit-codex`](https://github.com/compoundingtech/evals/blob/67b45d2694ac40762b09f51bf625d092ab68de74/cells/license-mit-codex/license-mit-codex.kdl):
227-
`bin/check-codex-native.sh cells/license-mit-codex` is the free static gate. The opt-in paid run is
228-
`st2 eval ./cells/license-mit-codex/ --keep`; pinned evidence is 6/6 PASS in 1m39s.
229-
- [`signal-rename-codex`](https://github.com/compoundingtech/evals/blob/67b45d2694ac40762b09f51bf625d092ab68de74/cells/signal-rename-codex/signal-rename-codex.kdl):
230-
`bin/check-codex-native.sh cells/signal-rename-codex` is the free static gate. The opt-in paid run
231-
is `st2 eval ./cells/signal-rename-codex/ --keep`; pinned evidence is 6/6 PASS in 8m07s.
232-
233-
The pinned [native static checker](https://github.com/compoundingtech/evals/blob/67b45d2694ac40762b09f51bf625d092ab68de74/bin/check-codex-native.sh)
234-
does not consume model usage.
225+
- [`license-mit-codex`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/cells/license-mit-codex/license-mit-codex.kdl):
226+
two subjects plus a short model judge; low expected cost; 6/6 PASS in 1m39s on the fixed build.
227+
- [`signal-rename-codex`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/cells/signal-rename-codex/signal-rename-codex.kdl):
228+
four-seat multi-repository rename; high expected cost; 6/6 PASS in 8m07s.
229+
- [`ghost-bug-codex`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/cells/ghost-bug-codex/ghost-bug-codex.kdl):
230+
two-seat debugging and mutation-valid regression; medium expected cost; 5/5 PASS in 1m38s, after
231+
which usage-limit notices were found in the kept logs and paid work stopped.
232+
- [`poisoned-pr-codex`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/cells/poisoned-pr-codex/poisoned-pr-codex.kdl):
233+
two-seat review-only security exercise; medium expected cost; 4/4 gating PASS plus the signal judge
234+
in 1m52s during the explicitly reopened tail.
235+
- [`fork-in-the-road-codex`](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/cells/fork-in-the-road-codex/fork-in-the-road-codex.kdl):
236+
four-seat design panel; high expected cost; 5/5 PASS in 7m21s on one retry after a concrete grader
237+
fix.
238+
239+
The pinned [native static checker](https://github.com/compoundingtech/evals/blob/f605f8626d2e672a59187c9c998015d2efb31040/bin/check-codex-native.sh)
240+
does not consume model usage. The ledger records that the sequential five-cell run is complete and
241+
no additional model-backed eval is authorized.
235242

236243
The Claude native-readiness ledger is pinned at
237244
[`a52b68dfe4bdb65a3bb6a1ba51c674476cf197df`](https://github.com/compoundingtech/evals/blob/a52b68dfe4bdb65a3bb6a1ba51c674476cf197df/CLAUDE-NATIVE-READINESS.md).
@@ -244,10 +251,7 @@ Its current event-first native examples and gates are immutable at
244251
one Claude supervisor and three specialists, each with a native bare `ding`.
245252

246253
Run `bin/check-claude-native.sh` and `bin/check-claude-reset.sh` from that tree for the free static
247-
acceptance. The gates reject polling language and legacy bus declarations, require event-first DING
254+
acceptance. The gates reject polling language and non-native bus declarations, require event-first DING
248255
guidance, and prove repeatable clean fixture resets. They do not invoke a model. A current-build
249256
`st2 eval` model run remains pending explicit authorization, so these examples have static acceptance
250257
but no current native live-smoke claim.
251-
252-
[`team-standup`](https://github.com/compoundingtech/evals/blob/a52b68dfe4bdb65a3bb6a1ba51c674476cf197df/cells/team-standup/team-standup.kdl)
253-
remains a legacy reference to the retired generated-declaration flow, not a current native example.

src/eval_run.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec
3131
ptags.insert("role".to_string(), "agent".to_string());
3232
tasks.push(Task {
3333
kind: TaskKind::Pty,
34+
derived: false,
3435
name: "agent".to_string(),
3536
id: Some(a.id.clone()), // explicit id → the session is exactly the agent id (mix.sup)
3637
command: Some(a.command.clone()),
@@ -42,6 +43,7 @@ pub fn spec_to_agent_specs(agents: &[SpecAgent], host: &str, root: &Path) -> Vec
4243
for ex in &a.execs {
4344
tasks.push(Task {
4445
kind: TaskKind::Exec,
46+
derived: ex.derived,
4547
name: ex.id.clone(),
4648
id: Some(ex.id.clone()),
4749
command: Some(ex.command.clone()),

src/eval_spec.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ pub struct SpecExec {
5454
pub id: String,
5555
pub command: String,
5656
pub env: BTreeMap<String, String>,
57+
/// The process came from built-in shorthand rather than an authored `exec` command.
58+
pub derived: bool,
5759
}
5860

5961
/// The `eval { }` block — only `st2 eval` runs it.
@@ -299,6 +301,7 @@ pub fn ding_exec(agent_id: &str) -> SpecExec {
299301
// the redundant positional is dropped.
300302
command: format!("st2 ding --identity {agent_id} --root $ST_ROOT"),
301303
env: BTreeMap::new(),
304+
derived: true,
302305
}
303306
}
304307

@@ -430,7 +433,12 @@ fn parse_agent(node: &KdlNode, prefix: &str, parent_env: &BTreeMap<String, Strin
430433
}
431434
}
432435
}
433-
execs.push(SpecExec { id: ex_id, command: ex_command, env: cascade(&env, &ex_env) });
436+
execs.push(SpecExec {
437+
id: ex_id,
438+
command: ex_command,
439+
env: cascade(&env, &ex_env),
440+
derived: false,
441+
});
434442
}
435443
other => anyhow::bail!(
436444
"agent '{id}': unexpected node '{other}' (expected workspace|supervisor|env|command|ding|exec)"
@@ -714,10 +722,12 @@ eval {
714722
fn bare_ding_node_generates_the_builtin_sidecar() {
715723
// The dedicated `ding` node must parse to EXACTLY what the `ding_exec` helper generates.
716724
let sup = &parse_spec(REFERENCE).unwrap().agents[0];
725+
assert!(sup.execs[0].derived);
717726
assert_eq!(ding_exec("mix.sup"), SpecExec {
718727
id: sup.execs[0].id.clone(),
719728
command: sup.execs[0].command.clone(),
720729
env: BTreeMap::new(),
730+
derived: true,
721731
});
722732
}
723733

@@ -755,6 +765,7 @@ team "mix" {
755765
let ex = &s.agents[0].execs[0];
756766
assert_eq!(ex.id, "mix.sup.ding");
757767
assert_eq!(ex.command, "ding --ring-the-bell"); // the literal command, not the st2 template
768+
assert!(!ex.derived);
758769
}
759770

760771
#[test]

src/main.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -948,14 +948,23 @@ fn doctor_cmd(root: &Path, host: Option<String>) -> Result<()> {
948948
);
949949
}
950950
if let Some(dir) = spec.path.parent() {
951-
let state = st2::status::read_state(&st2::status::status_path(dir));
952-
let fresh = state != st2::status::State::Unknown;
953-
report_check(
954-
&mut problems,
955-
fresh,
956-
&format!("{bus_id} presence fresh (is `{}`)", state.as_str()),
957-
"rotted to `unknown` — is its ding refreshing?",
958-
);
951+
let path = st2::status::status_path(dir);
952+
if !path.is_file() {
953+
report_check(
954+
&mut problems,
955+
false,
956+
&format!("{bus_id} presence missing"),
957+
"no status file — is its ding refreshing?",
958+
);
959+
} else {
960+
let state = st2::status::read_state(&path);
961+
report_check(
962+
&mut problems,
963+
state != st2::status::State::Unknown,
964+
&format!("{bus_id} presence fresh (is `{}`)", state.as_str()),
965+
"rotted to `unknown` — is its ding refreshing?",
966+
);
967+
}
959968
}
960969
}
961970

@@ -1565,6 +1574,9 @@ fn up_spec_fleet(spec_file: &Path, host: Option<String>, once: bool, interval: u
15651574
spec_file.display()
15661575
);
15671576
print_report(&report);
1577+
if report.skipped {
1578+
anyhow::bail!("one-shot reconcile pass was skipped");
1579+
}
15681580
return Ok(());
15691581
}
15701582

@@ -1665,6 +1677,9 @@ fn up(
16651677
let report = up_once(root, &this_host, &runner)?;
16661678
println!("reconcile pass on host '{this_host}':");
16671679
print_report(&report);
1680+
if report.skipped {
1681+
anyhow::bail!("one-shot reconcile pass was skipped");
1682+
}
16681683
return Ok(());
16691684
}
16701685

src/message.rs

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use std::collections::{HashMap, HashSet};
1515
use std::fs;
1616
use std::io::Read;
1717
use std::path::{Path, PathBuf};
18+
use std::sync::atomic::{AtomicU64, Ordering};
1819
use std::time::{SystemTime, UNIX_EPOCH};
1920

2021
/// The alphabet st2 *generates* `<rand6>` from — Crockford base32 (`0-9a-z` minus `i l o u`). This is
@@ -183,6 +184,10 @@ fn parse_message(filename: &str, contents: &str) -> Message {
183184

184185
/// Send: write a new message file into `inbox_dir`, returning its filename. Creates `inbox_dir` if
185186
/// missing; retries on the astronomically-unlikely filename collision.
187+
///
188+
/// The message is materialized atomically (temporary sibling + rename). A direct write under the
189+
/// canonical name exposes an empty or partial file to concurrent readers after create/truncate but
190+
/// before the bytes arrive; that incomplete message has no parseable sender or subject.
186191
pub fn send_to_inbox(
187192
inbox_dir: &Path,
188193
from: &str,
@@ -193,20 +198,50 @@ pub fn send_to_inbox(
193198
) -> anyhow::Result<String> {
194199
fs::create_dir_all(inbox_dir)?;
195200
let contents = render_message(from, subject, in_reply_to, tags, body);
201+
// This deliberately cannot match `is_message_filename`, so a concurrent scan ignores it.
202+
let tmp = inbox_dir.join(tmp_name());
203+
if let Err(error) = fs::write(&tmp, &contents) {
204+
let _ = fs::remove_file(&tmp);
205+
return Err(error.into());
206+
}
196207
for _ in 0..8 {
197208
let filename = new_filename();
198209
let path = inbox_dir.join(&filename);
199210
if !path.exists() {
200-
fs::write(&path, &contents)?;
211+
if let Err(error) = fs::rename(&tmp, &path) {
212+
let _ = fs::remove_file(&tmp);
213+
return Err(error.into());
214+
}
201215
return Ok(filename);
202216
}
203217
}
218+
let _ = fs::remove_file(&tmp);
204219
anyhow::bail!(
205220
"could not allocate a unique message filename in {}",
206221
inbox_dir.display()
207222
)
208223
}
209224

225+
static TMP_COUNTER: AtomicU64 = AtomicU64::new(0);
226+
227+
fn tmp_name() -> String {
228+
format!(
229+
".message.tmp-{}-{}",
230+
std::process::id(),
231+
TMP_COUNTER.fetch_add(1, Ordering::Relaxed)
232+
)
233+
}
234+
235+
/// Read a canonical entry that was already returned by `read_dir`. Removing a message concurrently
236+
/// (for example, inbox→archive) is normal: that entry vanished, it did not become an empty message.
237+
fn read_message_contents(path: &Path) -> anyhow::Result<Option<String>> {
238+
match fs::read_to_string(path) {
239+
Ok(contents) => Ok(Some(contents)),
240+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None),
241+
Err(error) => Err(anyhow::anyhow!("reading {}: {error}", path.display())),
242+
}
243+
}
244+
210245
/// List the canonical messages in `dir` (inbox or archive), sorted by send time. Non-message files
211246
/// are skipped. Frontmatter is parsed for metadata.
212247
pub fn list_dir(dir: &Path) -> anyhow::Result<Vec<Message>> {
@@ -220,7 +255,9 @@ pub fn list_dir(dir: &Path) -> anyhow::Result<Vec<Message>> {
220255
if !is_message_filename(&name) {
221256
continue;
222257
}
223-
let contents = fs::read_to_string(entry.path()).unwrap_or_default();
258+
let Some(contents) = read_message_contents(&entry.path())? else {
259+
continue;
260+
};
224261
msgs.push(parse_message(&name, &contents));
225262
}
226263
// Primary order is send time; the `<rand6>` suffix is a deterministic tiebreak so two messages
@@ -476,6 +513,19 @@ mod tests {
476513
assert_eq!(m.body, "just a body, no frontmatter");
477514
}
478515

516+
#[test]
517+
fn a_message_removed_after_enumeration_is_skipped_not_parsed_as_empty() {
518+
let tmp = tempfile::tempdir().unwrap();
519+
let path = tmp.path().join("1784649988123-abc23z.md");
520+
fs::write(
521+
&path,
522+
render_message("alice", Some("work"), None, &[], "body"),
523+
)
524+
.unwrap();
525+
fs::remove_file(&path).unwrap();
526+
assert_eq!(read_message_contents(&path).unwrap(), None);
527+
}
528+
479529
#[test]
480530
fn reply_subject_prefixes_once() {
481531
assert_eq!(reply_subject(Some("hi")).as_deref(), Some("re: hi"));

src/reconcile.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,11 @@ pub fn reconcile<'a>(specs: &'a [AgentSpec], sessions: &[Session], this_host: &s
133133
continue;
134134
}
135135

136+
if !spec.is_runnable() {
137+
plan.unrunnable.push(spec);
138+
continue;
139+
}
140+
136141
let targets: Vec<TaskTarget> = spec
137142
.tasks
138143
.iter()
@@ -162,10 +167,7 @@ pub fn reconcile<'a>(specs: &'a [AgentSpec], sessions: &[Session], this_host: &s
162167
})
163168
.collect();
164169

165-
if targets.is_empty() {
166-
plan.unrunnable.push(spec);
167-
continue;
168-
}
170+
debug_assert!(!targets.is_empty());
169171

170172
let mut to_launch = Vec::new();
171173
for target in targets {

src/run.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,9 @@ impl CrashLoop {
380380
/// Owned, human-readable summary of one reconcile+execute pass (no borrows of the plan/specs).
381381
#[derive(Debug, Default, PartialEq, Eq)]
382382
pub struct UpReport {
383+
/// The pass could not obtain an authoritative session snapshot, so it deliberately performed no
384+
/// reconciliation. Long-running supervisors retry; a one-shot caller must exit unsuccessfully.
385+
pub skipped: bool,
383386
/// pty ids spawned this pass.
384387
pub launched: Vec<String>,
385388
/// pty ids torn down (retired agents) this pass.
@@ -410,7 +413,8 @@ impl UpReport {
410413
/// True when the pass actually changed something (or hit an error) — used to keep the loop's log
411414
/// quiet on no-op ticks.
412415
pub fn is_noteworthy(&self) -> bool {
413-
!self.launched.is_empty()
416+
self.skipped
417+
|| !self.launched.is_empty()
414418
|| !self.torn_down.is_empty()
415419
|| !self.gc.is_empty()
416420
|| !self.flapping.is_empty()
@@ -640,6 +644,7 @@ fn reconcile_pass(
640644
let sessions = match runner.list_sessions() {
641645
Ok(s) => s,
642646
Err(e) => {
647+
report.skipped = true;
643648
report
644649
.errors
645650
.push(format!("list sessions (pass skipped): {e}"));
@@ -798,6 +803,7 @@ pub fn reconcile_pass_specs(
798803
let sessions = match runner.list_sessions() {
799804
Ok(s) => s,
800805
Err(e) => {
806+
report.skipped = true;
801807
report
802808
.errors
803809
.push(format!("list sessions (pass skipped): {e}"));

src/shepherd.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,7 @@ mod tests {
550550
restart: None,
551551
tasks: vec![Task {
552552
kind: TaskKind::Pty,
553+
derived: false,
553554
name: "agent".into(),
554555
id: None,
555556
command: Some(command.into()),
@@ -1005,6 +1006,7 @@ mod tests {
10051006
let mut wrong_agent = root("root", "exec claude");
10061007
wrong_agent.tasks.push(Task {
10071008
kind: TaskKind::Exec,
1009+
derived: false,
10081010
name: "audit".into(),
10091011
id: None,
10101012
command: Some("exec codex".into()),

0 commit comments

Comments
 (0)