Skip to content

Commit 14fe7dc

Browse files
schickling-assistantclaudemyobie
authored
refactor: extract the agent-spec crate (#54)
* refactor: extract the agent-spec crate Move the declaration model (`spec`), the KDL parser (`kdl_format`), and the catalog walk (`discovery`) into `crates/agent-spec`, and have st2 consume it. No behavior change: `spec.rs` and `kdl_format.rs` move byte-for-byte. st2 re-exports the crate under the original paths (`st2::spec::…`, `st2::discovery::…`), so `src/main.rs` and the whole test suite are untouched. The ~9 in-tree consumers change only `crate::spec::` → `agent_spec::spec::`. `kdl_format` stays private inside the crate, so the published surface is exactly `spec` + `discovery` — a catalog reader parses through `discover` and resolves identity/host the one way the runner does. `validate` needed the pre-lowering `type` and `identity`, which it read via the `pub(crate)` `parse_raw_file`. Rather than export the permissive on-disk `RawSpec`, the crate exposes a narrow `Declared { identity, job_type }` and `parse_declared`. `RawSpec` stays private and free to change. The root stays a real package — `flake.nix` reads `package.version` from it, and a virtual manifest has no `[package]`. `cargoTestFlags` gains `--workspace` because cargo would otherwise select only `st2` and silently stop gating the crate's 5 unit tests (149 lib tests before = 144 + 5 after). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> agent-session-id: 8464a883-6285-4cd0-a9ec-3eba09830300 agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty * docs: link JobType plainly in the Declared docs `#[cfg(doc)]`-importing a type just to satisfy an intra-doc link is a construct this tree uses nowhere else; plain backticks read the same and match the surrounding doc comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> agent-session-id: 8464a883-6285-4cd0-a9ec-3eba09830300 agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty * docs: correct the parse_declared ordering claim The doc said callers could pair the result positionally with `discover`'s specs for the same file. They cannot: `resolve_spec` returns `Ok(None)` for any node failing `looks_like_spec`, so specs are a subset of parsed nodes. `validate` never pairs positionally, so this is a doc fix only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> agent-session-id: 8464a883-6285-4cd0-a9ec-3eba09830300 agent-tool: Claude Code agent-tool-version: 2.1.220 agent-model: claude-opus-5 agent-runtime-profile: /nix/store/i8y8b542cyqi385ywcjw5fvsq24f75v4-coding-agent-runtime-profile/share/coding-agents/profile.json agent-skills-manifest: /nix/store/i81qxhzlrzcxrrdwpp6i8hagka2gby8y-agent-skills-corpus/share/agent-skills/manifest.json tooling-profile: dotfiles@unknown-dirty * test: preserve agent-spec coverage --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: myobie <179+myobie@users.noreply.github.com>
1 parent 85d5cc2 commit 14fe7dc

18 files changed

Lines changed: 145 additions & 37 deletions

Cargo.lock

Lines changed: 13 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
# The root stays a real package (not a virtual manifest): `flake.nix` reads
2+
# `package.version` out of this file as the single source of truth for the
3+
# build, and a virtual root has no `[package]` to read.
4+
[workspace]
5+
members = ["crates/agent-spec"]
6+
default-members = [".", "crates/agent-spec"]
7+
18
[package]
29
name = "st2"
310
version = "0.1.0"
@@ -16,6 +23,7 @@ path = "src/lib.rs"
1623
[dependencies]
1724
# Sync by design: the runner is I/O-light (shell-outs to `pty` plus a
1825
# folder watch + a sleep), so no tokio. Deps grow per milestone; M0 needs only parse + CLI.
26+
agent-spec = { path = "crates/agent-spec" }
1927
anyhow = "1"
2028
clap = { version = "4", features = ["derive"] }
2129
clap_complete = "4"

crates/agent-spec/Cargo.toml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
[package]
2+
name = "agent-spec"
3+
version = "0.1.0"
4+
edition = "2024"
5+
description = "Parse a catalog of rendered agent specs (KDL/TOML/JSON) into the runner-normative agent job model."
6+
license = "MIT"
7+
8+
[dependencies]
9+
anyhow = "1"
10+
kdl = "6"
11+
serde = { version = "1", features = ["derive"] }
12+
serde_json = "1"
13+
toml = "0.9"
14+
15+
[dev-dependencies]
16+
tempfile = "3"
Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,40 @@ fn collect_spec_files(root: &Path, dir: &Path, acc: &mut Vec<PathBuf>) {
9292
}
9393
}
9494

95+
/// What a declaration literally *says*, before lowering normalizes it away.
96+
///
97+
/// Lowering is lossy by design: a typo'd `type = "srvice"` becomes `JobType::Service`, and an
98+
/// identity omitted from the content is filled in from the path. Both are invisible in the resolved
99+
/// [`AgentSpec`], so a linter that wants to fault them has to see the declared form. This is that
100+
/// view — deliberately narrow, so the permissive on-disk shape itself stays private and is free to
101+
/// gain fields without breaking readers.
102+
#[derive(Debug, Clone, Default, PartialEq, Eq)]
103+
pub struct Declared {
104+
/// `identity` as written in the file. `None` when the file relies on [`path_defaults`].
105+
pub identity: Option<String>,
106+
/// `type` as written, before it is normalized to `JobType::Service`. `None` when unset.
107+
pub job_type: Option<String>,
108+
}
109+
110+
/// Read the declared (pre-lowering) values of every agent in a file — one per `agent` node for KDL,
111+
/// 0-or-1 for TOML/JSON, empty for a non-spec extension.
112+
///
113+
/// One entry per parsed node, *including* nodes [`discover`] skips as non-specs, so this is not
114+
/// positionally paired with that file's [`Discovered::specs`].
115+
pub fn parse_declared(path: &Path) -> anyhow::Result<Vec<Declared>> {
116+
Ok(parse_raw_file(path)?
117+
.into_iter()
118+
.map(|raw| Declared {
119+
identity: raw.identity,
120+
job_type: raw.job_type,
121+
})
122+
.collect())
123+
}
124+
95125
/// Parse a spec file into its raw (pre-resolution) shape — one per `agent` node for KDL, 0-or-1 for
96-
/// TOML/JSON. Non-spec extensions yield an empty vec. Shared by discovery and `validate` (which needs
97-
/// the *raw* `type` string before it is normalized away, to catch a typo'd `type = "srvice"`).
98-
pub(crate) fn parse_raw_file(path: &Path) -> anyhow::Result<Vec<RawSpec>> {
126+
/// TOML/JSON. Non-spec extensions yield an empty vec. Shared by discovery and [`parse_declared`]
127+
/// (which exposes the *raw* `type` and `identity` before normalization, without leaking [`RawSpec`]).
128+
fn parse_raw_file(path: &Path) -> anyhow::Result<Vec<RawSpec>> {
99129
let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("");
100130
let text = fs::read_to_string(path)?;
101131
Ok(match ext {

crates/agent-spec/src/lib.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//! agent-spec — read a catalog of rendered agent declarations.
2+
//!
3+
//! One agent is one declarative file, Nomad-style: the agent is the job and its `pty`/`exec` blocks
4+
//! are the tasks. This crate owns the two halves any reader of that catalog needs and nothing else:
5+
//!
6+
//! - [`spec`] — the runner-normative model a declaration lowers to ([`AgentSpec`], [`Task`], …).
7+
//! - [`discovery`] — the catalog walk: parse every `*.{kdl,toml,json}` that looks like a
8+
//! declaration, and resolve each one's `identity`/`host` with the catalog's precedence rule
9+
//! (content wins, the path supplies defaults, a mismatch is a warning).
10+
//!
11+
//! KDL is the canonical on-disk format; TOML and JSON lower to the same model. The KDL parser is a
12+
//! private implementation detail — [`discovery`] is the only supported entry point, so every reader
13+
//! resolves identity and host the same way rather than re-deriving it from filenames.
14+
//!
15+
//! st2 consumes this crate, which is what keeps it a reference implementation rather than a copy:
16+
//! a second reader (a TUI, a linter) sees exactly the fields the runner sees, including the ones
17+
//! the runner's roster JSON does not carry (`supervisor`, `role`, `workspace`, `host`).
18+
//!
19+
//! Render-only fields (`harness`, `model`, `persona`, `permissions`, `transport`, `strategy`,
20+
//! `meta{}`) are read by the render layer and deliberately dropped here — that is what keeps a
21+
//! consumer render-agnostic.
22+
23+
pub mod discovery;
24+
mod kdl_format;
25+
pub mod spec;
26+
27+
pub use discovery::{Declared, Discovered, SpecError, discover, parse_declared, path_defaults};
28+
pub use spec::{AgentSpec, JobType, Restart, RestartMode, Task, TaskKind, parse_duration};
Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
//! M1 correctness net: discovery + lowering of VRS `agent.kdl` jobs (spec.md §1–2, §4).
22
//!
33
//! Builds throwaway catalog folders, writes real job files (KDL/TOML/JSON, services), and
4-
//! asserts st2 lowers them per the spec: `pty`/`exec` task split, `restart{}`, `type`, `workspace`,
4+
//! asserts they lower per the spec: `pty`/`exec` task split, `restart{}`, `type`, `workspace`,
55
//! `supervisor`; render-only fields ignored; content/path precedence; malformed → error, not halt.
66
77
use std::fs;
88
use std::path::Path;
99
use std::time::Duration;
1010

11-
use st2::spec::TaskKind;
12-
use st2::{AgentSpec, JobType, discover};
11+
use agent_spec::spec::TaskKind;
12+
use agent_spec::{AgentSpec, JobType, discover};
1313

1414
fn write(root: &Path, rel: &str, contents: &str) {
1515
let path = root.join(rel);
@@ -91,7 +91,7 @@ fn parses_full_kdl_service_job() {
9191
assert_eq!(r.attempts, 5);
9292
assert_eq!(r.interval, Duration::from_secs(90));
9393
assert_eq!(r.delay, Duration::from_secs(5));
94-
assert_eq!(r.mode, st2::RestartMode::Fail);
94+
assert_eq!(r.mode, agent_spec::RestartMode::Fail);
9595

9696
// tasks: pty "agent" + exec "ding" (sorted by name)
9797
assert_eq!(s.tasks.len(), 2);
@@ -213,7 +213,10 @@ command = "st2 ding hetz.fetcher"
213213
let s = &found.specs[0];
214214
assert_eq!(s.identity, "fetcher");
215215
assert_eq!(s.job_type, JobType::Service);
216-
assert_eq!(s.restart.clone().unwrap().mode, st2::RestartMode::Delay);
216+
assert_eq!(
217+
s.restart.clone().unwrap().mode,
218+
agent_spec::RestartMode::Delay
219+
);
217220
assert_eq!(s.tasks.len(), 2);
218221
assert_eq!(
219222
s.tasks.iter().find(|t| t.name == "agent").unwrap().kind,

flake.nix

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,21 @@
8787
--fish completions-fish
8888
'';
8989

90-
# Run the hermetic unit tests plus the real lifecycle-hook integration
91-
# test. The remaining integration tests assume facilities the Nix build
92-
# sandbox deliberately lacks: `/usr/bin/git` on a hardcoded `PATH`,
93-
# live PTY backends, or a systemd `--user` manager. They remain native
94-
# gates, while the flake proves that its own packaged hooks execute.
90+
# Run the hermetic unit tests plus agent-spec's discovery test and the
91+
# real lifecycle-hook integration tests. The remaining root integration
92+
# tests assume facilities the Nix build sandbox deliberately lacks:
93+
# `/usr/bin/git` on a hardcoded `PATH`, live PTY backends, or a systemd
94+
# `--user` manager. They remain native gates, while the flake proves
95+
# that its parser and packaged hooks execute.
96+
# `--workspace` because the root is a real package: without it cargo
97+
# selects only `st2` and silently skips the `agent-spec` crate.
9598
cargoTestFlags = [
99+
"--workspace"
96100
"--lib"
97101
"--bins"
98102
"--test"
103+
"discovery"
104+
"--test"
99105
"codex_hooks"
100106
"--test"
101107
"hooks"

src/eval_run.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::expand::expand_catalog;
1616
use crate::flapping::FlappingCap;
1717
use crate::reconcile::reconcile;
1818
use crate::run::{Runner, SystemRunner, UpReport, detect_host, execute};
19-
use crate::spec::{AgentSpec, JobType, Task, TaskKind};
19+
use agent_spec::spec::{AgentSpec, JobType, Task, TaskKind};
2020

2121
macro_rules! eval_log {
2222
($($arg:tt)*) => {

0 commit comments

Comments
 (0)