feat(overlay): grey out the git verbs, with the reason on hover

Commit, Commit & push and Create pull request were offered whatever the state
of git, so the first thing a user in a broken repository learned was a toast
after the click. The daemon now reports git health on connect and after every
turn, and the turn menu greys them with the reason attached.

Greyed rather than hidden: a Commit that is simply missing reads as a bug in
the editor, and one that fails after the click has already cost the click.

Two gates, because the two failures are unrelated. Revert rewrites files from
the before-state the turn captured and needs no git at all, so it is greyed
only when this turn has files whose baseline was never captured — a fact the
bundle already carries on `noBaseline`. Commit and Create pull request need
git, equally for every turn.

`git:health` is sent beside the handshake and before `job:done`, not after: that
event is what paints the finished turn, and the menu captures the health it was
rendered with. An absent report reads as healthy, so an older daemon that never
sends one keeps its buttons.

A disabled row that carries a reason had to become hoverable to show it.
Browsers dispatch no pointer events from a disabled form control, and `Tooltips`
matches on `pointerover`, so `data-tip` on one is never read. The greying comes
from `[aria-disabled="true"]` in pop.css.ts rather than from `:disabled`, which
is what makes dropping the attribute free; `tabindex="-1"` keeps the row out of
the tab order the attribute used to hold it out of, and the reason rides the
accessible name too, since `aria-disabled` alone announces that a row is
unavailable and never why. Rows with no reason to give are unchanged.

The tooltip carries the reason and not the hint. `GitStatus.hint` is a command
to run, sized for a terminal, and a tip clamps at three lines — the banner and
`airship doctor` are where the fix gets spelled out. Every reason is inside the
44-character budget tooltip.copy.test.ts enforces, which is why `GitStatus`
carries no path and the banner adds the directory itself.
This commit is contained in:
Nayan
2026-08-16 12:16:20 +05:30
parent 448409f473
commit f256dc7afa
5 changed files with 286 additions and 10 deletions
+16
View File
@@ -7,6 +7,7 @@ import {
type CreateJobRequest,
type Editor,
type ElementContext,
type GitHealth,
type ImageInput,
type JobDiffBundle,
type JobHistorySummary,
@@ -631,6 +632,14 @@ export class AirshipApp {
private redoBtn!: HTMLButtonElement;
private tooltips: Tooltips | null = null;
/**
* Whether the daemon's git works, so the turn menu can grey its git verbs
* with the reason attached. Undefined until the first `git:health` arrives,
* which reads as healthy — an older daemon never sends one, and hiding a
* working Commit is worse than offering one that might fail.
*/
private gitHealth: GitHealth | undefined;
private selected: Selection | null = null;
private images: ImageInput[] = [];
private activeJobId: string | null = null;
@@ -4027,6 +4036,12 @@ export class AirshipApp {
this.previewKey = "";
this.schedulePreview();
break;
case "git:health":
// Stored, not rendered: the turn menu is built on open, so the next
// time it opens it reads this. Nothing on screen depends on it until
// then, which is why there is no repaint here.
this.gitHealth = ev.health;
break;
case "job:created":
if (this.awaiting && !this.activeJobId) {
this.activeJobId = ev.job.jobId;
@@ -4238,6 +4253,7 @@ export class AirshipApp {
private assistantActions(bundle: JobDiffBundle): AssistantActions {
const canRevert = bundle.status === "done" && bundle.diffs.length > 0;
return {
git: this.gitHealth,
onBranch:
bundle.status === "done" ? () => this.branch(bundle.jobId) : undefined,
onComment: canRevert
+56 -9
View File
@@ -1,4 +1,4 @@
import type { Editor, JobDiffBundle } from "@airship/protocol";
import type { Editor, GitHealth, JobDiffBundle } from "@airship/protocol";
import { firstHunkLine, renderDiff, selectedLineRange } from "../diff-view";
import { clear, cls, el } from "../dom";
import { icon } from "../icons";
@@ -9,6 +9,16 @@ import { type TimelineView, timelineView } from "./timeline";
/** Callbacks a finished assistant turn can offer. Omit any to hide its entry. */
export interface AssistantActions {
/**
* Whether git works, from the daemon.
*
* Present for the git-backed rows, which are shown greyed with the reason
* rather than hidden: a Commit that is simply missing reads as a bug, and a
* Commit that fails after the click has already cost the user the click.
* Absent is treated as healthy, which is what an older daemon that never
* sends `git:health` should look like.
*/
git?: GitHealth;
onBranch?: () => void;
onComment?: (file: string, body: HTMLElement) => void;
onCommit?: (push: boolean) => void;
@@ -334,18 +344,30 @@ function turnMenuButton(
return btn;
}
/** The turn kebab's contents, grouped. Exported for the same reason it is
* separate: the shape of this menu is a product decision worth reading. */
export function turnMenu(
/**
* The "This change" rows: revert, commit, and open a pull request.
*
* Separate from `turnMenu` because all three carry an availability gate, and
* there are two different gates. Revert does not need git at all — it rewrites
* files from the before-state the turn captured — so it is greyed only when
* this turn has files whose baseline was never captured, which is a fact the
* bundle already carries. Commit and Create pull request do need git, equally
* for every turn, so they read the daemon's health report.
*
* Greyed with a reason rather than hidden: a Commit that is simply missing
* reads as a bug, and one that fails after the click has already cost the click.
*/
function changeRows(
bundle: JobDiffBundle,
actions: AssistantActions,
anchor: HTMLElement
): MenuEntry[] {
const out: MenuEntry[] = [];
const file = bundle.diffs?.[0]?.file ?? bundle.target?.source?.file;
const line = bundle.diffs?.[0]
? firstHunkLine(bundle.diffs[0].patch)
: (bundle.target?.source?.line ?? undefined);
// The reason only, never the hint: `GitStatus.hint` is a command to run, sized
// for a terminal, and a tooltip clamps at three lines. The banner and `airship
// doctor` are where the fix gets spelled out.
const gitBroken = actions.git && !actions.git.ok;
const gitTip = gitBroken ? actions.git?.reason : undefined;
const unrestorable = bundle.diffs?.some((d) => d.noBaseline) ?? false;
const change: MenuEntry[] = [];
if (actions.onUndo) {
@@ -358,23 +380,29 @@ export function turnMenu(
// direct-manipulation stack. `app.ts` says in as many words that the two
// must never be wired together — and this menu was telling the user they
// were. There is no chord for this, so it advertises none.
disabled: unrestorable,
icon: "rotate-ccw",
label: "Revert this change",
run: actions.onUndo,
tip: unrestorable ? "No previous content to restore from" : undefined,
});
}
const { onCommit } = actions;
if (onCommit) {
change.push(
{
disabled: gitBroken,
icon: "version-current",
label: "Commit to git",
run: () => onCommit(false),
tip: gitTip,
},
{
disabled: gitBroken,
icon: "version-merged",
label: "Commit & push",
run: () => onCommit(true),
tip: gitTip,
}
);
}
@@ -382,6 +410,7 @@ export function turnMenu(
if (onCreatePr) {
const files = bundle.diffs?.length ?? 0;
change.push({
disabled: gitBroken,
icon: "version-branch",
label: "Create pull request…",
// Pushing is the only thing in this application that cannot be taken
@@ -397,8 +426,26 @@ export function turnMenu(
run: onCreatePr,
},
]).open(anchor, "above"),
tip: gitTip,
});
}
return change;
}
/** The turn kebab's contents, grouped. Exported for the same reason it is
* separate: the shape of this menu is a product decision worth reading. */
export function turnMenu(
bundle: JobDiffBundle,
actions: AssistantActions,
anchor: HTMLElement
): MenuEntry[] {
const out: MenuEntry[] = [];
const file = bundle.diffs?.[0]?.file ?? bundle.target?.source?.file;
const line = bundle.diffs?.[0]
? firstHunkLine(bundle.diffs[0].patch)
: (bundle.target?.source?.line ?? undefined);
const change = changeRows(bundle, actions, anchor);
if (change.length) {
out.push({ header: "This change" }, ...change);
}
+125
View File
@@ -0,0 +1,125 @@
/**
* The turn kebab's git verbs, and when they are offered.
*
* Greyed with the reason rather than hidden. A Commit that is simply missing
* reads as a bug in the editor; a Commit that is offered and then fails has
* already cost the user the click and told them nothing they can act on.
*
* Two gates, because the two failures are unrelated. Revert rewrites files from
* the before-state the turn captured and needs no git at all, so it is greyed
* only when *this* turn has files whose baseline was never captured. Commit and
* Create pull request need git for every turn equally.
*/
import type { JobDiffBundle } from "@airship/protocol";
import { describe, expect, it, vi } from "vitest";
import { el } from "../dom";
import { isMenuItem, type MenuItem } from "../popover-host";
import { type AssistantActions, turnMenu } from "./transcript";
function bundleWith(noBaseline: boolean): JobDiffBundle {
return {
agent: "codex",
createdAt: 0,
diffs: [
{
additions: 1,
deletions: 0,
file: "src/app.ts",
isDeleted: false,
isNew: false,
...(noBaseline ? { noBaseline: true } : {}),
patch: "",
},
],
filesChanged: 1,
jobId: "job-1",
prompt: "make it blue",
promptPreview: "make it blue",
status: "done",
target: {},
} as JobDiffBundle;
}
const ALL_ACTIONS: AssistantActions = {
onBranch: vi.fn(),
onCommit: vi.fn(),
onCreatePr: vi.fn(),
onUndo: vi.fn(),
};
function rowsFor(
actions: Partial<AssistantActions>,
noBaseline = false
): MenuItem[] {
return turnMenu(
bundleWith(noBaseline),
{ ...ALL_ACTIONS, ...actions },
el("button")
).filter(isMenuItem);
}
const row = (rows: MenuItem[], label: string): MenuItem =>
rows.find((r) => r.label === label) as MenuItem;
const GIT_ROWS = ["Commit to git", "Commit & push", "Create pull request…"];
describe("turnMenu git verbs", () => {
it("offers every git row when git is healthy", () => {
const rows = rowsFor({ git: { ok: true } });
for (const label of GIT_ROWS) {
expect(row(rows, label).disabled).toBeFalsy();
expect(row(rows, label).tip).toBeUndefined();
}
});
it("greys every git row with the reason when git is broken", () => {
const rows = rowsFor({
git: {
hint: "Run `git init` there.",
ok: false,
reason: "not a git repository",
},
});
for (const label of GIT_ROWS) {
expect(row(rows, label).disabled).toBe(true);
expect(row(rows, label).tip).toBe("not a git repository");
}
});
it("keeps the hint out of the tooltip", () => {
// `GitStatus.hint` is a command to run, sized for a terminal. The tooltip
// clamps at three lines, so the banner and `airship doctor` carry the fix.
const rows = rowsFor({
git: {
hint: "Run `git init` there.",
ok: false,
reason: "not a git repository",
},
});
expect(row(rows, "Commit to git").tip).not.toContain("git init");
});
it("treats an absent health report as healthy", () => {
// An older daemon never sends `git:health`. Hiding a working Commit is
// worse than offering one that might fail.
expect(row(rowsFor({}), "Commit to git").disabled).toBeFalsy();
});
it("leaves Revert alone when only git is broken", () => {
// Revert is pure filesystem. On the Claude path it works with no git at all.
const rows = rowsFor({
git: { ok: false, reason: "not a git repository" },
});
expect(row(rows, "Revert this change").disabled).toBeFalsy();
});
it("greys Revert when the turn captured no baseline", () => {
const rows = rowsFor({ git: { ok: true } }, true);
const revert = row(rows, "Revert this change");
expect(revert.disabled).toBe(true);
expect(revert.tip).toBe("No previous content to restore from");
// The git rows are unaffected: git works, so committing still does.
expect(row(rows, "Commit to git").disabled).toBeFalsy();
});
});
+61
View File
@@ -5,6 +5,7 @@ import {
closeOpenPopover,
createMenu,
type MenuGroup,
type MenuItem,
mountPopoverHost,
openPopover,
type PopoverHandle,
@@ -528,6 +529,66 @@ describe("a menu with collapsible groups", () => {
expect(shells()[0].querySelector(".custom-form")).toBe(custom);
});
/*
* A greyed row that carries a reason has to stay hoverable.
*
* Browsers do not dispatch pointer events from a `disabled` form control, and
* `Tooltips` matches on `pointerover`, so `data-tip` on one is never read. The
* greying comes from `[aria-disabled="true"]` in `pop.css.ts` rather than from
* `:disabled`, which is what makes dropping the attribute free.
*/
describe("a disabled row", () => {
const rowFor = (item: MenuItem): HTMLElement => {
createMenu([item]).open(anchor(), "below");
return shells()[0].querySelector<HTMLElement>(
`.${cls("pop-item")}`
) as HTMLElement;
};
it("stays hoverable, and out of both cursors, when it has a tip", () => {
const row = rowFor({
disabled: true,
label: "Commit to git",
run: () => undefined,
tip: "not a git repository",
});
expect(row.getAttribute("aria-disabled")).toBe("true");
expect(row.hasAttribute("disabled")).toBe(false);
expect(row.dataset.tip).toBe("not a git repository");
// Neither the roving cursor nor Tab reaches it.
expect(row.hasAttribute("data-pop-item")).toBe(false);
expect(row.getAttribute("tabindex")).toBe("-1");
// The reason rides the accessible name too: `aria-disabled` alone
// announces that the row is unavailable and never why.
expect(row.getAttribute("aria-label")).toBe(
"Commit to git. not a git repository"
);
});
it("keeps the plain disabled attribute when it has no reason to give", () => {
const row = rowFor({ disabled: true, label: "Nothing", run: vi.fn() });
expect(row.hasAttribute("disabled")).toBe(true);
expect(row.hasAttribute("data-tip")).toBe(false);
expect(row.hasAttribute("aria-label")).toBe(false);
});
it("does not run when clicked", () => {
const run = vi.fn();
rowFor({
disabled: true,
label: "Commit to git",
run,
tip: "not a git repository",
}).click();
expect(run).not.toHaveBeenCalled();
// And the menu stays open, so the tip can still be read.
expect(shells()).toHaveLength(1);
});
});
});
/*
+28 -1
View File
@@ -756,6 +756,14 @@ export interface MenuItem {
label: string;
on?: boolean;
run: () => void;
/**
* Why this row is greyed, shown on hover.
*
* A disabled row that does not say why is a dead end: the user can see that
* Commit is unavailable and has nothing to act on. Pair it with `disabled`
* whenever the reason is something they can fix.
*/
tip?: string;
}
/**
@@ -1002,16 +1010,35 @@ export function createMenu(entries: MenuEntry[]): MenuHandle {
const buildRow = (entry: MenuItem): HTMLElement => {
const disabled = Boolean(entry.disabled);
/*
* A disabled row that carries a reason keeps `aria-disabled` and loses the
* `disabled` *attribute*.
*
* Browsers do not dispatch pointer events from a disabled form control, so
* `data-tip` on one is never read: `Tooltips` matches on `pointerover`, and
* the event it sees is targeted at the ancestor instead. The greying is
* driven by `[aria-disabled="true"]` in `pop.css.ts`, not by `:disabled`, so
* dropping the attribute costs nothing visually, and `tabindex="-1"` keeps
* the row out of the tab order that the attribute used to hold it out of.
* Rows with no reason to give keep the plain `disabled` and are unchanged.
*/
const hoverable = disabled && Boolean(entry.tip);
const row = el(
"button",
{
"aria-label":
hoverable && entry.tip ? `${entry.label}. ${entry.tip}` : undefined,
class: `${cls("pop-item")}${entry.on ? ` ${cls("pop-item-on")}` : ""}`,
"data-tip": entry.tip,
role: "menuitem",
type: "button",
// Omitting the attribute is what keeps the roving cursor off it; the
// arrow-key code only ever queries `[data-pop-item]`.
...(disabled
? { "aria-disabled": "true", disabled: "" }
? {
"aria-disabled": "true",
...(hoverable ? { tabindex: "-1" } : { disabled: "" }),
}
: { "data-pop-item": "" }),
onClick: disabled
? undefined