fix(git-status): address #537 review (docker workspaces, gone upstream, in-flight reset, docs)

- never inspect a repository at or inside a Docker case workspace (walk-up, scan, diff route): git would run its clean filters on the host
- a branch whose upstream was deleted and pruned reports upstreamGone and falls back to commits on no remote, instead of green
- turning the setting off during a poll releases the in-flight flag
- log.showSignature=false; reword the docs: clean filters still run
- CLAUDE.md frontend load order, changeset names git-diff
- discovery reads a bounded, sorted directory listing; leading-dash paths allowed; diff 500 redacts credentials
- keyboard focus survives the poll re-render; panel stays on screen on narrow viewports; aria-expanded visible on light skins

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JrzFKEdBLwVfu6ev2ZscJS
This commit is contained in:
Devvyn
2026-10-05 11:49:38 +08:00
co-authored by Claude Sonnet 5.5
parent b2423c90ce
commit 0c4bb5169f
11 changed files with 352 additions and 49 deletions
+45 -1
View File
@@ -203,6 +203,23 @@ describe('Git status indicator in a real browser', () => {
await page.click('#gitStatusBody button:has-text("Back")');
});
it('keeps keyboard focus on the same file row across the 15 s re-render', async () => {
await page.focus('.git-status-file:has-text("a.txt")');
const key = () => page.evaluate(() => (document.activeElement as HTMLElement | null)?.dataset?.gitKey ?? null);
expect(await key()).toBe('unstaged|a.txt');
await refresh();
await page.waitForFunction(() => document.activeElement?.getAttribute('data-git-key') === 'unstaged|a.txt');
expect(await key()).toBe('unstaged|a.txt');
});
it('the panel never starts off-screen, even on a 650px-wide viewport', async () => {
const original = page.viewportSize()!;
await page.setViewportSize({ width: 650, height: original.height });
const left = await page.evaluate(() => document.getElementById('gitStatusPanel')!.getBoundingClientRect().left);
await page.setViewportSize(original);
expect(left).toBeGreaterThanOrEqual(0);
});
it('drags by the header', async () => {
const before = await page.evaluate(() => document.getElementById('gitStatusPanel')!.getBoundingClientRect().left);
const box = (await page.locator('.git-status-header').boundingBox())!;
@@ -352,6 +369,33 @@ describe('Git status indicator in a real browser', () => {
});
});
it('turning the setting off while a read is in flight, then on again, does not leave polling dead', async () => {
let slow = true;
await page.route('**/api/sessions/*/git-status*', async (route) => {
if (slow) await new Promise((r) => setTimeout(r, 1500));
await route.continue();
});
page.setDefaultTimeout(6000);
// A background poll may be mid-read: let it finish so OUR read is the one the slow route holds.
await page.waitForFunction(() => (window as any).app._gitStatusInFlight === false);
await page.evaluate(() => void (window as any).app.refreshGitStatus({ fresh: true }));
await page.waitForFunction(() => (window as any).app._gitStatusInFlight === true);
await setSetting(false);
expect(await page.evaluate(() => (window as any).app._gitStatusInFlight)).toBe(false);
slow = false;
await setSetting(true);
// Re-enabling starts its own read. With the flag stuck true that read is skipped for this session
// and the indicator never comes back.
await page.waitForFunction(() => !!(window as any).app._currentGitStatus());
expect(await page.evaluate(() => (window as any).app._gitStatusInFlight)).toBe(false);
page.setDefaultTimeout(30000);
await page.unroute('**/api/sessions/*/git-status*');
expect(await buttonVisible()).toBe(true);
// Turning the setting off closed the panel; reopen it for the tests that follow.
await page.click('#gitStatusBtn');
await page.waitForSelector('#gitStatusPanel.visible');
}, 30000);
it('closing the panel resets it; turning the setting off hides the button, closes the panel and stops polling', async () => {
await page.click('.git-status-actions button[aria-label="Close git status"]');
expect(await page.isVisible('#gitStatusPanel')).toBe(false);
@@ -364,5 +408,5 @@ describe('Git status indicator in a real browser', () => {
const before = gitStatusRequests.length;
await page.waitForTimeout(3000);
expect(gitStatusRequests.length).toBe(before);
});
}, 30000);
});
+106 -1
View File
@@ -33,6 +33,18 @@ describe('parsePorcelainV2', () => {
});
});
it('flags an upstream whose remote branch is gone: branch.upstream without branch.ab', () => {
expect(
parsePorcelainV2(['# branch.oid x', '# branch.head feature', '# branch.upstream origin/feature'].join(NUL) + NUL)
).toMatchObject({ upstream: 'origin/feature', upstreamGone: true });
expect(
parsePorcelainV2(
['# branch.oid x', '# branch.head main', '# branch.upstream origin/main', '# branch.ab +0 -0'].join(NUL) + NUL
)
).toMatchObject({ upstreamGone: false });
expect(parsePorcelainV2(['# branch.oid x', '# branch.head feature'].join(NUL) + NUL).upstreamGone).toBe(false);
});
it('reads a detached HEAD and a branch with no upstream (no branch.ab line either)', () => {
expect(parsePorcelainV2(['# branch.oid x', '# branch.head (detached)'].join(NUL) + NUL)).toMatchObject({
branch: null,
@@ -314,6 +326,21 @@ describe('getGitWorkspaceStatus against a real repository', () => {
expect(s.unpushed.map((c) => c.subject)).toEqual(['f2', 'f1']);
});
it('a branch whose upstream was deleted and pruned is NOT reported as everything pushed', async () => {
git(repo, 'checkout', '-q', '-b', 'feature');
write('f1.txt');
commit('f1');
git(repo, 'push', '-q', '-u', 'origin', 'feature');
git(repo, 'push', '-q', 'origin', '--delete', 'feature');
git(repo, 'fetch', '-q', '--prune');
write('f2.txt');
commit('f2');
const s = await getGitWorkspaceStatus(repo);
expect(s).toMatchObject({ branch: 'feature', upstream: 'origin/feature', upstreamGone: true });
expect(s.unpushedCount).toBe(2);
expect(s.unpushed.map((c) => c.subject)).toEqual(['f2', 'f1']);
});
it('a pushed branch is not reported as unpushed once it has an upstream', async () => {
git(repo, 'checkout', '-q', '-b', 'feature');
write('f1.txt');
@@ -668,7 +695,8 @@ describe('isSafeRepoRelativePath', () => {
['a.txt', true],
['src/deep/x.ts', true],
['', false],
['-rf', false],
['-rf', true], // every operand follows `--`, so a leading dash is just a name
['-', true],
['/etc/passwd', false],
['../x', false],
['a/../../x', false],
@@ -725,3 +753,80 @@ describe('getGitFileDiff', () => {
expect(cut.diff.endsWith('x')).toBe(true);
});
});
describe('Docker case workspaces are never inspected', () => {
let top: string;
let home: string;
const repoAt = (p: string): string => {
mkdir(p, { recursive: true });
git(p, 'init', '-q', '-b', 'main');
writeFileSync(join(p, 'f.txt'), '1\n');
git(p, 'add', '-A');
git(p, 'commit', '-q', '-m', 'c');
return p;
};
/** A repository whose clean filter drops a marker file: proof that git ran on it. */
const booby = (p: string): string => {
repoAt(p);
git(p, 'config', 'filter.mark.clean', 'touch RAN; cat');
writeFileSync(join(p, '.gitattributes'), 'f.txt filter=mark\n');
// Same size as the committed '1\n', so git must read the content (running the filter) to see the change.
writeFileSync(join(p, 'f.txt'), 'x\n');
return p;
};
beforeEach(() => {
top = mkdtempSync(join(tmpdir(), 'git-docker-'));
home = join(top, 'home');
mkdir(home, { recursive: true });
clearGitStatusCache();
});
afterEach(() => rmSync(top, { recursive: true, force: true }));
it('control: without the exclusion git does run the repository’s clean filter', async () => {
const ws = join(home, 'case');
booby(join(ws, 'proj'));
await getGitWorkspaceOverview(ws, { home, git: undefined });
expect(existsSync(join(ws, 'proj', 'RAN'))).toBe(true);
});
it('drops a Docker workspace found below the folder, and runs nothing in it', async () => {
const ws = join(home, 'case');
booby(join(ws, 'sandbox'));
repoAt(join(ws, 'plain'));
const o = await getGitWorkspaceOverview(ws, { home, dockerWorkspaces: [join(ws, 'sandbox')] });
expect(o.repos.map((r) => r.path)).toEqual(['plain']);
expect(existsSync(join(ws, 'sandbox', 'RAN'))).toBe(false);
});
it('answers unsupported/docker for a folder at or inside a Docker workspace, before any git runs', async () => {
const dock = booby(join(home, 'dock'));
mkdir(join(dock, 'sub'));
for (const cwd of [dock, join(dock, 'sub')]) {
clearGitStatusCache();
const git = vi.fn(async () => '');
const o = await getGitWorkspaceOverview(cwd, { home, git, dockerWorkspaces: [dock] });
expect(o).toMatchObject({ state: 'unsupported', reason: 'docker', repos: [] });
expect(git).not.toHaveBeenCalled();
}
expect(existsSync(join(dock, 'RAN'))).toBe(false);
});
it('sees through a symlink to the workspace', async () => {
const dock = booby(join(home, 'dock'));
const ws = join(home, 'case');
mkdir(ws, { recursive: true });
symlink(dock, join(ws, 'link'));
const o = await getGitWorkspaceOverview(join(ws, 'link'), { home, dockerWorkspaces: [dock] });
expect(o.state).toBe('unsupported');
expect(existsSync(join(dock, 'RAN'))).toBe(false);
});
it('a folder next to the workspace, and one whose name merely starts the same, are not excluded', async () => {
const dock = join(home, 'dock');
const sibling = repoAt(join(home, 'dock-two'));
mkdir(dock, { recursive: true });
const o = await getGitWorkspaceOverview(sibling, { home, dockerWorkspaces: [dock] });
expect(o.state).toBe('ok');
});
});
+20 -4
View File
@@ -26,10 +26,15 @@ const git = (cwd: string, ...args: string[]) => execFileSync('git', args, { cwd,
let dir: string;
let session: Record<string, unknown>;
async function setup(opts: { git?: GitRunner; authUser?: { username: string; role: 'admin' | 'user' } } = {}) {
const h = await createRouteTestHarness((app, ctx) => registerGitStatusRoutes(app, ctx, opts.git), {
authUser: opts.authUser,
});
async function setup(
opts: { git?: GitRunner; authUser?: { username: string; role: 'admin' | 'user' }; dockerWorkspaces?: string[] } = {}
) {
const h = await createRouteTestHarness(
(app, ctx) => registerGitStatusRoutes(app, ctx, opts.git, async () => opts.dockerWorkspaces ?? []),
{
authUser: opts.authUser,
}
);
session = h.ctx._session as unknown as Record<string, unknown>;
session.workingDir = dir;
return h;
@@ -253,4 +258,15 @@ describe('GET /api/sessions/:id/git-diff', () => {
expect(conflict.statusCode).toBe(200);
expect(conflict.json().data.diff).toMatch(/<<<<<<<|\+\+<<<<<<</);
});
it('404s a repository inside a Docker case workspace without running git in it', async () => {
const runner = vi.fn<GitRunner>(async () => '');
const { app } = await setup({ git: runner, dockerWorkspaces: [realpathSync(dir)] });
const res = await app.inject({
method: 'GET',
url: url({ repo: realpathSync(dir), path: 'a.txt', kind: 'unstaged' }),
});
expect(res.statusCode).toBe(404);
expect(runner).not.toHaveBeenCalled();
});
});