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
8 changes: 8 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ README. Update that file in the same change whenever you add, remove, or change
route in `crates/giskard-server/src/routes.rs` (path, method, request/response shape, or documented
behavior) so it never drifts from the code.

Per-thread Git worktrees are documented in `docs/git-worktrees.md`, linked from the README and
referenced from spec §7.1. Keep it synchronized with `crates/giskard-server/src/worktree.rs` —
especially the branch/path naming, the boundary between what a worktree isolates and what it shares
with the project's repository, and what archive, delete and project delete do to a worktree and its
branch. The documentation is the mitigation for this feature's
sharpest edge (work that exists only inside a worktree), so a behavior change that is not reflected
there is not finished.

## Build & Test

```bash
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ Then open **http://127.0.0.1:8787**, log in, and:
server machine (the agent's workspace).
2. **+** on the project → draft a new thread. No Codex thread is created until the first message is
sent, so choose the **Plan/Build** mode, **permission preset**, and **model** first if needed.
A draft on a Git project also picks its **Git checkout**: shared with the project, or a **Git
worktree** of its own, so its file changes never touch the project's checkout. The choice is
available only on the draft — the workspace is fixed once the thread exists. Isolation changes where the thread works, not what it
is allowed to do: its permission preset still applies unchanged. See
[Per-thread Git worktrees](docs/git-worktrees.md) for what does and does not come across, branch
naming, and what deleting an isolated thread destroys.
3. Type in the composer (Enter to send). Use the attachment button or drop files onto the composer
to include images, PDFs, or other files with the message. A message accepts up to eight files
and 25 MiB total. The first send creates the Codex thread with the selected
Expand All @@ -124,6 +130,8 @@ Then open **http://127.0.0.1:8787**, log in, and:
Git repository, a one-line **Git status** sits just above the composer — branch, ahead/behind,
changed-file count and total diffstat — and expands in place into the changed files, each
opening its diff. It refreshes as the agent changes the tree, so it stays current during a turn.
For a thread isolated in a worktree, the row reports that worktree rather than the project's
checkout.
4. Linked child threads appear in the **Sub-agents** monitor and can be opened from their activity
rows; their header **Parent** button returns to the owning thread. See
[Sub-agent threads](docs/subagents.md) for spawning protocols, monitoring, prompts, direct
Expand Down Expand Up @@ -298,6 +306,7 @@ $GISKARD_DATA_DIR/
│ ├── threads/
│ │ ├── <thread_id>.json # thread metadata, permission preset, token cache
│ │ └── <thread_id>.jsonl # authoritative turn history — one Turn per line, append-only
│ ├── worktrees/<thread_id>/ # Git worktree for a thread started isolated (docs/git-worktrees.md)
│ └── tokens.json # per-project token ledger (total, by_day, by_model)
└── tokens-global.json # cross-project token ledger
```
Expand Down
60 changes: 60 additions & 0 deletions crates/giskard-git-parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,31 @@ pub fn apply_numstat_counts(
status.deleted_total = deleted_total;
}

/// Read the commit a deleted branch pointed at out of `git branch -d`/`-D`'s own report.
///
/// Unlike the other formats here this one is *porcelain* — text written for a person, of the shape
/// `Deleted branch <name> (was <sha>).` Git offers no machine-readable form of it, and the sha is
/// worth having: it is the only record of where the branch was, and it makes the reflog window
/// usable after a thread is deleted. So this parses it, narrowly, and answers `None` for anything
/// that does not match rather than guessing.
///
/// Being porcelain, it is also translated. Callers must run Git with `LC_ALL=C`; nothing here can
/// check that for them.
pub fn parse_deleted_branch_sha(stdout: &str) -> Option<String> {
let (_, rest) = stdout.split_once("(was ")?;
let (sha, _) = rest.split_once(')')?;
let sha = sha.trim();
// Git fills this slot with whatever the ref pointed at, and for a symbolic ref that is another
// ref's name rather than a commit — `Deleted branch sym (was refs/heads/main).`. Returning it
// would put a non-commit where callers report "the commit it pointed at", and send anyone
// following the log to a reflog lookup that cannot succeed. Only a hex object name counts.
if sha.len() >= 4 && sha.chars().all(|c| c.is_ascii_hexdigit()) {
Some(sha.to_string())
} else {
None
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -812,4 +837,39 @@ mod tests {
assert_eq!(status.added_total, 10);
assert_eq!(status.deleted_total, 5);
}

/// Git's own wording, and the shapes that are not it.
#[test]
fn reads_the_deleted_branch_sha_from_gits_wording() {
assert_eq!(
parse_deleted_branch_sha("Deleted branch giskard/worktree-04demo (was e17b742).\n"),
Some("e17b742".to_string())
);
// A branch name containing the marker must not be mistaken for the report's own. The first
// marker still wins — which is why the caller passes only this command's output — but what
// it yields is not an object name, so the report is refused rather than guessed at.
assert_eq!(
parse_deleted_branch_sha("Deleted branch feature/(was x) (was abc1234).\n"),
None
);
assert_eq!(parse_deleted_branch_sha("nothing to report"), None);
assert_eq!(parse_deleted_branch_sha("Deleted branch x (was )."), None);
assert_eq!(parse_deleted_branch_sha("Deleted branch x (was abc"), None);
}

/// Deleting a symbolic ref succeeds and reports the ref it pointed at, not a commit. Handing
/// that back would be a non-commit in the one field that records where the branch was.
#[test]
fn refuses_a_symbolic_refs_target_as_the_commit() {
assert_eq!(
parse_deleted_branch_sha("Deleted branch sym (was refs/heads/main).\n"),
None
);
// Nor does a short-but-hex-looking fragment pass: Git abbreviates to at least four.
assert_eq!(parse_deleted_branch_sha("Deleted branch x (was ab)."), None);
assert_eq!(
parse_deleted_branch_sha("Deleted branch x (was abcd)."),
Some("abcd".to_string())
);
}
}
1 change: 1 addition & 0 deletions crates/giskard-harness-replay/tests/replay_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ async fn replay_persisted_state_roundtrip() {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
};

store.save_thread(pid, &thread_file).await.unwrap();
Expand Down
130 changes: 130 additions & 0 deletions crates/giskard-persist/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,96 @@ pub struct ThreadFile {
pub updated_at: DateTime<Utc>,
#[serde(default, skip_serializing_if = "is_false")]
pub archived: bool,
/// The Git workspace this thread was created with, when it was created with one. Absent for
/// ordinary threads, which work in the project's own workspace.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub git_workspace: Option<ThreadGitWorkspace>,
}

/// A Git workspace a thread owns, tagged by the strategy that produced it.
///
/// Tagged rather than a bare struct because the strategies are meant to *coexist*: a worktree and a
/// thread-owned repository answer different needs — one shares the project's history and delivers
/// work by simply existing, the other trades that for a boundary — and a user picking per thread
/// wants both available. So this is not a placeholder for one shape replacing another. A new
/// strategy is a new variant, old records keep parsing because their tag still names their own
/// variant, and nothing has to be migrated.
///
/// The tag values match `GitStrategy` on the wire, so a record says which choice produced it in the
/// same vocabulary the request used.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
#[serde(tag = "strategy", rename_all = "snake_case")]
pub enum ThreadGitWorkspace {
Worktree(ThreadWorktree),
}

impl ThreadGitWorkspace {
/// Where the thread works — the one question every strategy must answer, and the only thing
/// most callers need. Everything else about a workspace is strategy-specific.
pub fn workspace_root(&self) -> &str {
match self {
Self::Worktree(worktree) => worktree.workspace_root(),
}
}

/// The worktree behind this workspace, if that is what it is.
///
/// Deliberately fallible rather than a field: creation, removal and deletion impact are
/// strategy-specific, so a caller doing one of those has to say which strategy it handles and
/// what it does about the others.
pub fn as_worktree(&self) -> Option<&ThreadWorktree> {
match self {
Self::Worktree(worktree) => Some(worktree),
}
}
}

/// A linked Git worktree owned by one thread, so its file changes never touch the project's
/// checkout or another thread's.
///
/// The Git directories are recorded rather than derived: `<workspace>/.git` is only the repository
/// when the project directory is an ordinary checkout, and is a pointer *file* when the project is
/// itself a linked worktree. Both paths come from `git rev-parse` inside the worktree at creation.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct ThreadWorktree {
/// Absolute path of the worktree — the checkout Git created, and what `git worktree remove`
/// and the impact probes operate on. Not necessarily where the thread works: see `workspace`.
pub path: String,
/// Absolute path the thread actually works in, and what every path resolves against.
///
/// Equal to `path` when the project directory is the repository's top level. When the project
/// is rooted in a *subdirectory* of its repository, Git can only check out the whole repository,
/// so the worktree is its root while the thread works in the same subdirectory beneath it —
/// otherwise an isolated thread would silently work one or more levels above the directory the
/// project scoped it to, and a path would name a different file than it does for an ordinary
/// thread of the same project.
///
/// Absent for the top-level case, which is the common one; read it through
/// [`ThreadWorktree::workspace_root`] rather than directly.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub workspace: Option<String>,
/// The branch created for this thread. Only its *starting* branch: the agent may switch away,
/// so this names what to clean up, never what is currently checked out.
pub branch: String,
/// The commit the branch started from, or `None` in a repository with no commits, where the
/// worktree is created on an orphan branch and there is nothing to count from.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub base_commit: Option<String>,
/// The checkout that owns the worktree — where `git worktree remove` and `git branch -D` run.
pub repo_root: String,
/// `git rev-parse --git-common-dir`: the shared repository (objects, refs, config, hooks).
pub common_dir: String,
/// `git rev-parse --git-dir`: this worktree's private directory (its index, HEAD, reflog).
pub git_dir: String,
}

impl ThreadWorktree {
/// Where the thread works. The whole point of the pair: `path` is the checkout Git manages,
/// this is the directory inside it that stands in for the project's own.
pub fn workspace_root(&self) -> &str {
self.workspace.as_deref().unwrap_or(&self.path)
}
}

fn is_false(value: &bool) -> bool {
Expand Down Expand Up @@ -849,6 +939,39 @@ impl PersistStore {
}
}

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

/// The tag is what makes another strategy additive rather than a migration, so it has to be
/// written, and the variant's own fields have to survive sitting beside it — `ThreadWorktree`
/// denies unknown fields, and an internally tagged enum is exactly where that can go wrong.
#[test]
fn git_workspace_round_trips_with_its_strategy_tag() {
let workspace = ThreadGitWorkspace::Worktree(ThreadWorktree {
path: "/data/wt".into(),
workspace: Some("/data/wt/packages/api".into()),
branch: "giskard/worktree-01test".into(),
base_commit: Some("e17b742".into()),
repo_root: "/home/me/project".into(),
common_dir: "/home/me/project/.git".into(),
git_dir: "/home/me/project/.git/worktrees/t".into(),
});

let json = serde_json::to_value(&workspace).unwrap();
assert_eq!(
json["strategy"], "worktree",
"the record names the strategy that produced it"
);
assert_eq!(json["path"], "/data/wt");
assert_eq!(
serde_json::from_value::<ThreadGitWorkspace>(json).unwrap(),
workspace,
"and reads back as the same variant"
);
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -957,6 +1080,7 @@ mod tests {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
};
store.save_thread(pid, &thread).await.unwrap();

Expand Down Expand Up @@ -1002,6 +1126,7 @@ mod tests {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
};
let mut value = serde_json::to_value(&thread).unwrap();
let object = value.as_object_mut().unwrap();
Expand Down Expand Up @@ -1057,6 +1182,7 @@ mod tests {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
};
let mut value = serde_json::to_value(&thread).unwrap();
value.as_object_mut().unwrap().remove("permission_preset");
Expand Down Expand Up @@ -1106,6 +1232,7 @@ mod tests {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
};
store.save_thread(pid, &thread).await.unwrap();
}
Expand Down Expand Up @@ -1281,6 +1408,7 @@ mod tests {
created_at: Utc::now(),
updated_at: Utc::now(),
archived: false,
git_workspace: None,
},
)
.await
Expand Down Expand Up @@ -1461,6 +1589,7 @@ mod tests {
created_at: Utc::now(),
updated_at: Utc::now(),
archived: false,
git_workspace: None,
},
)
.await
Expand Down Expand Up @@ -1503,6 +1632,7 @@ mod tests {
created_at: now,
updated_at: now,
archived: false,
git_workspace: None,
},
)
.await
Expand Down
1 change: 1 addition & 0 deletions crates/giskard-persist/tests/giskard_admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ fn test_thread(
created_at: now,
updated_at: now,
archived,
git_workspace: None,
}
}

Expand Down
49 changes: 49 additions & 0 deletions crates/giskard-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,27 @@ pub struct GitDiffResponse {
pub is_empty: bool,
}

/// What deleting a thread would destroy, per worktree in the subtree it cascades to (spec §7.1).
#[derive(Debug, Clone, Serialize)]
pub struct ThreadDeletionImpactResponse {
/// Empty when no thread in the subtree has a worktree, which is the ordinary case.
pub worktrees: Vec<WorktreeImpactResponse>,
}

#[derive(Debug, Clone, Serialize)]
pub struct WorktreeImpactResponse {
pub thread_id: ThreadId,
pub branch: String,
/// Modified or untracked files in the worktree. Ignored files are excluded: they do not block
/// removal and are not work.
pub uncommitted_changes: usize,
/// Commits on the thread's branch that no other ref reaches, which deleting it destroys.
pub unreachable_commits: usize,
/// A sentence naming what would be lost, or `None` when nothing would be.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub summary: Option<String>,
}

#[derive(Debug, Clone, Serialize)]
pub struct ListProjectsResponse {
pub projects: Vec<ProjectSummary>,
Expand Down Expand Up @@ -476,6 +497,34 @@ pub struct StartThreadRequest {
pub model_ref: ModelRef,
pub mode: Mode,
pub permission_preset: PermissionPreset,
/// How this thread gets the working tree it runs in (spec §7.1).
///
/// Only creation carries this: a thread's workspace is fixed once it exists, so there is no
/// endpoint that changes it afterwards.
#[serde(default)]
pub git_strategy: GitStrategy,
}

/// Where a thread's working tree comes from.
///
/// An enum rather than a flag because the question has more than two answers and the set is open —
/// giving a thread a checkout it genuinely owns, rather than a second view of the project's
/// repository, is a different strategy again. A boolean could never carry a third choice, and a
/// client that had learned to send `true` could not be told about one.
///
/// Serde rejects a variant it does not know, so a client asking for a strategy this server does not
/// implement is refused rather than quietly started in the shared checkout — which is the failure
/// worth designing against here, since it looks like it worked.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum GitStrategy {
/// The project's own checkout, shared with every other thread of the project. The default, and
/// the only possibility when the workspace is not a Git repository.
#[default]
Shared,
/// A linked Git worktree of the project's repository, private to this thread and the sub-agents
/// it spawns. Isolates files; shares the repository (`docs/git-worktrees.md`).
Worktree,
}

#[derive(Debug, Clone, Serialize)]
Expand Down
1 change: 1 addition & 0 deletions crates/giskard-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub mod running_commands;
mod thread_graph;
pub mod throttle;
pub mod tokens;
pub mod worktree;

pub use app::{AppState, build_app};
pub use registry::{HarnessFactory, HarnessRegistry};
Loading
Loading