Skip to content

Commit e120f71

Browse files
authored
Record leases from the pre-tool hook, three CLI paper cuts, and the fork/exclude finding (#21)
* Record what a tool is about to touch, from the pre-tool hook Anything scheduling work alongside an agent needs to know which paths the agent is in right now. Predicting that from a plan does not work: asked to declare its writes, one agent named a single file having written five, and a planned node predicted one path against nine observed. The pre-tool hook is already handed the exact path before the edit lands, so it records it. Two placement decisions, both forced by measurement. It rides on the existing pre-tool hook rather than being a hook of its own. A separate hook process costs about as much as this whole binary's hook path, so a second one roughly doubles what every Edit, Write and Bash pays to record a path this process already holds. Here it is one append. It writes into the store, not the repo. The obvious placement, <repo>/.speculation/leases, took the hook from 65ms to 120ms with a daemon running: a write inside the tree wakes the watcher, and this hook then waits for the checkpoint its own write caused. It would also have appeared in every blast radius as a changed path. Outside the tree, neither happens, and the cost is 1.4ms on a 65ms hook. A missing path records a wildcard, because a Bash command can touch anything and a reader should block rather than guess. ACYCLIC_NO_LEASES turns the whole thing off without a rebuild, which is also what made the measurement honest. * Say what went wrong, cap `turns`, and spell out how `exclude` matches Three small things, each one a paper cut hit while driving the CLI from scripts. A daemon that exits mid-answer closes the socket, so the read succeeds with nothing and serde called that "decode: EOF while parsing a value at line 1 column 0". That reads like corruption; it means the daemon stopped. Running `stop` and then any other verb was enough to see it. The response parse moves into its own function so the shutdown case is covered by a test rather than a timing-dependent race — it could not be reproduced on demand in six tries. `turns` had no `--limit` while `timeline` did, so a long session printed everything. Trimmed on the rendering side, since the daemon already answers with the session's turns and a limit is a display concern, and the trim says how many turns it hid rather than quietly dropping history. `exclude` matches paths, not names, and the wrong form fails silently while looking like it worked: exclude = ["__pycache__"] leaves src/__pycache__ captured. The README now says so, with the measured cost of getting it wrong on a Rust tree — 1.4 GB of store and +29s per build against 14 MB and 35s. Also hardens the lease writer from the previous commit: it creates the store root if a tool call precedes `init`, and six tests cover repo-relative paths, the wildcard for tools that name no file, tab rejection, the off switch, append behaviour, and — the regression that cost 65ms to 120ms — that nothing is ever written inside the repo. * Record that `exclude` does not govern what a fork writes A speculative agent working inside a fork mount, with no shell and no LSP tool, still ended up with a target/ in its fork: the host's edit-time diagnostics ran cargo check for it. Nothing consults the exclude set on a fork's overlay writes, so all of it went into the object store — 1.3 GB in five minutes on a 2.4 MB source tree — and promote then snapshotted the fork, target/ and all, for another 2.2 GB in one minute. The backend answered "Objects capacity exhausted", the rewind's cleanup hit the same wall, and the daemon fell into a recovery rescan it could not finish. Ten minutes after init, restore and checkpoint both failed. The design already says forks do not see excluded paths, and they do not: the base generation holds none. The gap is paths created inside the fork. The write-up records the store's own growth by minute, the promote message that shows the seam from the other side ("target is excluded from snapshots; no checkpoint holds it" — after capturing it as a fork path), and three fixes in order of how much they change: apply exclude to fork writes, have promote skip excluded paths, refuse a snapshot before it can exhaust the store. * Test fixture: build the repo config path from product::, not a literal The product-name guard in CI caught a hardcoded `.acyclic` in the lease writer's test fixture. The helpers for exactly this already exist: product::repo_config_dir() and repo_config_file(). * Leases: relativise the path by component, not by string prefix Stripping the repo as a string and then trimming '/' left a leading backslash on Windows (CI: '\src/report.py'), and a real Windows host would have recorded 'src\report.py', which never matches a path from `diff`. Strip the prefix as a Path and join the components with '/'.
1 parent e7684c2 commit e120f71

7 files changed

Lines changed: 382 additions & 9 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ Not yet covered: an `install` writer for Codex's MCP config (TOML), Kimi Code CL
7474

7575
Speculation is configured separately, in `~/.config/acyclic/speculate.toml` — per developer, never checked in, because turning it on can spend that developer's money. See [Speculation](#speculation).
7676

77+
`exclude` matches **paths, not names**: `exclude = ["__pycache__"]` excludes a top-level `__pycache__/` and nothing else — it will not exclude `src/__pycache__/`. Name every path you mean (`"src/__pycache__"`), or exclude the directory that contains them. The wrong form fails silently and looks like it worked: the build output is captured anyway, and a Rust `target/` measured 1.4&nbsp;GB of store and +29&nbsp;s per build against 14&nbsp;MB and 35&nbsp;s with it excluded.
78+
7779
Adding a path to `exclude` takes effect at the next daemon start; the baseline it builds is scrubbed, and every later checkpoint skips the path. Generations captured before the rule still hold it (see below).
7880

7981
## Speculation

crates/acyclic/src/client.rs

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,26 @@ impl Client {
144144
self.stream
145145
.read_line(&mut response_line)
146146
.map_err(|error| format!("receive: {error}"))?;
147-
let response: proto::Response =
148-
serde_json::from_str(&response_line).map_err(|error| format!("decode: {error}"))?;
149-
match response.payload {
150-
proto::Payload::Ok(reply) => Ok(*reply),
151-
proto::Payload::Err { message } => Err(message),
152-
}
147+
// A daemon that exits mid-answer closes the socket, so the read
148+
// succeeds with nothing. Left to serde that surfaced as
149+
// "decode: EOF while parsing a value at line 1 column 0", which reads
150+
// like corruption rather than what it is: the daemon stopped. Anyone
151+
// running `stop` and then any other verb hit it.
152+
parse_response(&response_line)
153+
}
154+
}
155+
156+
/// One response line to a reply. Split out from the socket so the
157+
/// shutdown case can be tested without a daemon.
158+
fn parse_response(line: &str) -> Result<proto::Reply, String> {
159+
if line.trim().is_empty() {
160+
return Err("daemon stopped while answering; nothing was recorded".to_owned());
161+
}
162+
let response: proto::Response =
163+
serde_json::from_str(line).map_err(|error| format!("decode: {error}"))?;
164+
match response.payload {
165+
proto::Payload::Ok(reply) => Ok(*reply),
166+
proto::Payload::Err { message } => Err(message),
153167
}
154168
}
155169

@@ -335,3 +349,61 @@ fn wait_for_socket(
335349
std::thread::sleep(Duration::from_millis(200));
336350
}
337351
}
352+
353+
#[cfg(test)]
354+
mod tests {
355+
use super::*;
356+
357+
#[test]
358+
fn a_closed_socket_says_the_daemon_stopped() {
359+
// The regression: a daemon that exits mid-answer closes the socket, the
360+
// read succeeds with nothing, and serde called that
361+
// "decode: EOF while parsing a value at line 1 column 0" — which reads
362+
// as corruption. Anyone running `stop` then any other verb saw it.
363+
for line in ["", "\n", " \n"] {
364+
let error = parse_response(line).expect_err("empty must be an error");
365+
assert!(
366+
error.contains("daemon stopped"),
367+
"unhelpful message for {line:?}: {error}"
368+
);
369+
assert!(!error.contains("decode"), "leaked serde wording: {error}");
370+
}
371+
}
372+
373+
#[test]
374+
fn malformed_json_still_reports_a_decode_error() {
375+
// Genuine corruption must stay distinguishable from a clean shutdown.
376+
let error = parse_response("{not json").expect_err("must be an error");
377+
assert!(error.starts_with("decode:"), "{error}");
378+
}
379+
380+
#[test]
381+
fn an_error_payload_surfaces_its_own_message() {
382+
// Built from the protocol types and serialized, rather than a
383+
// hand-written literal: the payload is flattened and renamed, so a
384+
// literal here would test my guess at the wire format instead of the
385+
// format. The first attempt did exactly that and failed.
386+
let line = serde_json::to_string(&proto::Response {
387+
id: 1,
388+
payload: proto::Payload::Err {
389+
message: "no such checkpoint".to_owned(),
390+
},
391+
})
392+
.expect("serialize");
393+
let error = parse_response(&line).expect_err("must be an error");
394+
assert_eq!(error, "no such checkpoint");
395+
}
396+
397+
#[test]
398+
fn an_ok_payload_round_trips() {
399+
let line = serde_json::to_string(&proto::Response {
400+
id: 1,
401+
payload: proto::Payload::Ok(Box::new(proto::Reply::Pong)),
402+
})
403+
.expect("serialize");
404+
assert!(matches!(
405+
parse_response(&line).expect("ok payload"),
406+
proto::Reply::Pong
407+
));
408+
}
409+
}

crates/acyclic/src/hook.rs

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,19 @@ struct Payload {
4040
/// run, standing in for `tool_name` when that field is absent.
4141
#[serde(default)]
4242
command: Option<String>,
43+
/// `PreToolUse`: what the tool is about to touch. Edit/Write/MultiEdit
44+
/// name a path here; Bash does not, and a shell command can touch
45+
/// anything — which is why a missing path records a wildcard.
46+
#[serde(default)]
47+
tool_input: Option<ToolInput>,
48+
}
49+
50+
#[derive(Debug, Default, serde::Deserialize)]
51+
struct ToolInput {
52+
#[serde(default)]
53+
file_path: Option<String>,
54+
#[serde(default)]
55+
path: Option<String>,
4356
}
4457

4558
impl Payload {
@@ -132,6 +145,18 @@ pub fn run(repo: &Path, event: &str) -> i32 {
132145
let mut payload = parse_payload(&raw);
133146
let host = std::env::var("ACYCLIC_HOST").unwrap_or_else(|_| "claude-code".into());
134147

148+
// Before the connect, deliberately. A lease says what the agent is about
149+
// to touch, and that is worth recording whether or not a daemon is up —
150+
// the connect returns early when there is none, so recording afterwards
151+
// would silently stop working exactly when checkpointing is off.
152+
if event == HookEvent::PreTool {
153+
let path = payload
154+
.tool_input
155+
.as_ref()
156+
.and_then(|i| i.file_path.as_deref().or(i.path.as_deref()));
157+
record_lease(repo, payload.tool_name.as_deref(), path);
158+
}
159+
135160
// A session start may spawn the daemon, but never waits for its first
136161
// snapshot: the agent's first turn is behind this hook.
137162
let spawn = if event == HookEvent::SessionStart {
@@ -239,6 +264,70 @@ fn parse_payload(raw: &str) -> Payload {
239264
serde_json::from_str(raw).unwrap_or_default()
240265
}
241266

267+
/// Record what the agent is *about* to touch, for anything scheduling work
268+
/// alongside it.
269+
///
270+
/// Two decisions here, both forced by measurement.
271+
///
272+
/// **It rides on the pre-tool hook** rather than being a hook of its own. A
273+
/// separate hook process measured ~10ms at best against this binary's own
274+
/// ~10ms, so a second hook roughly doubles what every Edit, Write and Bash
275+
/// pays — to record a path this process is already holding. Here the marginal
276+
/// cost is one append.
277+
///
278+
/// **It writes into the STORE, not the repo.** The obvious placement,
279+
/// `<repo>/.speculation/leases`, took the pre-tool hook from 65ms to 120ms with
280+
/// a daemon running: a write inside the tree wakes the watcher, and this very
281+
/// hook then waits for the resulting checkpoint. The lease write became work
282+
/// the lease writer waited on. It would also have shown up in every blast
283+
/// radius as a changed path. Outside the tree, neither happens.
284+
///
285+
/// Every failure is swallowed. A hook may not break a tool call, and a missing
286+
/// lease only means a speculator schedules more conservatively.
287+
fn record_lease(repo: &Path, tool: Option<&str>, path: Option<&str>) {
288+
use std::io::Write;
289+
290+
// An off switch, because this sits on the agent's critical path. Anything
291+
// that runs on every Edit, Write and Bash should be disableable without a
292+
// rebuild.
293+
if std::env::var_os("ACYCLIC_NO_LEASES").is_some() {
294+
return;
295+
}
296+
let Ok(paths) = crate::store_paths(repo) else {
297+
return;
298+
};
299+
// The store root exists after `init`, but a lease is worth recording from
300+
// the very first tool call, which can precede it.
301+
if std::fs::create_dir_all(&paths.root).is_err() {
302+
return;
303+
}
304+
let at = std::time::SystemTime::now()
305+
.duration_since(std::time::UNIX_EPOCH)
306+
.map_or(0, |d| d.as_secs());
307+
// Repo-relative with `/` separators whatever the host wrote, and no tab:
308+
// a reader compares these against paths from `diff` and the file is tab
309+
// separated, so a line must never mis-split.
310+
let path = path
311+
.map(|p| {
312+
let p = Path::new(p);
313+
let rel = p.strip_prefix(repo).unwrap_or(p);
314+
rel.components()
315+
.map(|c| c.as_os_str().to_string_lossy())
316+
.collect::<Vec<_>>()
317+
.join("/")
318+
})
319+
.filter(|p| !p.is_empty() && !p.contains('\t'))
320+
.unwrap_or_else(|| "*".to_owned());
321+
let tool = tool.filter(|t| !t.contains('\t')).unwrap_or("?");
322+
if let Ok(mut f) = std::fs::OpenOptions::new()
323+
.create(true)
324+
.append(true)
325+
.open(paths.root.join("leases"))
326+
{
327+
let _ = writeln!(f, "{at}\t{tool}\t{path}");
328+
}
329+
}
330+
242331
fn connect(repo: &Path, spawn: Spawn) -> Result<Client, ConnectError> {
243332
let paths = crate::store_paths(repo).map_err(ConnectError::Other)?;
244333
let log = paths.root.join("daemon.log");
@@ -298,4 +387,108 @@ mod tests {
298387
assert!(payload.tool_name.is_none());
299388
}
300389
}
390+
391+
/// A scratch repo whose store lives beside it, so `record_lease` writes
392+
/// somewhere real and nothing touches the developer's own stores. The
393+
/// store root comes from the repo's own config, so that is where the
394+
/// redirect goes — there is no env override, deliberately.
395+
fn scratch() -> (tempfile::TempDir, std::path::PathBuf) {
396+
let dir = tempfile::tempdir().expect("tempdir");
397+
let repo = dir.path().join("repo");
398+
std::fs::create_dir_all(repo.join(acyclic_engine::product::repo_config_dir()))
399+
.expect("repo");
400+
let stores = dir.path().join("stores");
401+
std::fs::create_dir_all(&stores).expect("stores");
402+
std::fs::write(
403+
repo.join(acyclic_engine::product::repo_config_file()),
404+
format!("store_dir = {:?}\n", stores.to_string_lossy()),
405+
)
406+
.expect("config");
407+
// The store root is created lazily by `init`; the lease writer must
408+
// work before that, which is what create_dir_all in it is for.
409+
(dir, repo)
410+
}
411+
412+
fn leases_of(repo: &Path) -> String {
413+
let paths = crate::store_paths(repo).expect("store paths");
414+
std::fs::read_to_string(paths.root.join("leases")).unwrap_or_default()
415+
}
416+
417+
#[test]
418+
fn a_path_is_recorded_repo_relative() {
419+
let (_dir, repo) = scratch();
420+
let absolute = repo.join("src/report.py");
421+
record_lease(&repo, Some("Edit"), Some(&absolute.to_string_lossy()));
422+
record_lease(&repo, Some("Write"), Some("src/money.py"));
423+
let text = leases_of(&repo);
424+
// Absolute and relative inputs both land relative: a reader compares
425+
// these against paths from `diff`, which are repo-relative.
426+
assert!(
427+
text.contains("\tEdit\tsrc/report.py\n"),
428+
"absolute path not made relative: {text}"
429+
);
430+
assert!(text.contains("\tWrite\tsrc/money.py\n"), "{text}");
431+
}
432+
433+
#[test]
434+
fn a_tool_with_no_path_records_a_wildcard() {
435+
let (_dir, repo) = scratch();
436+
// Bash names no file and can touch anything, so a reader must block
437+
// rather than guess.
438+
record_lease(&repo, Some("Bash"), None);
439+
assert!(
440+
leases_of(&repo).contains("\tBash\t*\n"),
441+
"{}",
442+
leases_of(&repo)
443+
);
444+
}
445+
446+
#[test]
447+
fn a_tab_in_either_field_is_refused() {
448+
let (_dir, repo) = scratch();
449+
// The file is tab separated. A tab smuggled in through a filename
450+
// would make a reader mis-split the line and treat junk as a path.
451+
record_lease(&repo, Some("Ed\tit"), Some("src/a\tb.py"));
452+
let text = leases_of(&repo);
453+
assert!(text.contains("\t?\t*\n"), "tabs not neutralised: {text}");
454+
assert_eq!(text.lines().count(), 1, "one line per call: {text}");
455+
}
456+
457+
#[test]
458+
fn the_kill_switch_writes_nothing() {
459+
let (_dir, repo) = scratch();
460+
std::env::set_var("ACYCLIC_NO_LEASES", "1");
461+
record_lease(&repo, Some("Edit"), Some("src/report.py"));
462+
std::env::remove_var("ACYCLIC_NO_LEASES");
463+
assert!(
464+
leases_of(&repo).is_empty(),
465+
"the off switch must be an off switch"
466+
);
467+
}
468+
469+
#[test]
470+
fn leases_never_land_inside_the_repo() {
471+
let (_dir, repo) = scratch();
472+
record_lease(&repo, Some("Edit"), Some("src/report.py"));
473+
// The regression this guards: writing into the tree woke the watcher,
474+
// and the pre-tool hook then waited for the checkpoint its own write
475+
// caused — 65ms to 120ms. It would also have shown up in every blast
476+
// radius as a changed path.
477+
assert!(
478+
!repo.join(".speculation").exists(),
479+
"a lease inside the repo is captured by the watcher and inflates \
480+
every diff"
481+
);
482+
}
483+
484+
#[test]
485+
fn appending_keeps_earlier_lines() {
486+
let (_dir, repo) = scratch();
487+
for path in ["a.py", "b.py", "c.py"] {
488+
record_lease(&repo, Some("Edit"), Some(path));
489+
}
490+
// A reader takes the live window by timestamp, so history must not be
491+
// truncated by a later write.
492+
assert_eq!(leases_of(&repo).lines().count(), 3, "{}", leases_of(&repo));
493+
}
301494
}

crates/acyclic/src/main.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ enum Command {
9191
Turns {
9292
#[arg(long)]
9393
session: Option<String>,
94+
/// Newest first, like `timeline --limit`. A long session has one turn
95+
/// per prompt, so the default is the recent history rather than all of
96+
/// it.
97+
#[arg(long, default_value_t = 50)]
98+
limit: usize,
9499
},
95100
/// One checkpoint resolved to its session, turn, and prompt.
96101
Show { checkpoint: i64 },
@@ -669,7 +674,7 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
669674
}
670675
Ok(())
671676
}
672-
Command::Turns { session } => {
677+
Command::Turns { session, limit } => {
673678
let reply = client.call(proto::Op::Turns {
674679
session_id: session,
675680
})?;
@@ -680,7 +685,11 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
680685
println!("no turns recorded (the user-prompt hook records them)");
681686
return Ok(());
682687
}
683-
for turn in turns {
688+
// Trimmed here rather than in the protocol: the daemon already
689+
// answers with the session's turns, and a limit is a display
690+
// concern. Newest first, matching `timeline`.
691+
let hidden = turns.len().saturating_sub(limit);
692+
for turn in turns.into_iter().take(limit) {
684693
let range = match (turn.first_checkpoint, turn.last_checkpoint) {
685694
(Some(first), Some(last)) if first != last => format!("#{first}..#{last}"),
686695
(Some(first), _) => format!("#{first}"),
@@ -695,6 +704,9 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
695704
brief::quote(&turn.prompt, 72),
696705
);
697706
}
707+
if hidden > 0 {
708+
println!("… {hidden} older turn(s) not shown (--limit)");
709+
}
698710
Ok(())
699711
}
700712
Command::Show { checkpoint } => {

docs/design/03-forks.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ is what exists, including where it diverged from the plan.
7373
## Notes
7474

7575
- Fork orchestration of subagents is uniquely plugin-shaped — it must live inside the host.
76+
- `exclude` does not apply to paths created inside a fork: build output written into a mount is
77+
captured in full and snapshotted by `promote`, which wedged a store in one live run. See
78+
[fork-writes-bypass-exclude.md](fork-writes-bypass-exclude.md).
7679
- Filesystem-layer enforcement for Launch 4's guarded paths arrives with the mount option. This turned out to be the decisive argument: the mount shipped and reflinks never did, and Safe Mode refuses to start without a mount provider.
7780

7881
## Open questions (not yet settled)

docs/design/05-monorepo.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ which reads as though Launch 5 exists.
7575
answer it gives will be silently incomplete rather than refused. *No lean.*
7676
4. **Forks and Safe Mode sessions do not see excluded paths either**, which
7777
bears directly on "searches against a fork must answer from that tree's
78-
state". *No lean.*
78+
state". *No lean.* The converse is now a known gap: paths *created* inside
79+
a fork are captured regardless of `exclude` — see
80+
[fork-writes-bypass-exclude.md](fork-writes-bypass-exclude.md).
7981
5. **Does the name change?** Calling this "the index engine" collides with the
8082
shipped metadata index. *Current lean: rename this launch, not the shipped
8183
component.*

0 commit comments

Comments
 (0)