Skip to content
Open
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
7 changes: 7 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,13 @@ This is a local dev-loop gap only, not a pipeline defect: CI always builds `keyc
3. **Slash commands are string-matched** — no command registry
4. **Context thresholds hardcoded** — 85%/90% tool disable thresholds not configurable
5. **AppLayout combines View + Controller** — separation planned
6. **A broken pipe reports two different ways** — ExecV3 joins pipeline stages at the file descriptor, but Node's stdio is a *socketpair*, not a pipe. A consumer that exits leaving unread bytes makes the kernel reset the connection, so the producer's write fails with `ECONNRESET`, it exits non-zero, and it prints its own error (`yes: standard output: Connection reset by peer`). A clean close instead raises SIGPIPE and the producer dies silently. Which one happens is timing: measured over 40 rounds, SIGPIPE every time on macOS and about one in three on Linux. The hang fix is unaffected — the producer always stops — and the blast radius is only a consumer that exits early, never a full-drain pipe.

The cost worth caring about is not the `signal` field, which nothing reads. It is the stderr line: a plausible-looking error the model may act on, for a command that did exactly what was asked.

**Do not "fix" this by reaching for a shell.** Bash is deterministic here because it calls `pipe(2)`; ExecV3 has no shell on purpose, so this nondeterminism is a cost of that decision, not a defect to route around. Two real options if it ever matters: a FIFO per link (rejected — Node has no `mkfifo` either, so it means spawning a process per link or native code anyway, and it puts a predictable path in a shared tmpdir inside the one tool that runs arbitrary commands), or a native `pipe(2)` binding (the clean answer, following the `keychain-native` N-API pattern, but it turns `exec-core` from pure JS into a native package needing per-platform prebuilds). Revisit only if Linux becomes a platform we ship to rather than one we test on.

Do not assert a specific signal for a torn-down producer in a test. `pipeline-teardown.spec.ts` asserts it stopped and did not succeed, which holds on both.

## Az Auth Hardening

Expand Down
4 changes: 4 additions & 0 deletions apps/claude-sdk-cli/test/MemoryFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ export class MemoryFileSystem extends IFileSystem {
return this.#arch;
}

public openWriteStream(path: string, options: { flags: 'a' | 'w' }): Writable {
return this.createWriteStream(path, options);
}

public createWriteStream(path: string, options: { flags: 'a' | 'w' }): Writable {
const initial = options.flags === 'a' ? (this.files.get(path) ?? '') : '';
const chunks: string[] = [initial];
Expand Down
2 changes: 2 additions & 0 deletions packages/claude-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Adopt core-di-lite property injection: config loading splits into a pure read, a holder, and a watch handle with no load or start step, and the shared provider and contract abstractions live here for every package to resolve against
- canonicalisePath takes an optional working directory, for a caller whose relative paths belong to somewhere other than the filesystem's own; resolving by hand first would strip the chance to expand ~ and $VAR
- Depend on @shellicar/core-di instead of @shellicar/core-di-lite
- File discovery returns records carrying type, size, and symlink target instead of bare path strings
- IFileSystem gained openWriteStream, which opens the file before returning so an unwritable path fails at the caller rather than later on the stream; an existing implementation must add it
- setupKeypressHandler accepts an optional escFastPathEnabled callback, checked live before taking the lone-ESC fast path, so a consumer can disable it (e.g. over a fragmented remote connection) without a restart
- Update runtime and build dependencies
- Updated patch and minor dependencies
Expand Down
2 changes: 2 additions & 0 deletions packages/claude-core/changes.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,5 @@
{"description":"IFileSystem gains tmpdir, uid, mkdir with an explicit mode, lstat, and a synchronous readlinkSync, so code that needs a temporary directory, or has to create one and check who owns it, can reach all of it through the filesystem seam instead of node:os and node:fs directly","category":"added"}
{"description":"StatResult now carries uid and mode, so a caller can tell who owns a path and who else can reach it","category":"added"}
{"description":"canonicalisePath resolves a path to where it actually lands, following symlinks even when the target does not exist yet, for callers that must decide on the destination rather than the string they were handed","category":"added"}
{"description":"IFileSystem gained openWriteStream, which opens the file before returning so an unwritable path fails at the caller rather than later on the stream; an existing implementation must add it","category":"changed"}
{"description":"canonicalisePath takes an optional working directory, for a caller whose relative paths belong to somewhere other than the filesystem's own; resolving by hand first would strip the chance to expand ~ and $VAR","category":"changed"}
9 changes: 7 additions & 2 deletions packages/claude-core/src/fs/canonicalisePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,14 @@ const MAX_DANGLING_HOPS = 32;
* it was written. Throws when the path cannot be canonicalised at all, carrying the OS's own reason:
* a caller that needs a verdict rather than a path decides what to make of that, and a caller that
* needs a path is better told than handed something that only looks like one.
*
* `cwd` is the directory a relative path is relative *to*, and defaults to the filesystem's own.
* A caller whose paths belong to somewhere else passes it rather than resolving first: expansion
* has to happen before resolution, so `~/x` resolved by hand becomes a literal `~` directory that
* no later expansion can undo.
*/
export function canonicalisePath(value: string, fs: IFileSystem): string {
const absolute = path.resolve(fs.cwd(), expandPath(value, fs));
export function canonicalisePath(value: string, fs: IFileSystem, cwd: string = fs.cwd()): string {
const absolute = path.resolve(cwd, expandPath(value, fs));
return resolve(absolute, fs, MAX_DANGLING_HOPS);
}

Expand Down
7 changes: 7 additions & 0 deletions packages/claude-core/src/fs/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,11 @@ export abstract class IFileSystem {
public abstract arch(): NodeJS.Architecture;
/** Open a writable stream to a file, for a redirect target rather than a one-shot write. */
public abstract createWriteStream(path: string, options: { flags: 'a' | 'w' }): Writable;
/**
* The same, except the file is opened before this returns, so a path that cannot be written
* throws here rather than failing later on the stream. That is the difference between a
* caller being able to refuse to run a command whose output would have nowhere to go, and
* reporting success for one whose output it silently discarded.
*/
public abstract openWriteStream(path: string, options: { flags: 'a' | 'w' }): Writable;
}
4 changes: 4 additions & 0 deletions packages/claude-core/test/MemoryFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ export class MemoryFileSystem extends IFileSystem {
throw new Error('MemoryFileSystem: createWriteStream() not supported');
}

public openWriteStream(): Writable {
throw new Error('MemoryFileSystem: openWriteStream() not supported');
}

public readlink(): Promise<string> {
throw new Error('MemoryFileSystem: readlink() not supported');
}
Expand Down
4 changes: 4 additions & 0 deletions packages/claude-core/test/SymlinkFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,4 +190,8 @@ export class SymlinkFileSystem extends IFileSystem {
public createWriteStream(): Writable {
throw new Error('SymlinkFileSystem: createWriteStream() not supported');
}

public openWriteStream(): Writable {
throw new Error('SymlinkFileSystem: openWriteStream() not supported');
}
}
30 changes: 30 additions & 0 deletions packages/claude-core/test/canonicalisePath.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,33 @@ describe('canonicalisePath', () => {
expect(actual).toBe(expected);
});
});

// A caller whose paths are relative to somewhere other than the filesystem's own working directory
// passes that directory rather than resolving first, because expansion has to happen before
// resolution: `~/x` resolved by hand becomes a literal `~` component nothing can undo afterwards.
describe('canonicalisePath with a caller-supplied working directory', () => {
it('resolves a relative path against the directory it was given', () => {
const expected = '/private/var/folders/xk/T/claude-501/conversation/scratchpad/existing.txt';
const actual = canonicalisePath('existing.txt', fsWith(), WORKSPACE);
expect(actual).toBe(expected);
});

it('leaves an absolute path alone', () => {
const expected = '/project/src/file.ts';
const actual = canonicalisePath('/project/src/file.ts', fsWith(), WORKSPACE);
expect(actual).toBe(expected);
});

it('expands the home directory before resolving, so it is never treated as a directory name', () => {
const home = fsWith().homedir();
const expected = canonicalisePath(`${home}/notes.txt`, fsWith());
const actual = canonicalisePath('~/notes.txt', fsWith(), WORKSPACE);
expect(actual).toBe(expected);
});

it('still defaults to the filesystem working directory when none is given', () => {
const expected = '/project/src/file.ts';
const actual = canonicalisePath('src/file.ts', fsWith());
expect(actual).toBe(expected);
});
});
5 changes: 5 additions & 0 deletions packages/claude-sdk-tools/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- A pipe stage stopped because its consumer exited may report a SIGPIPE signal or a non-zero exit carrying the program's own broken-pipe message; both mean the pipe broke, and which one appears is timing
- Adopt core-di-lite property injection: TsServerService resolves its options through injection and disposes its tsserver process on scope exit
- Az account changes take effect immediately across AzCli, EscalatedAzCli, and AzureDevOps.PullRequest.*, with no restart
- AzureDevOps.PullRequest.* tools reuse AzCli/EscalatedAzCli's session cache instead of logging in fresh each call
Expand Down Expand Up @@ -90,12 +91,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- A command producing very large output no longer fails the whole ExecV3 call; each captured stream keeps up to 8 MiB and says so when there was more
- A command whose redirect target cannot be opened now fails and says why, instead of running and reporting success while its output went nowhere
- A failed tsserver request now throws instead of returning an empty result that was indistinguishable from a clean file
- An interactive az identity no longer gets a silent, unattended background relogin; the browser/MFA prompt only ever appears attached to a real caller's call
- AzCli, EscalatedAzCli, and every AzureDevOps.PullRequest.* tool now honor cancellation — an in-progress az login or command can be aborted instead of blocking until the process crashes or restarts
- AzureDevOps.PullRequest.* tools accept an account field, matching AzCli/EscalatedAzCli
- Binary files are blocked from text reads when the format is recognised; unrecognised formats are still treated as text
- ExecV3 and Memory import defineTool, ToolCancelledError, ToolRefusedError, and pathSchema from their own claude-sdk subpaths instead of the barrel, so a consumer bundling this package no longer pulls in the whole SDK module graph
- ExecV3 pipelines now run over real OS pipes, so a cancelled or timed-out pipe returns instead of hanging the caller
- ExecV3 rejects stdout and stderr redirected to the same file, which silently discarded one of them; use stderr "&1" to merge
- Find tool follows symlinks with cycle detection
- Fix version metadata
- GitHub_PullRequest_AutoMerge takes a required strategy (merge, squash, rebase) when enabling, so it can queue a specific merge method instead of only accepting the repo default
Expand Down
5 changes: 5 additions & 0 deletions packages/claude-sdk-tools/changes.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,8 @@
{"description":"An interactive az identity no longer gets a silent, unattended background relogin; the browser/MFA prompt only ever appears attached to a real caller's call","category":"fixed"}
{"description":"The az session's own login and command env now strips the same ambient Azure credential vars ExecV3 strips, so the CLI's own environment can no longer steer a login it believes it fully controls","category":"security"}
{"description":"NodeFileSystem implements the new IFileSystem members: the real OS temp directory, the process user id, a recursive create that honours an explicit mode, a symlink-preserving lstat, and a readlinkSync that answers null rather than throwing when there is nothing to follow","category":"added"}
{"description":"ExecV3 pipelines now run over real OS pipes, so a cancelled or timed-out pipe returns instead of hanging the caller","category":"fixed"}
{"description":"A command whose redirect target cannot be opened now fails and says why, instead of running and reporting success while its output went nowhere","category":"fixed"}
{"description":"ExecV3 rejects stdout and stderr redirected to the same file, which silently discarded one of them; use stderr \"&1\" to merge","category":"fixed"}
{"description":"A command producing very large output no longer fails the whole ExecV3 call; each captured stream keeps up to 8 MiB and says so when there was more","category":"fixed"}
{"description":"A pipe stage stopped because its consumer exited may report a SIGPIPE signal or a non-zero exit carrying the program's own broken-pipe message; both mean the pipe broke, and which one appears is timing","category":"changed"}
Loading
Loading