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
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ 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
A draft on a Git project also picks its **Git checkout** from the dropdown on the **Git status
row** above the composer: shared with the project, or a **Git worktree** of its own, so its file
changes never touch the project's checkout. Choosing a worktree while the project has uncommitted
work prints how much of it stays behind. 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
Expand Down
17 changes: 14 additions & 3 deletions crates/giskard-server/static/app.css
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,20 @@
.git-act:hover { border-color:var(--border); background:var(--panel2); color:var(--fg); }
.git-act[hidden] { display:none; }
.git-act-icon { display:inline-flex; align-items:center; padding:3px 5px; }
/* The draft's checkout choice, sized to sit among this row's actions rather than to look like a
form field: the row is 27px tall and every other control on it is a small muted chip. */
.git-strategy {
flex:none; max-width:9.5em; padding:1px 4px; border-radius:5px;
border:1px solid var(--border); background:var(--panel2); color:var(--muted);
font:inherit; font-size:11px; line-height:1.5;
}
.git-strategy:hover, .git-strategy:focus { color:var(--fg); border-color:var(--accent); }
/* Aligned under the row's text, not its caret, so it reads as a note on the row rather than as a
caption to the dropdown at the far end of it. */
.git-strategy-warning {
margin:0; padding:0 12px 5px 30px; color:var(--muted); font-size:11px; line-height:1.35;
}
.git-strategy-warning[hidden] { display:none; }

.git-line-body {
max-height:min(40vh, 320px); overflow-y:auto; padding:2px 12px 8px;
Expand Down Expand Up @@ -654,9 +668,6 @@
.mp-field[hidden] { display:none; } /* the grid display above would otherwise defeat [hidden] */
.mp-field > label, .mp-field > span { color:var(--muted); font-size:11px; }
.mp-field select { width:100%; }
/* The Git strategy choice and its explanation need the full width: the hint is a sentence, not a
label, and wrapping it into a column would make it unreadable at the popover's width. */
.mp-git-strategy { grid-column:1 / -1; }
.mp-hint { color:var(--muted); font-size:11px; line-height:1.35; }
.mp-hint:empty { display:none; }
/* A confirmation that names what it destroys, so the warning reads as part of the question
Expand Down
74 changes: 46 additions & 28 deletions crates/giskard-server/static/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -3271,12 +3271,11 @@ function resetTranscriptForAuthoritativeSnapshot() {
else setTurnActive(false);
}
const MODE_LABELS = { build:"Build", plan:"Plan" };
/* The `git_strategy` values `threads/start` accepts, and what each adds to the closed turn chip.
The default says nothing there — a chip that named the ordinary case on every thread would stop
being a signal. Keyed by wire value so this doubles as the set the UI will send: a strategy the
server does not know must not reach it. */
/* The `git_strategy` values `threads/start` accepts. This is the set the UI will send, so a strategy
the server does not know must not reach it; the names the user reads are the row's own `<option>`
elements. */
const GIT_STRATEGY_SHARED = "shared";
const GIT_STRATEGY_CHIP = { shared:"", worktree:" · Worktree" };
const GIT_STRATEGIES = new Set(["shared", "worktree"]);
const PERMISSION_PRESET_LABELS = {
ask:"Ask first",
read_only:"Ask first",
Expand All @@ -3296,49 +3295,68 @@ function updateTurnButton() {
const btn = $("turnPickerBtn"); if (!btn) return;
const mode = MODE_LABELS[state.mode] || "Build";
const preset = PERMISSION_PRESET_LABELS[state.permissionPreset] || "Ask first";
const strategy = isDraftThread() ? GIT_STRATEGY_CHIP[state.draftGitStrategy] || "" : "";
btn.querySelector(".mp-label").textContent = `${mode} · ${preset}${strategy}`;
// The checkout choice is not summarised here: it has its own control on the Git row, which reads
// its value outright, and repeating it would put the same fact in two places a hand's width apart.
btn.querySelector(".mp-label").textContent = `${mode} · ${preset}`;
renderGitStrategyControl();
}

/* The workspace choice belongs to a draft only: once a thread exists its workspace is fixed, so the
field is absent rather than shown disabled. Every strategy but the shared checkout also needs a
repository to branch from — without one there is nothing to isolate, and the reason is worth
saying rather than leaving a dead control. */
control is absent rather than shown disabled. Every strategy but the shared checkout also needs a
repository to branch from, and a workspace that is not one has no Git row to carry the control in
the first place. */
function isDraftWorkspaceRepo() {
return state.gitRepoByWorkspace.get(gitWorkspaceKey()) === true;
}

/* The control lives on the Git row, which is itself absent for a workspace that is not a repository
— so "there is nothing to branch from" needs no wording here: there is no row to say it on, and no
choice to make either. The two halves of the explanation land in different places, because they
are needed at different moments: what the option *is* is the select's `title`, and what it would
*cost* is printed under the row where a phone can read it. */
function renderGitStrategyControl() {
const control = $("gitStrategyControl");
const select = $("gitStrategySel");
if (!control || !select) return;
const draft = isDraftThread();
control.hidden = !draft;
if (!draft) return;
const warning = $("gitStrategyWarning");
if (!select || !warning) return;
const offered = isDraftThread() && isDraftWorkspaceRepo();

select.hidden = !offered;
select.title = offered ? gitStrategyHintText() : "";
if (offered) select.value = state.draftGitStrategy;

// Cleared as well as hidden: left in place it would be read out by the live region the next time
// the row came back, describing a decision that is no longer on the table.
const cost = offered ? gitStrategyWarningText() : "";
warning.textContent = cost;
warning.hidden = !cost;
}

const isRepo = isDraftWorkspaceRepo();
select.disabled = !isRepo;
select.value = isRepo ? state.draftGitStrategy : GIT_STRATEGY_SHARED;
$("gitStrategyHint").textContent = gitStrategyHintText(isRepo);
/* What the option *is*. Short enough for a tooltip, and true whether or not anything is at stake. */
function gitStrategyHintText() {
return state.draftGitStrategy === "worktree"
? "Starts from the last commit, on a branch of its own."
: "Shares the project's working tree with every other thread.";
}

function gitStrategyHintText(isRepo) {
if (!isRepo) return "This project is not a Git repository, so there is nothing to branch from.";
/* What the option would *cost*, which is why this one is printed rather than hovered: the row above
shows the project's changed-file count at this moment, so saying nothing would let the user send
expecting those changes to come along — and a tooltip cannot say it on a phone. Empty when there
is nothing to lose, so the row stays quiet in the ordinary case. */
function gitStrategyWarningText() {
if (state.draftGitStrategy !== "worktree") return "";
// The row directly above shows the project's changed-file count at this moment, so saying nothing
// would let the user send expecting those changes to come along. A worktree is the last commit,
// exactly.
const dirty = gitDirtyCount(state.gitStatus);
const base = "Starts from the last commit, on a branch of its own.";
if (!dirty) return base;
return `${base} Your ${dirty} uncommitted change${dirty === 1 ? "" : "s"} stay in the project's checkout.`;
if (!dirty) return "";
// The verb agrees with the count too: "1 uncommitted change stay" reads as a typo at exactly the
// moment the sentence is asking to be trusted about what it will not carry across.
return dirty === 1
? "Your 1 uncommitted change stays in the project's checkout."
: `Your ${dirty} uncommitted changes stay in the project's checkout.`;
}

function setDraftGitStrategy(strategy) {
// Anything the server would reject is not worth holding: an unknown value here could only come
// from a stale option, and defaulting to the shared checkout is the choice that changes nothing.
state.draftGitStrategy = GIT_STRATEGY_CHIP[strategy] === undefined ? GIT_STRATEGY_SHARED : strategy;
state.draftGitStrategy = GIT_STRATEGIES.has(strategy) ? strategy : GIT_STRATEGY_SHARED;
updateTurnButton();
}
function setMode(mode) {
Expand Down
34 changes: 20 additions & 14 deletions crates/giskard-server/static/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,20 @@ <h1>Giskard</h1>
<span class="git-diffstat mono" id="gitDiffstat" hidden></span>
</button>
<div class="git-line-actions">
<!-- Only a draft can choose this: a thread's workspace is fixed once it exists, so the
control is absent for open threads rather than shown disabled. It sits on this row
because this row describes the very tree the choice is about — the branch a worktree
would start from, and the changed files it would leave behind. A select rather than
a checkbox because where a thread's working tree comes from is an open question, not
a yes/no one: a third answer is a new option here rather than a new control. The
row has no width for a sentence, so what an option *is* rides on the select's
`title`, refreshed per choice; `aria-label` names the control itself, since the row
has no room for a visible label either. What an option would *cost* is printed
below instead — see `gitStrategyWarning`. -->
<select id="gitStrategySel" class="git-strategy" aria-label="Git checkout" hidden>
<option value="shared">Shared</option>
<option value="worktree">Worktree</option>
</select>
<button type="button" class="git-act" id="gitReviewAll" hidden
title="Open the whole working-tree diff">Review all</button>
<button type="button" class="git-act git-act-icon" id="gitRefresh"
Expand All @@ -172,6 +186,12 @@ <h1>Giskard</h1>
</button>
</div>
</div>
<!-- What the chosen checkout would leave behind, printed rather than hovered: it is the one
fact a user needs *before* sending, and a tooltip does not exist on a phone. It appears
only when there is something to lose — a worktree with uncommitted work in the project's
checkout — so the row stays quiet in the ordinary case. Announced politely because it
appears in response to choosing, away from where the eye is. -->
<p class="git-strategy-warning" id="gitStrategyWarning" role="status" aria-live="polite" hidden></p>
<div class="git-line-body" id="gitLineBody" hidden></div>
</div>
<div class="composer" id="composer" style="display:none">
Expand Down Expand Up @@ -215,20 +235,6 @@ <h1>Giskard</h1>
<option value="full_access">⚠ Full Access</option>
</select>
</div>
<!-- Only a draft can choose this: a thread's workspace is fixed once it exists, so the
field is absent for open threads rather than shown disabled. A select rather than a
checkbox because where a thread's working tree comes from is an open question, not
a yes/no one — a third answer is a new option here rather than a new control. The
hint carries what the labels cannot: a new worktree is the last commit, exactly, so
work in progress in the project's checkout stays there. -->
<div class="mp-field mp-git-strategy" id="gitStrategyControl" hidden>
<label for="gitStrategySel">Git checkout</label>
<select id="gitStrategySel" aria-describedby="gitStrategyHint">
<option value="shared">Shared with the project</option>
<option value="worktree">Its own worktree</option>
</select>
<span class="mp-hint" id="gitStrategyHint"></span>
</div>
</div>
</div>
<div class="picker picker-right" id="modelPicker">
Expand Down
93 changes: 77 additions & 16 deletions crates/giskard-server/tests/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2933,31 +2933,72 @@ fn app_js() -> &'static str {
include_str!("../static/app.js")
}

/// Where a thread's working tree comes from is decided once, on the draft, and the choice has to be
/// visible without reopening the popover that set it.
/// Where a thread's working tree comes from is decided once, on the draft. The choice belongs to the
/// Git row — the row that describes the very tree it changes — rather than to the mode/permissions
/// popover, where it was filed under a card about neither.
#[test]
fn browser_offers_the_git_strategy_choice_on_drafts_only() {
let source = app_js();
let markup = include_str!("../static/index.html");

let row = between(
markup,
"class=\"git-line-actions\"",
"class=\"git-line-body\"",
);
assert!(
row.contains("id=\"gitStrategySel\""),
"the checkout choice sits on the Git row, beside the branch and counts it changes"
);
let turn_picker = between(markup, "id=\"turnPickerMenu\"", "id=\"modelPicker\"");
assert!(
markup.contains("id=\"gitStrategyControl\"") && markup.contains("id=\"gitStrategySel\""),
"the draft picker carries the Git strategy choice"
!turn_picker.contains("gitStrategy"),
"and no longer in the mode/permission card, which is about neither"
);
// A select, not a checkbox: the set of strategies is open, and the next one has to be an option
// here rather than a second control.
// here rather than a second control. It is also the control itself rather than a chip that opens
// one — the choice is one click from the row, not two.
assert!(
row.contains("<option value=\"shared\"") && row.contains("<option value=\"worktree\""),
"both strategies are offered by their wire value, on the row"
);
// The row is 27px of chips with no width for a sentence, so what an option *is* rides on the
// select's own tooltip, refreshed per choice.
assert!(
source.contains("select.title = offered ? gitStrategyHintText() : \"\";"),
"what the option is, is the control's tooltip rather than a line on the row"
);
// What it would *cost* is printed instead: a tooltip does not exist on a phone, and this is the
// one fact a user needs before sending.
assert!(
between(markup, "class=\"git-line-head\"", "class=\"git-line-body\"")
.contains("id=\"gitStrategyWarning\""),
"the cost is printed on its own line under the row, where touch can read it"
);
assert!(
markup.contains("<option value=\"shared\"")
&& markup.contains("<option value=\"worktree\""),
"both strategies are offered by their wire value"
markup.contains("id=\"gitStrategyWarning\" role=\"status\" aria-live=\"polite\""),
"and announced, since it appears in response to a choice made elsewhere on the row"
);
// Pinned as the adjacent pair rather than as one line: a lone `contains` of the assignment still
// matches when a guard is prepended to it, which is exactly the regression that would leave a
// stale cost sitting in the live region.
assert!(
source.contains("control.hidden = !draft;"),
"a thread's workspace is fixed once it exists, so the field is absent for open threads"
source.contains(concat!(
" const cost = offered ? gitStrategyWarningText() : \"\";\n",
" warning.textContent = cost;\n",
" warning.hidden = !cost;"
)),
"the line is cleared as well as hidden, so a stale cost is never announced later"
);
// No visible label fits either, so the control still has to name itself to a screen reader.
assert!(
source.contains("select.disabled = !isRepo;"),
"there is nothing to branch from without a repository"
row.contains("aria-label=\"Git checkout\""),
"the control names itself where no visible label fits"
);
assert!(
source.contains("const offered = isDraftThread() && isDraftWorkspaceRepo();"),
"a thread's workspace is fixed once it exists, and a non-repository has nothing to branch \
from — so the control is absent in both cases rather than shown dead"
);
assert!(
source.contains(
Expand All @@ -2971,12 +3012,32 @@ fn browser_offers_the_git_strategy_choice_on_drafts_only() {
"each draft chooses for itself rather than inheriting the last draft's choice"
);
assert!(
source.contains("GIT_STRATEGY_CHIP[state.draftGitStrategy]"),
"the closed chip reports the choice"
source.contains("GIT_STRATEGIES.has(strategy) ? strategy : GIT_STRATEGY_SHARED"),
"a value the server would refuse never becomes the draft's choice"
);
// Pinned positively rather than as the absence of the old suffix: "no longer contains that exact
// fragment" would pass again the moment a suffix came back spelled any other way.
assert!(
source.contains("textContent = `${mode} · ${preset}`;"),
"and the turn chip summarises mode and preset only — the checkout choice is on the row, and \
one fact belongs in one place"
);
assert!(
source.contains("Your 1 uncommitted change stays in the project's checkout.")
&& source.contains("uncommitted changes stay in the project's checkout."),
"the warning names the work that stays behind, with the verb agreeing with the count"
);
// Nothing is at stake without a worktree, or without uncommitted work, and a line that appeared
// every time would stop being read.
let warning = between(
source,
"function gitStrategyWarningText()",
"function setDraftGitStrategy",
);
assert!(
source.contains("Your ${dirty} uncommitted change"),
"the hint names the work that stays in the project's checkout"
warning.contains("if (state.draftGitStrategy !== \"worktree\") return \"\";")
&& warning.contains("if (!dirty) return \"\";"),
"and it is absent when there is nothing to lose"
);
}

Expand Down
Loading
Loading