Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ Not yet covered: an `install` writer for Codex's MCP config (TOML), Kimi Code CL

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).

`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 GB of store and +29 s per build against 14 MB and 35 s with it excluded.

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).

## Speculation
Expand Down
84 changes: 78 additions & 6 deletions crates/acyclic/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,26 @@ impl Client {
self.stream
.read_line(&mut response_line)
.map_err(|error| format!("receive: {error}"))?;
let response: proto::Response =
serde_json::from_str(&response_line).map_err(|error| format!("decode: {error}"))?;
match response.payload {
proto::Payload::Ok(reply) => Ok(*reply),
proto::Payload::Err { message } => Err(message),
}
// A daemon that exits mid-answer closes the socket, so the read
// succeeds with nothing. Left to serde that surfaced as
// "decode: EOF while parsing a value at line 1 column 0", which reads
// like corruption rather than what it is: the daemon stopped. Anyone
// running `stop` and then any other verb hit it.
parse_response(&response_line)
}
}

/// One response line to a reply. Split out from the socket so the
/// shutdown case can be tested without a daemon.
fn parse_response(line: &str) -> Result<proto::Reply, String> {
if line.trim().is_empty() {
return Err("daemon stopped while answering; nothing was recorded".to_owned());
}
let response: proto::Response =
serde_json::from_str(line).map_err(|error| format!("decode: {error}"))?;
match response.payload {
proto::Payload::Ok(reply) => Ok(*reply),
proto::Payload::Err { message } => Err(message),
}
}

Expand Down Expand Up @@ -335,3 +349,61 @@ fn wait_for_socket(
std::thread::sleep(Duration::from_millis(200));
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn a_closed_socket_says_the_daemon_stopped() {
// The regression: a daemon that exits mid-answer closes the socket, the
// read succeeds with nothing, and serde called that
// "decode: EOF while parsing a value at line 1 column 0" — which reads
// as corruption. Anyone running `stop` then any other verb saw it.
for line in ["", "\n", " \n"] {
let error = parse_response(line).expect_err("empty must be an error");
assert!(
error.contains("daemon stopped"),
"unhelpful message for {line:?}: {error}"
);
assert!(!error.contains("decode"), "leaked serde wording: {error}");
}
}

#[test]
fn malformed_json_still_reports_a_decode_error() {
// Genuine corruption must stay distinguishable from a clean shutdown.
let error = parse_response("{not json").expect_err("must be an error");
assert!(error.starts_with("decode:"), "{error}");
}

#[test]
fn an_error_payload_surfaces_its_own_message() {
// Built from the protocol types and serialized, rather than a
// hand-written literal: the payload is flattened and renamed, so a
// literal here would test my guess at the wire format instead of the
// format. The first attempt did exactly that and failed.
let line = serde_json::to_string(&proto::Response {
id: 1,
payload: proto::Payload::Err {
message: "no such checkpoint".to_owned(),
},
})
.expect("serialize");
let error = parse_response(&line).expect_err("must be an error");
assert_eq!(error, "no such checkpoint");
}

#[test]
fn an_ok_payload_round_trips() {
let line = serde_json::to_string(&proto::Response {
id: 1,
payload: proto::Payload::Ok(Box::new(proto::Reply::Pong)),
})
.expect("serialize");
assert!(matches!(
parse_response(&line).expect("ok payload"),
proto::Reply::Pong
));
}
}
193 changes: 193 additions & 0 deletions crates/acyclic/src/hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ struct Payload {
/// run, standing in for `tool_name` when that field is absent.
#[serde(default)]
command: Option<String>,
/// `PreToolUse`: what the tool is about to touch. Edit/Write/MultiEdit
/// name a path here; Bash does not, and a shell command can touch
/// anything — which is why a missing path records a wildcard.
#[serde(default)]
tool_input: Option<ToolInput>,
}

#[derive(Debug, Default, serde::Deserialize)]
struct ToolInput {
#[serde(default)]
file_path: Option<String>,
#[serde(default)]
path: Option<String>,
}

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

// Before the connect, deliberately. A lease says what the agent is about
// to touch, and that is worth recording whether or not a daemon is up —
// the connect returns early when there is none, so recording afterwards
// would silently stop working exactly when checkpointing is off.
if event == HookEvent::PreTool {
let path = payload
.tool_input
.as_ref()
.and_then(|i| i.file_path.as_deref().or(i.path.as_deref()));
record_lease(repo, payload.tool_name.as_deref(), path);
}

// A session start may spawn the daemon, but never waits for its first
// snapshot: the agent's first turn is behind this hook.
let spawn = if event == HookEvent::SessionStart {
Expand Down Expand Up @@ -239,6 +264,70 @@ fn parse_payload(raw: &str) -> Payload {
serde_json::from_str(raw).unwrap_or_default()
}

/// Record what the agent is *about* to touch, for anything scheduling work
/// alongside it.
///
/// Two decisions here, both forced by measurement.
///
/// **It rides on the pre-tool hook** rather than being a hook of its own. A
/// separate hook process measured ~10ms at best against this binary's own
/// ~10ms, so a second hook roughly doubles what every Edit, Write and Bash
/// pays — to record a path this process is already holding. Here the marginal
/// cost is one append.
///
/// **It writes into the STORE, not the repo.** The obvious placement,
/// `<repo>/.speculation/leases`, took the pre-tool hook from 65ms to 120ms with
/// a daemon running: a write inside the tree wakes the watcher, and this very
/// hook then waits for the resulting checkpoint. The lease write became work
/// the lease writer waited on. It would also have shown up in every blast
/// radius as a changed path. Outside the tree, neither happens.
///
/// Every failure is swallowed. A hook may not break a tool call, and a missing
/// lease only means a speculator schedules more conservatively.
fn record_lease(repo: &Path, tool: Option<&str>, path: Option<&str>) {
use std::io::Write;

// An off switch, because this sits on the agent's critical path. Anything
// that runs on every Edit, Write and Bash should be disableable without a
// rebuild.
if std::env::var_os("ACYCLIC_NO_LEASES").is_some() {
return;
}
let Ok(paths) = crate::store_paths(repo) else {
return;
};
// The store root exists after `init`, but a lease is worth recording from
// the very first tool call, which can precede it.
if std::fs::create_dir_all(&paths.root).is_err() {
return;
}
let at = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map_or(0, |d| d.as_secs());
// Repo-relative with `/` separators whatever the host wrote, and no tab:
// a reader compares these against paths from `diff` and the file is tab
// separated, so a line must never mis-split.
let path = path
.map(|p| {
let p = Path::new(p);
let rel = p.strip_prefix(repo).unwrap_or(p);
rel.components()
.map(|c| c.as_os_str().to_string_lossy())
.collect::<Vec<_>>()
.join("/")
})
.filter(|p| !p.is_empty() && !p.contains('\t'))
.unwrap_or_else(|| "*".to_owned());
let tool = tool.filter(|t| !t.contains('\t')).unwrap_or("?");
if let Ok(mut f) = std::fs::OpenOptions::new()
.create(true)
.append(true)
.open(paths.root.join("leases"))
{
let _ = writeln!(f, "{at}\t{tool}\t{path}");
}
}

fn connect(repo: &Path, spawn: Spawn) -> Result<Client, ConnectError> {
let paths = crate::store_paths(repo).map_err(ConnectError::Other)?;
let log = paths.root.join("daemon.log");
Expand Down Expand Up @@ -298,4 +387,108 @@ mod tests {
assert!(payload.tool_name.is_none());
}
}

/// A scratch repo whose store lives beside it, so `record_lease` writes
/// somewhere real and nothing touches the developer's own stores. The
/// store root comes from the repo's own config, so that is where the
/// redirect goes — there is no env override, deliberately.
fn scratch() -> (tempfile::TempDir, std::path::PathBuf) {
let dir = tempfile::tempdir().expect("tempdir");
let repo = dir.path().join("repo");
std::fs::create_dir_all(repo.join(acyclic_engine::product::repo_config_dir()))
.expect("repo");
let stores = dir.path().join("stores");
std::fs::create_dir_all(&stores).expect("stores");
std::fs::write(
repo.join(acyclic_engine::product::repo_config_file()),
format!("store_dir = {:?}\n", stores.to_string_lossy()),
)
.expect("config");
// The store root is created lazily by `init`; the lease writer must
// work before that, which is what create_dir_all in it is for.
(dir, repo)
}

fn leases_of(repo: &Path) -> String {
let paths = crate::store_paths(repo).expect("store paths");
std::fs::read_to_string(paths.root.join("leases")).unwrap_or_default()
}

#[test]
fn a_path_is_recorded_repo_relative() {
let (_dir, repo) = scratch();
let absolute = repo.join("src/report.py");
record_lease(&repo, Some("Edit"), Some(&absolute.to_string_lossy()));
record_lease(&repo, Some("Write"), Some("src/money.py"));
let text = leases_of(&repo);
// Absolute and relative inputs both land relative: a reader compares
// these against paths from `diff`, which are repo-relative.
assert!(
text.contains("\tEdit\tsrc/report.py\n"),
"absolute path not made relative: {text}"
);
assert!(text.contains("\tWrite\tsrc/money.py\n"), "{text}");
}

#[test]
fn a_tool_with_no_path_records_a_wildcard() {
let (_dir, repo) = scratch();
// Bash names no file and can touch anything, so a reader must block
// rather than guess.
record_lease(&repo, Some("Bash"), None);
assert!(
leases_of(&repo).contains("\tBash\t*\n"),
"{}",
leases_of(&repo)
);
}

#[test]
fn a_tab_in_either_field_is_refused() {
let (_dir, repo) = scratch();
// The file is tab separated. A tab smuggled in through a filename
// would make a reader mis-split the line and treat junk as a path.
record_lease(&repo, Some("Ed\tit"), Some("src/a\tb.py"));
let text = leases_of(&repo);
assert!(text.contains("\t?\t*\n"), "tabs not neutralised: {text}");
assert_eq!(text.lines().count(), 1, "one line per call: {text}");
}

#[test]
fn the_kill_switch_writes_nothing() {
let (_dir, repo) = scratch();
std::env::set_var("ACYCLIC_NO_LEASES", "1");
record_lease(&repo, Some("Edit"), Some("src/report.py"));
std::env::remove_var("ACYCLIC_NO_LEASES");
assert!(
leases_of(&repo).is_empty(),
"the off switch must be an off switch"
);
}

#[test]
fn leases_never_land_inside_the_repo() {
let (_dir, repo) = scratch();
record_lease(&repo, Some("Edit"), Some("src/report.py"));
// The regression this guards: writing into the tree woke the watcher,
// and the pre-tool hook then waited for the checkpoint its own write
// caused — 65ms to 120ms. It would also have shown up in every blast
// radius as a changed path.
assert!(
!repo.join(".speculation").exists(),
"a lease inside the repo is captured by the watcher and inflates \
every diff"
);
}

#[test]
fn appending_keeps_earlier_lines() {
let (_dir, repo) = scratch();
for path in ["a.py", "b.py", "c.py"] {
record_lease(&repo, Some("Edit"), Some(path));
}
// A reader takes the live window by timestamp, so history must not be
// truncated by a later write.
assert_eq!(leases_of(&repo).lines().count(), 3, "{}", leases_of(&repo));
}
}
16 changes: 14 additions & 2 deletions crates/acyclic/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ enum Command {
Turns {
#[arg(long)]
session: Option<String>,
/// Newest first, like `timeline --limit`. A long session has one turn
/// per prompt, so the default is the recent history rather than all of
/// it.
#[arg(long, default_value_t = 50)]
limit: usize,
},
/// One checkpoint resolved to its session, turn, and prompt.
Show { checkpoint: i64 },
Expand Down Expand Up @@ -669,7 +674,7 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
}
Ok(())
}
Command::Turns { session } => {
Command::Turns { session, limit } => {
let reply = client.call(proto::Op::Turns {
session_id: session,
})?;
Expand All @@ -680,7 +685,11 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
println!("no turns recorded (the user-prompt hook records them)");
return Ok(());
}
for turn in turns {
// Trimmed here rather than in the protocol: the daemon already
// answers with the session's turns, and a limit is a display
// concern. Newest first, matching `timeline`.
let hidden = turns.len().saturating_sub(limit);
for turn in turns.into_iter().take(limit) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Limit keeps oldest turns

The daemon returns turns oldest-first, so take(limit) retains the oldest entries and hides the newest ones when a session exceeds the limit. The footer then incorrectly calls the hidden turns “older.” Reverse the iterator before applying the limit, as the MCP implementation already does.

Suggested change
for turn in turns.into_iter().take(limit) {
for turn in turns.into_iter().rev().take(limit) {
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/acyclic/src/main.rs
Line: 687

Comment:
**Limit keeps oldest turns**

The daemon returns turns oldest-first, so `take(limit)` retains the oldest entries and hides the newest ones when a session exceeds the limit. The footer then incorrectly calls the hidden turns “older.” Reverse the iterator before applying the limit, as the MCP implementation already does.

```suggestion
            for turn in turns.into_iter().rev().take(limit) {
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

let range = match (turn.first_checkpoint, turn.last_checkpoint) {
(Some(first), Some(last)) if first != last => format!("#{first}..#{last}"),
(Some(first), _) => format!("#{first}"),
Expand All @@ -695,6 +704,9 @@ fn execute(client: &mut Client, command: Command) -> Result<(), String> {
brief::quote(&turn.prompt, 72),
);
}
if hidden > 0 {
println!("… {hidden} older turn(s) not shown (--limit)");
}
Ok(())
}
Command::Show { checkpoint } => {
Expand Down
3 changes: 3 additions & 0 deletions docs/design/03-forks.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ is what exists, including where it diverged from the plan.
## Notes

- Fork orchestration of subagents is uniquely plugin-shaped — it must live inside the host.
- `exclude` does not apply to paths created inside a fork: build output written into a mount is
captured in full and snapshotted by `promote`, which wedged a store in one live run. See
[fork-writes-bypass-exclude.md](fork-writes-bypass-exclude.md).
- 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.

## Open questions (not yet settled)
Expand Down
4 changes: 3 additions & 1 deletion docs/design/05-monorepo.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ which reads as though Launch 5 exists.
answer it gives will be silently incomplete rather than refused. *No lean.*
4. **Forks and Safe Mode sessions do not see excluded paths either**, which
bears directly on "searches against a fork must answer from that tree's
state". *No lean.*
state". *No lean.* The converse is now a known gap: paths *created* inside
a fork are captured regardless of `exclude` — see
[fork-writes-bypass-exclude.md](fork-writes-bypass-exclude.md).
5. **Does the name change?** Calling this "the index engine" collides with the
shipped metadata index. *Current lean: rename this launch, not the shipped
component.*
Expand Down
Loading
Loading