Skip to content

Commit fc56b74

Browse files
roodboiclaude
andcommitted
fix: gate tickets git cache on extension enablement, cover it in ignores
Field report: hack setup sync --all-scopes left an untracked .hack/tickets/ local bare-git cache in a repo. setup sync itself was already correctly gated (checkTicketsSkill/installTicketsSkill only run when resolveTicketsIntegrationEnablement().project is true), so the actual culprit is `hack doctor` (and `--fix`), which every project runs as routine maintenance per this repo's own instructions ("run hack doctor, then hack doctor --fix"). checkProjectTicketsGitHealth and maybeRepairProjectTicketsGitHealth (src/commands/doctor.ts) only checked controlPlane.config.tickets.git.enabled — a config key that defaults to `true` and is unrelated to whether the tickets extension is actually enabled for the project (controlPlane.extensions["dance.hack.tickets"].enabled, default `false`). So every `hack doctor` run called createGitTicketsChannel(...).inspect(), whose ensureCheckedOut() creates .hack/tickets/git/{bare.git,worktree} as a side effect of merely inspecting health — regardless of enablement. Both call sites now also check resolveTicketsIntegrationEnablement() (the same gate setup.ts already uses for the tickets skill) before touching the store. Ignore coverage: .hack/tickets/ is genuinely machine-local (confirmed against docs/guides/tickets.md: local working state that syncs via the hidden refs/hack/tickets ref; no project is documented to intentionally commit it). Added `tickets/` to HACK_DIR_GITIGNORE_ENTRIES (src/templates.ts) so the committed .hack/.gitignore covers it, and `<dir>/tickets` to the doctor generated-files pathspecs (src/lib/doctor-generated-files.ts) so a tracked copy is flagged and `hack doctor --fix` can untrack it. Tests: new tests/doctor-tickets-dir-gating.test.ts covers hack doctor and hack doctor --fix creating no .hack/tickets/ when the extension is disabled, and creating it (gitignored, clean git status) when enabled. Extended tests/hack-gitignore.test.ts and tests/doctor-generated-files.test.ts for the new tickets/ pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 78e068b commit fc56b74

6 files changed

Lines changed: 334 additions & 0 deletions

File tree

src/commands/doctor.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
} from "../constants.ts";
2727
import { resolveGatewayConfig } from "../control-plane/extensions/gateway/config.ts";
2828
import { listGatewayTokens } from "../control-plane/extensions/gateway/tokens.ts";
29+
import { resolveTicketsIntegrationEnablement } from "../control-plane/extensions/tickets/enablement.ts";
2930
import { createGitTicketsChannel } from "../control-plane/extensions/tickets/tickets-git-channel.ts";
3031
import { readControlPlaneConfig } from "../control-plane/sdk/config.ts";
3132
import { probeDaemonApi } from "../daemon/client.ts";
@@ -1695,6 +1696,17 @@ async function checkProjectTicketsGitHealth({
16951696
};
16961697
}
16971698

1699+
const ticketsEnablement = await resolveTicketsIntegrationEnablement({
1700+
projectRoot: project.projectRoot,
1701+
});
1702+
if (!ticketsEnablement.project) {
1703+
return {
1704+
name: "tickets git",
1705+
status: "ok",
1706+
message: "Disabled",
1707+
};
1708+
}
1709+
16981710
const controlPlane = await readControlPlaneConfig({
16991711
projectDir: project.projectDir,
17001712
});
@@ -2777,6 +2789,13 @@ async function maybeRepairProjectTicketsGitHealth(opts: {
27772789
return;
27782790
}
27792791

2792+
const ticketsEnablement = await resolveTicketsIntegrationEnablement({
2793+
projectRoot: project.projectRoot,
2794+
});
2795+
if (!ticketsEnablement.project) {
2796+
return;
2797+
}
2798+
27802799
const controlPlane = await readControlPlaneConfig({
27812800
projectDir: project.projectDir,
27822801
});

src/lib/doctor-generated-files.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ export type TrackedGeneratedFilesInspection = {
1616
* Intentionally excluded: `<dir>/hack.env.local.yaml` — older repos may track
1717
* it on purpose as the shared `--env local` overlay (legacy compatibility in
1818
* `project-env-config.ts`), so a tracked copy is not treated as a leak.
19+
*
20+
* `<dir>/tickets` is included because the tickets extension's local git
21+
* cache (`<dir>/tickets/git/bare.git`, `.../worktree`) is machine-local
22+
* working state that syncs via the hidden `refs/hack/tickets` ref — see
23+
* `docs/guides/tickets.md`. No project is documented to intentionally commit
24+
* it, so a tracked copy is always treated as a leak.
1925
*/
2026
export function buildGeneratedFilePathspecs(opts: {
2127
readonly projectDirName: string;
@@ -27,6 +33,7 @@ export function buildGeneratedFilePathspecs(opts: {
2733
`${dir}/.env`,
2834
`${dir}/.env.state.json`,
2935
`${dir}/hack.env.*.local.yaml`,
36+
`${dir}/tickets`,
3037
PROJECT_ENV_KEY_FILENAME,
3138
];
3239
}

src/templates.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ export const HACK_DIR_GITIGNORE_ENTRIES = [
318318
".env.state.json",
319319
"hack.env.local.yaml",
320320
"hack.env.*.local.yaml",
321+
"tickets/",
321322
] as const;
322323

323324
/**

tests/doctor-generated-files.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,55 @@ test("tracked .hack/hack.env.local.yaml is not flagged (legacy shared overlay)",
125125
expect(inspection?.trackedPaths).toEqual([]);
126126
});
127127

128+
test("buildGeneratedFilePathspecs covers the tickets extension's local git cache", () => {
129+
expect(buildGeneratedFilePathspecs({ projectDirName: ".hack" })).toContain(
130+
".hack/tickets"
131+
);
132+
});
133+
134+
test("tracked .hack/tickets/ (leaked tickets git cache) is flagged and can be untracked", async () => {
135+
const dir = await mkdtemp(join(tmpdir(), "hack-doctor-generated-tickets-"));
136+
tempDirs.add(dir);
137+
const repoRoot = resolve(dir, "repo");
138+
await mkdir(resolve(repoRoot, ".hack", "tickets", "git"), {
139+
recursive: true,
140+
});
141+
await runGit(["init", "-b", "main"], repoRoot);
142+
await runGit(["config", "user.name", "Hack Test"], repoRoot);
143+
await runGit(["config", "user.email", "hack@example.com"], repoRoot);
144+
await writeFile(
145+
resolve(repoRoot, ".hack", "docker-compose.yml"),
146+
"services:\n api: {}\n"
147+
);
148+
await writeFile(
149+
resolve(repoRoot, ".hack", "tickets", "git", "marker.txt"),
150+
"leaked tickets cache\n"
151+
);
152+
await runGit(["add", "-f", "."], repoRoot);
153+
await runGit(["commit", "-m", "leak tickets cache"], repoRoot);
154+
155+
const inspection = await inspectTrackedGeneratedFiles({
156+
projectRoot: repoRoot,
157+
projectDirName: ".hack",
158+
});
159+
expect(inspection?.trackedPaths).toEqual([".hack/tickets/git/marker.txt"]);
160+
161+
const untracked = await untrackGeneratedFiles({
162+
projectRoot: repoRoot,
163+
paths: inspection?.trackedPaths ?? [],
164+
});
165+
expect(untracked).toEqual({ ok: true, error: null });
166+
167+
const second = await inspectTrackedGeneratedFiles({
168+
projectRoot: repoRoot,
169+
projectDirName: ".hack",
170+
});
171+
expect(second?.trackedPaths).toEqual([]);
172+
expect(
173+
await pathExists(resolve(repoRoot, ".hack", "tickets", "git", "marker.txt"))
174+
).toBe(true);
175+
});
176+
128177
test("untrackGeneratedFiles removes offenders from the index, keeps files on disk, and a rerun is clean", async () => {
129178
const repoRoot = await createLeakedRepo();
130179

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
import {
2+
afterAll,
3+
afterEach,
4+
beforeAll,
5+
beforeEach,
6+
expect,
7+
test,
8+
} from "bun:test";
9+
import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises";
10+
import { tmpdir } from "node:os";
11+
import { join, resolve } from "node:path";
12+
13+
import { ensureHackDirGitignore } from "../src/lib/project-env-config.ts";
14+
import { registerScopedModuleMock } from "./helpers/scoped-module-mock.ts";
15+
16+
/**
17+
* Regression coverage for the field report: `hack doctor` (and `--fix`) must
18+
* not create the tickets extension's local git cache (`.hack/tickets/`)
19+
* unless the extension is actually enabled for the project — it previously
20+
* checked the unrelated `tickets.git.enabled` flag (default `true`) instead
21+
* of `controlPlane.extensions["dance.hack.tickets"].enabled` (default
22+
* `false`), so every `hack doctor` run silently created the directory.
23+
*
24+
* Also covers the companion fix: once the committed `.hack/.gitignore` is
25+
* generated, `tickets/` is covered so an enabled project's cache never shows
26+
* up as untracked.
27+
*
28+
* Docker/shell/OS are mocked (as in tests/doctor-fix-noninteractive.test.ts)
29+
* so `--fix` never touches this machine's real Docker/global infra state.
30+
*/
31+
32+
async function dirExists(path: string): Promise<boolean> {
33+
try {
34+
const info = await stat(path);
35+
return info.isDirectory();
36+
} catch {
37+
return false;
38+
}
39+
}
40+
41+
const clackMock = await registerScopedModuleMock({
42+
importerPath: import.meta.path,
43+
specifier: "@clack/prompts",
44+
overrides: {
45+
confirm: async () => {
46+
throw new Error(
47+
"confirm() must not be called under HACK_NO_INTERACTIVE=1"
48+
);
49+
},
50+
isCancel: () => false,
51+
note: () => {},
52+
spinner: () => ({
53+
start: () => {},
54+
stop: () => {},
55+
}),
56+
},
57+
});
58+
59+
const shellMock = await registerScopedModuleMock({
60+
importerPath: import.meta.path,
61+
specifier: "../src/lib/shell.ts",
62+
overrides: {
63+
exec: async (cmd: readonly string[]) => {
64+
if (cmd[0] === "docker" && cmd[1] === "info") {
65+
return { exitCode: 0, stdout: "", stderr: "" };
66+
}
67+
if (cmd[0] === "docker" && cmd[1] === "network" && cmd[2] === "inspect") {
68+
return { exitCode: 0, stdout: "[]", stderr: "" };
69+
}
70+
return { exitCode: 1, stdout: "", stderr: "" };
71+
},
72+
execOrThrow: async () => ({ exitCode: 0, stdout: "", stderr: "" }),
73+
run: async () => 0,
74+
findExecutableInPath: (name?: string) => {
75+
if (name === "hack" || name === "bun" || name === "docker") {
76+
return `/usr/local/bin/${name}`;
77+
}
78+
return null;
79+
},
80+
CommandError: class CommandError extends Error {},
81+
},
82+
});
83+
84+
const osMock = await registerScopedModuleMock({
85+
importerPath: import.meta.path,
86+
specifier: "../src/lib/os.ts",
87+
overrides: {
88+
isMac: () => true,
89+
isLinux: () => false,
90+
openUrl: async () => 0,
91+
},
92+
});
93+
94+
let tempHome: string | null = null;
95+
let originalHome: string | undefined;
96+
let originalMutagenPath: string | undefined;
97+
98+
beforeAll(() => {
99+
clackMock.activate();
100+
shellMock.activate();
101+
osMock.activate();
102+
});
103+
104+
beforeEach(async () => {
105+
originalHome = process.env.HOME;
106+
originalMutagenPath = process.env.HACK_MUTAGEN_PATH;
107+
tempHome = await mkdtemp(join(tmpdir(), "hack-doctor-tickets-home-"));
108+
process.env.HOME = tempHome;
109+
// Short-circuit mutagen path resolution so --fix never attempts a real
110+
// network install.
111+
process.env.HACK_MUTAGEN_PATH = "/usr/local/bin/mutagen";
112+
});
113+
114+
afterEach(async () => {
115+
if (tempHome) {
116+
await rm(tempHome, { recursive: true, force: true });
117+
tempHome = null;
118+
}
119+
process.env.HOME = originalHome;
120+
if (originalMutagenPath === undefined) {
121+
Reflect.deleteProperty(process.env, "HACK_MUTAGEN_PATH");
122+
} else {
123+
process.env.HACK_MUTAGEN_PATH = originalMutagenPath;
124+
}
125+
});
126+
127+
afterAll(() => {
128+
clackMock.deactivate();
129+
shellMock.deactivate();
130+
osMock.deactivate();
131+
});
132+
133+
const tempDirs = new Set<string>();
134+
135+
afterEach(async () => {
136+
for (const dir of tempDirs) {
137+
await rm(dir, { recursive: true, force: true });
138+
}
139+
tempDirs.clear();
140+
});
141+
142+
async function createTempDir(): Promise<string> {
143+
const dir = await mkdtemp(join(tmpdir(), "hack-doctor-tickets-repo-"));
144+
tempDirs.add(dir);
145+
return dir;
146+
}
147+
148+
async function runGit(args: readonly string[], cwd: string): Promise<string> {
149+
const proc = Bun.spawn({
150+
cmd: ["git", ...args],
151+
cwd,
152+
stdout: "pipe",
153+
stderr: "pipe",
154+
});
155+
const [stdout, stderr, exitCode] = await Promise.all([
156+
new Response(proc.stdout).text(),
157+
new Response(proc.stderr).text(),
158+
proc.exited,
159+
]);
160+
if (exitCode !== 0) {
161+
throw new Error(stderr || stdout || `git ${args.join(" ")} failed`);
162+
}
163+
return stdout.trim();
164+
}
165+
166+
async function gitCheckIgnore(opts: {
167+
readonly repoRoot: string;
168+
readonly path: string;
169+
}): Promise<boolean> {
170+
const proc = Bun.spawn({
171+
cmd: ["git", "check-ignore", "-q", "--", opts.path],
172+
cwd: opts.repoRoot,
173+
stdout: "ignore",
174+
stderr: "ignore",
175+
});
176+
return (await proc.exited) === 0;
177+
}
178+
179+
async function createFixtureRepo(opts: {
180+
readonly ticketsEnabled: boolean;
181+
}): Promise<string> {
182+
const dir = await createTempDir();
183+
const repoRoot = resolve(dir, "repo");
184+
await mkdir(resolve(repoRoot, ".hack"), { recursive: true });
185+
await runGit(["init", "-b", "main"], repoRoot);
186+
await runGit(["config", "user.name", "Hack Test"], repoRoot);
187+
await runGit(["config", "user.email", "hack@example.com"], repoRoot);
188+
189+
await writeFile(
190+
resolve(repoRoot, ".hack", "docker-compose.yml"),
191+
"services:\n api:\n image: alpine:3.19\n"
192+
);
193+
await writeFile(
194+
resolve(repoRoot, ".hack", "hack.config.json"),
195+
`${JSON.stringify(
196+
{
197+
name: "fixture",
198+
controlPlane: {
199+
extensions: {
200+
"dance.hack.tickets": { enabled: opts.ticketsEnabled },
201+
},
202+
},
203+
},
204+
null,
205+
2
206+
)}\n`
207+
);
208+
await ensureHackDirGitignore({ projectDir: resolve(repoRoot, ".hack") });
209+
await runGit(["add", "."], repoRoot);
210+
await runGit(["commit", "-m", "init"], repoRoot);
211+
return repoRoot;
212+
}
213+
214+
async function runDoctor(opts: {
215+
readonly repoRoot: string;
216+
readonly extraArgs?: readonly string[];
217+
}): Promise<number> {
218+
const { runCli } = await import("../src/cli/run.ts");
219+
return await runCli([
220+
"doctor",
221+
"--path",
222+
opts.repoRoot,
223+
"--no-interactive",
224+
...(opts.extraArgs ?? []),
225+
]);
226+
}
227+
228+
test("hack doctor does not create .hack/tickets/ when the extension is disabled", async () => {
229+
const repoRoot = await createFixtureRepo({ ticketsEnabled: false });
230+
231+
await runDoctor({ repoRoot });
232+
233+
expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(false);
234+
});
235+
236+
test("hack doctor --fix does not create .hack/tickets/ when the extension is disabled", async () => {
237+
const repoRoot = await createFixtureRepo({ ticketsEnabled: false });
238+
239+
await runDoctor({ repoRoot, extraArgs: ["--fix"] });
240+
241+
expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(false);
242+
});
243+
244+
test("hack doctor creates .hack/tickets/ when the extension is enabled, and it is gitignored", async () => {
245+
const repoRoot = await createFixtureRepo({ ticketsEnabled: true });
246+
247+
await runDoctor({ repoRoot });
248+
249+
expect(await dirExists(resolve(repoRoot, ".hack", "tickets"))).toBe(true);
250+
expect(
251+
await gitCheckIgnore({ repoRoot, path: ".hack/tickets/git/bare.git" })
252+
).toBe(true);
253+
254+
const status = await runGit(["status", "--porcelain"], repoRoot);
255+
expect(status).toBe("");
256+
});

tests/hack-gitignore.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ test("nested .hack/.gitignore makes git ignore generated files, including in lin
185185
".hack/.env.state.json",
186186
".hack/hack.env.local.yaml",
187187
".hack/hack.env.qa.local.yaml",
188+
".hack/tickets/git/bare.git",
188189
]) {
189190
expect(
190191
await gitCheckIgnore({ repoRoot: sourceRoot, path }),
@@ -200,6 +201,7 @@ test("nested .hack/.gitignore makes git ignore generated files, including in lin
200201
".hack/.branch/compose.x.override.yml",
201202
".hack/.env.state.json",
202203
".hack/hack.env.qa.local.yaml",
204+
".hack/tickets/git/bare.git",
203205
]) {
204206
expect(
205207
await gitCheckIgnore({ repoRoot: linkedRoot, path }),

0 commit comments

Comments
 (0)