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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ All notable changes to this project will be documented in this file. See [standa

### 🐛 Bug Fixes

- `hook-dispatch` removes the temp file that carries a payload to a background child on every exit, including the paths that return before reading it, and a background pass clears payload files an earlier one left in the temp directory. A Windows hook whose child exited early, or one killed before its child started, kept a full hook payload — tool input and output — in `%TEMP%` for good (for [#1812](https://github.com/Tencent/teamai-cli/issues/1812)).
- Pull says when it could not apply the team MCP servers, and why, such as `Team MCP servers were not applied: EACCES: permission denied, open '.../.mcp.json...'`. It wrote that only to `~/.teamai/debug.log`, so a failed write looked like a pull that changed nothing
- A tool teamai starts in a directory gets `PWD` set to that directory. OpenCode works in `PWD`, not in its process's directory, so a workflow step or benchmark trial on OpenCode started from a member's checkout edited that checkout instead of its own worktree or clone (for [#1793](https://github.com/Tencent/teamai-cli/issues/1793)).
- A label a local confined judge gave names the model alias its scorer asked for, beside the model the tool reported. It named only the model, so a benchmark read every judged verdict as a deviation from its pinned judge (for [#1793](https://github.com/Tencent/teamai-cli/issues/1793)).
Expand Down
74 changes: 73 additions & 1 deletion src/__tests__/hook-dispatch-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ vi.mock('node:child_process', async (importOriginal) => ({

const { runHookFastPath } = await import('../hook-fast-path.js');
const { readStdin } = await import('../utils/hook-stdin.js');
const { parseStdin, trySpawnDetachedViaWmi, deriveDispatchSessionId, hookDispatchCli, claudeHookRunsInAnotherHost } =
const { parseStdin, trySpawnDetachedViaWmi, deriveDispatchSessionId, hookDispatchCli, claudeHookRunsInAnotherHost, sweepStalePayloads } =
await import('../hook-dispatch-cli.js');
const { claudeHookOtherHostSkip } = await import('../claude-hook-host.js');
const { reconcileHooks } = await import('../hooks.js');
Expand Down Expand Up @@ -74,6 +74,43 @@ describe('hookDispatchCli', () => {
} finally { resolveScope.mockRestore(); readFile.mockRestore(); vi.unstubAllEnvs(); }
});

// The payload file is a child's to remove, and an early return used to skip
// that: every dispatch became another full hook payload left in %TEMP% (#1812).
it('removes the payload file on a path that returns before reading it', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'payload-exit-'));
const file = path.join(dir, 'payload.json');
fs.writeFileSync(file, '{"session_id":"abc"}');
vi.stubEnv('TEAMAI_LAUNCHED_BY', 'judge');
try {
await hookDispatchCli('post-tool-use', 'claude', '*', { bgOnly: true, stdinFile: file });
expect(fs.existsSync(file)).toBe(false);
} finally {
vi.unstubAllEnvs();
fs.rmSync(dir, { recursive: true, force: true });
}
});

// A pass runs its handlers for seconds; the payload holds tool input and
// output, so it goes the moment it is read, not when the pass ends (#1813).
it('removes the payload file as soon as it has read it', async () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'payload-read-'));
const file = path.join(dir, 'payload.json');
fs.writeFileSync(file, JSON.stringify({ session_id: 's', cwd: dir, tool_name: 'Read' }));
let presentWhileDispatching: boolean | null = null;
mockDispatcher.dispatch.mockImplementationOnce(async () => {
presentWhileDispatching = fs.existsSync(file);

return { errors: [], output: null };
});
try {
await hookDispatchCli('post-tool-use', 'claude', '*', { stdinFile: file });
expect(presentWhileDispatching).toBe(false);
expect(fs.existsSync(file)).toBe(false);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});

// The fast path read stdin before it found the payload had no session id, so the pipe is drained (#1788).
it('dispatches the payload the fast path read before it fell through to the full dispatch', async () => {
const stdin = new PassThrough();
Expand Down Expand Up @@ -140,6 +177,8 @@ describe('hookDispatchCli', () => {

expect(mockDispatcher.dispatch).not.toHaveBeenCalled();
expect(log.debug).toHaveBeenCalledWith(`[hook-dispatch] ${event}/claude no scope`);
// The payload file is this child's to remove, read or not (#1812).
expect(fs.existsSync(stdinFile)).toBe(false);
});
});

Expand Down Expand Up @@ -663,6 +702,39 @@ afterEach(() => {
for (const f of createdPayloads.splice(0)) fs.rmSync(f, { force: true });
});

describe('sweepStalePayloads', () => {
let dir: string;
let stale: string;
let fresh: string;
let other: string;

beforeEach(() => {
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hook-sweep-'));
stale = path.join(dir, 'teamai-hook-123-1700000000000.json');
fresh = path.join(dir, 'teamai-hook-456-1700000000001.json');
other = path.join(dir, 'settings.json');
for (const file of [stale, fresh, other]) fs.writeFileSync(file, '{}');
const anHourAgo = new Date(Date.now() - 60 * 60 * 1000);
fs.utimesSync(stale, anHourAgo, anHourAgo);
fs.utimesSync(other, anHourAgo, anHourAgo);
});

afterEach(() => fs.rmSync(dir, { recursive: true, force: true }));

it('removes payload files no live child can still be reading, and nothing else', () => {
expect(sweepStalePayloads(dir, Date.now(), 'win32')).toBe(1);

expect(fs.existsSync(stale)).toBe(false);
expect(fs.existsSync(fresh)).toBe(true);
expect(fs.existsSync(other)).toBe(true);
});

it('leaves the temp directory alone on a platform whose spawn paths pipe the payload', () => {
expect(sweepStalePayloads(dir, Date.now(), 'darwin')).toBe(0);
expect(fs.existsSync(stale)).toBe(true);
});
});

describe('trySpawnDetachedViaWmi', () => {
it('creates the child through Win32_Process, hidden, with the payload as a file', async () => {
await expect(trySpawnDetachedViaWmi(
Expand Down
81 changes: 77 additions & 4 deletions src/hook-dispatch-cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ export function salvageStdinFields(raw: string): Partial<Record<SalvageableStdin
return salvaged;
}

/** Name a hook payload handed over as a temp file, so a sweep can tell it apart. */
const PAYLOAD_FILE_PREFIX = 'teamai-hook-';

/** Past this age a payload file has no live reader; well beyond any hook run. */
const STALE_PAYLOAD_AGE_MS = 10 * 60 * 1000;

/**
* Create a detached child through the WMI service instead of CreateProcess.
*
Expand All @@ -174,7 +180,8 @@ export function salvageStdinFields(raw: string): Partial<Record<SalvageableStdin
* even it flashes.
*
* WMI has no STDIN pipe, so `stdin` travels as a temp file named on the command
* line (`--stdin-file`); the child reads and removes it (readStdinFile).
* line (`--stdin-file`); the child removes it as soon as it reads it, and on any
* other exit.
*
* @returns true when the child was created; false when WMI refused or failed —
* the caller then falls back to the plain detached spawn.
Expand All @@ -194,7 +201,7 @@ export async function trySpawnDetachedViaWmi(
let payloadFile: string | undefined;

if (stdin !== undefined) {
payloadFile = path.join(os.tmpdir(), `teamai-hook-${process.pid}-${Date.now()}.json`);
payloadFile = path.join(os.tmpdir(), `${PAYLOAD_FILE_PREFIX}${process.pid}-${Date.now()}.json`);

try {
fs.writeFileSync(payloadFile, stdin, 'utf8');
Expand Down Expand Up @@ -300,7 +307,7 @@ function quoteWindowsArg(arg: string): string {
/**
* Read the STDIN payload a parent could not pipe, and remove it. The Windows/WMI
* spawn path has no STDIN pipe, so the parent hands the payload over as a temp
* file.
* file; a dispatch that never reaches this read removes it on its own exit.
*/
function readStdinFile(file: string): string {
try {
Expand All @@ -310,8 +317,51 @@ function readStdinFile(file: string): string {

return '';
} finally {
removeStdinFile(file);
}
}

/** Remove one payload file. A hook never fails over this, and a second removal is a no-op. */
function removeStdinFile(file: string): void {
try {
fs.rmSync(file, { force: true });
} catch (e) {
log.debug(`hook-dispatch: could not remove STDIN file ${file}: ${e instanceof Error ? e.message : String(e)}`);
}
}

/**
* Remove payload files a killed hook left in the temp directory (#1812). A
* parent that dies before its child starts cannot remove its own file, and the
* child that would have is the only other process that knows the path, so the
* next background pass sweeps what is clearly stale. Windows-only: the plain
* spawn paths pipe the payload and create no file.
*/
export function sweepStalePayloads(dir = os.tmpdir(), now = Date.now(), platform = process.platform): number {
if (platform !== 'win32') return 0;

let removed = 0;

try {
for (const name of fs.readdirSync(dir)) {
if (!name.startsWith(PAYLOAD_FILE_PREFIX) || !name.endsWith('.json')) continue;

const file = path.join(dir, name);

try {
if (now - fs.statSync(file).mtimeMs < STALE_PAYLOAD_AGE_MS) continue;

fs.rmSync(file, { force: true });
removed += 1;
} catch {
// Another hook cleaned it up, or it is not ours to read any more.
}
}
} catch (e) {
log.debug(`hook-dispatch: could not sweep stale payloads in ${dir}: ${e instanceof Error ? e.message : String(e)}`);
}

return removed;
}

/** Parse STDIN JSON and normalize the event name for downstream handlers. */
Expand Down Expand Up @@ -395,6 +445,9 @@ async function runDispatch(

export { deriveDispatchSessionId };

/** The internal switches a detached child or its parent passes to a dispatch. */
type HookDispatchOptions = { bgOnly?: boolean; copilotGlobal?: string; stdinFile?: string; hookArgs?: string[] };

/**
* Main CLI handler for hook-dispatch.
*
Expand All @@ -407,7 +460,23 @@ export async function hookDispatchCli(
event: string,
tool: string,
matcher: string,
options: { bgOnly?: boolean; copilotGlobal?: string; stdinFile?: string; hookArgs?: string[] } = {},
options: HookDispatchOptions = {},
): Promise<void> {
try {
await runHookDispatch(event, tool, matcher, options);
} finally {
// The read path removed it already; this covers every exit that never got
// that far (#1812) — the file is this child's only, and it is the only
// process that knows its path.
if (options.stdinFile) removeStdinFile(options.stdinFile);
}
}

async function runHookDispatch(
event: string,
tool: string,
matcher: string,
options: HookDispatchOptions,
): Promise<void> {
// A session teamai launched for itself, a judge or an AI call, is not the member's: only a workflow step's runs hooks.
const launcher = launchedBy(process.env);
Expand All @@ -416,6 +485,10 @@ export async function hookDispatchCli(
const { bgOnly = false, stdinFile } = options;
setStderrOnly(true);

// A parent killed before its child starts leaves its payload behind, and no
// process is left to remove it: this pass is that removal (#1812).
if (bgOnly) sweepStalePayloads();

if (claudeHookRunsInAnotherHost(tool, process.env, false)) {
log.debug('hook-dispatch: skipping claude hooks because Cursor or Copilot CLI has its own teamai hooks');

Expand Down
Loading