mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
Merge pull request #156 from deepseek-harness/worktree-plugin-config-guideline
No hardcoded tunables in plugins: convention, review check, audit + fixes
This commit is contained in:
@@ -21,7 +21,7 @@ Independent judgment governs *what to look at* and *how to apply a rule to this
|
||||
|
||||
These define the conventions and gates this repo is checked against, and they are authoritative. Read them at the source so this skill never drifts out of sync — and apply judgment in *interpreting* them for the case at hand, not in deciding whether they apply.
|
||||
|
||||
- **[AGENTS.md](../../../AGENTS.md) § Conventions** — effect-based registrations, declaration-merging for events/ctx keys, waterfall `next()` discipline, discriminated-union match-don't-chain, explicit-over-implicit at seams, the empty-`catch` rule, symmetry.
|
||||
- **[AGENTS.md](../../../AGENTS.md) § Conventions** — effect-based registrations, declaration-merging for events/ctx keys, waterfall `next()` discipline, discriminated-union match-don't-chain, explicit-over-implicit at seams, no hardcoded tunables in plugins, the empty-`catch` rule, symmetry.
|
||||
- **[docs/defensive-patterns.md](../../../docs/defensive-patterns.md)** — each section is a bug class that bit us. Reviewing anything touching process lifecycle, async/await, disposal, or adapter error paths? Re-read this first — then look for the *adjacent* mistake it doesn't name.
|
||||
- **AGENTS.md § Type safety and documentation + [docs/AGENTS.md](../../../docs/AGENTS.md)** — the doc-sync rule (code change ⇒ update README + JSDoc in the SAME commit) and the writing rules (current-state-never-history, one line per paragraph, one home per fact, the word-budget gate).
|
||||
- **[packages/AGENTS.md](../../../packages/AGENTS.md)** — per-package conventions (file layout, the HMR-safety test requirement).
|
||||
@@ -44,6 +44,7 @@ Where your independent reasoning earns its keep. Start here, then keep going acr
|
||||
- **e2e verifies the world, not the agent's self-report.** For real-API tests, confirm the assertion re-runs the command/checks the file externally — a keyword probe lets a cheating agent pass. For a behavior change to the agent's real flows, a no-key/mock test alone is usually insufficient: a with-key e2e (especially a smoke test that boots the real example and checks the world) is cheap here and catches "green units, broken product" — encourage it rather than treating real-API tests as expensive (see [docs/testing.md](../../../docs/testing.md)).
|
||||
- **Plugin export shape + real-loader coverage.** A new/changed `cordis.yml`-loaded plugin: is it a function/namespace plugin (`name`/`inject`/`Config`/`apply` named exports) with NO `export default`? A stray default export makes the Loader's `unwrapExports` drop `inject` and the plugin crashes at load with `cannot get property … without inject` — invisible to hand-built `ctx.plugin({...})` tests and to line coverage. Confirm there's a test driving it through the REAL loader path (the no-key subprocess e2e for ACP is the model). And any opportunistic read of a service NOT in `static inject` should use `ctx.get(name)`, not `ctx.<name>` (the property proxy throws through a foreign shadow). See [packages/AGENTS.md](../../../packages/AGENTS.md) and [docs/postmortem/0001](../../../docs/postmortem/0001-acp-default-export-drops-inject.md).
|
||||
- **Seam discipline.** New swappable capability? Check it's split per the capability-seams RFC (interface / impl / consumer), and that the consumer injects the interface key, never an implementation type.
|
||||
- **Hardcoded tunables that should be plugin config.** A literal timeout, grace period, output/truncation cap, result-count limit, retry count, buffer size, model name, API base URL, user agent, or filesystem path introduced inside a plugin belongs on the plugin's schemastery `Config` with the shipped value as its default (AGENTS.md § Conventions "No hardcoded tunables in plugins"). A named `DEFAULT_*` constant or a test-only injection seam is not configurability — the question to ask is whether a `cordis.yml` deployment can change the value without a code edit. Protocol/wire constants, semantic constants, values pinned by an external spec, and security invariants are exempt; a new `Config` field also needs its README row and range validation. No gate detects a hardcoded tunable — this check is entirely on the reviewer.
|
||||
- **Test quality — sufficiency, not just coverage.** 100% per-file coverage and a green suite are necessary, not sufficient: they prove the lines *ran*, not that the feature *works the way it ships*. Judge whether the tests are sufficient on two axes. (1) **Would they fail if the behavior regressed?** A test that passes but asserts the wrong thing — or restates the implementation instead of the contract (events fired, disposal reached, the world changed) — is worse than none. (2) **Do they exercise the REAL thing, the way it's actually used?** Prefer the genuine collaborator over a fake, drive the change through its real entry path (the cordis Loader, the ACP bridge, a booted subprocess — not a hand-built `ctx.plugin({...})` that bypasses `unwrapExports`), and verify the WORLD (re-read the file/log/registry externally), not the agent's self-report. A test that fakes the inputs just enough to cover every line will agree with whatever the author assumed; the real thing won't. When a test sets up a *clean/happy* path to reach a line, ask whether the line's PURPOSE is exercised — e.g. a durability/teardown path "tested" by a fully-completed turn never proves the mid-flight teardown it exists for; a torn-tail recovery branch covered by a well-formed log never proves recovery. Flag tests that hit the line but not the scenario. See [docs/testing.md](../../../docs/testing.md) § "Test the real entry path" and § "Prefer the real implementation over a mock".
|
||||
- **Snapshot coverage for transcript/UX changes.** If the PR changes the editor-facing transcript or end-to-end agent UX — the ACP bridge's event→update translation, the agent loop's observable output, tool presentation, or anything an editor renders — it must add or update a snapshot scenario (`examples/*/tests/**/*.snapshot.ts`, goldens under `examples/acp-agent/tests/snapshots/`) or note explicitly why none applies (AGENTS.md § Conventions). Review the golden diff itself: a changed `stdout.golden.txt` / `session.golden.txt` is a behavior change in disguise — confirm it's intended, not an accidental regression someone re-recorded away. A pure internal refactor with no observable-output change is exempt, but the PR should say so. See [docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md](../../../docs/rfc/implemented/testing/2026-06-19-acp-snapshot-tests.md).
|
||||
- **Bilingual docs: review translation quality, not just pairing.** If the PR adds or edits a doc pair, read the changed English and Chinese sides and compare the meaning, not only the mechanical diff. Verify terms against [terminology.md](../../../docs/i18n/terminology.md), including first-occurrence annotations and "do not translate as" prohibitions; if a new term has no established precedent, the PR should keep it in English, list it under `待定术语`, and update the terminology table once the rendering is decided. A green `verify-translation-pairing` only proves hashes, switchers, and structure were recorded — it does not prove the translation is faithful, natural, or correctly termed. Treat [translation-rules.md](../../../docs/i18n/translation-rules.md) MUST/MUST NOT violations as blocking.
|
||||
|
||||
@@ -89,6 +89,7 @@ Real-API tests and demos read `DEEPSEEK_API_KEY` (and optional `DEEPSEEK_BASE_UR
|
||||
- **Plugins, not loop changes**: new behavior goes on the documented extension seams; changing `agent-loop` requires updating docs/architecture.md.
|
||||
- **Capability seams are three packages** — interface / implementation / consumer ([capability seams](docs/rfc/implemented/architecture/2026-06-13-capability-seams.md)); don't split preemptively.
|
||||
- **Explicit > implicit at package seams**: no optional field silently filled by a hidden `?? default` inside `run()`; defaulting is an explicit `resolve(request): Spec` step in the owning implementation (the `dsh-bash` request/spec split is the template).
|
||||
- **No hardcoded tunables in plugins**: anything two deployments could want different — timeouts, caps, grace periods, model names, base URLs — is a defaulted, validated `Config` field, not a literal; a `DEFAULT_*` constant or test-only seam is not configurability. The test: changeable from `cordis.yml`, no code edit. Protocol/wire constants, external-spec values, security invariants stay hardcoded.
|
||||
- **Opaque cross-boundary ids are branded** (`Branded<B>` from `dsh-brand`), never bare `string` ([branded IDs](docs/rfc/implemented/architecture/2026-06-20-branded-ids.md)).
|
||||
- **An empty `catch` names what it swallows** and why nothing else can reach it; keep the `try` to one statement.
|
||||
- **Symmetry is usually more correct**: parallel values get parallel form; asymmetry is a smell for a missed extraction.
|
||||
|
||||
@@ -10,7 +10,7 @@ Search and fetch share no request schema and no business logic, but they are del
|
||||
|
||||
## Search request and result
|
||||
|
||||
The model-facing tool argument is just a `query`; `maxResults` is a consumer-owned bound (`dsh-tool-web`'s `WEB_SEARCH_MAX_RESULTS`, default `8`) passed through the seam and enforced on the way back — if a provider over-returns, the seam truncates `sources[]` and sets `truncated`.
|
||||
The model-facing tool argument is just a `query`; `maxResults` is a consumer-owned bound (`dsh-tool-web`'s `searchMaxResults` config, default `8`) passed through the seam and enforced on the way back — if a provider over-returns, the seam truncates `sources[]` and sets `truncated`.
|
||||
|
||||
```ts type-equiv
|
||||
interface WebSearchRequest {
|
||||
|
||||
@@ -175,7 +175,7 @@ The first `web_search` model-facing tool should be small. The only model-facing
|
||||
|
||||
- `query`: required string.
|
||||
|
||||
`max_results` is NOT exposed to the model in the first version. It is a `dsh-tool-web`-layer decision: the tool sets the result bound — a default of `8` (aligning with OpenCode's Exa default), as an exported constant mirroring `dsh-tool-fs`'s `READ_LIMIT` / `GREP_LIMIT` — and passes it to the seam as `maxResults` on the `WebSearchRequest`. Keeping it off the model schema means the model just asks a question and the product controls how much context comes back; the field can be promoted to a model-facing argument later without breaking the seam.
|
||||
`max_results` is NOT exposed to the model in the first version. It is a `dsh-tool-web`-layer decision: the tool sets the result bound — the `searchMaxResults` plugin config, default `8` (aligning with OpenCode's Exa default), mirroring `dsh-tool-fs`'s `readLimit` — and passes it to the seam as `maxResults` on the `WebSearchRequest`. Keeping it off the model schema means the model just asks a question and the product controls how much context comes back; the field can be promoted to a model-facing argument later without breaking the seam.
|
||||
|
||||
`maxResults` flows tool → seam → provider, and the bound is enforced on the way back:
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@ Two forces shape the design. First, compaction is **swappable**: token counting
|
||||
Per the [capability-seams RFC](../../implemented/architecture/2026-06-13-capability-seams.md), compaction ships as separate packages so the contract, the algorithm, and (later) the consumer surface evolve independently:
|
||||
|
||||
1. **Interface** — `@deepseek-ai/dsh-compact`: an abstract `CompactService` owning the `ctx.compact` key, the `CompactionResult` vocabulary, and the `compact/*` session events. It declares `compactIfNeeded()` and `compactRegion()` as **abstract** — the contract states *what* compaction does, not *how*.
|
||||
2. **Implementation** — `@deepseek-ai/dsh-compact-basic`: a concrete `BasicCompactService` that owns the entire algorithm — token estimation (char/4 + per-block overhead), the tail→head retention walk, summarization via `ctx.llm.stream()`, the surface replacement, the lock, and the `agent/pre-step` auto-compaction listener. A tokenizer-based or template-based backend is a sibling package (or a subclass overriding the two protected estimation/summarization hooks).
|
||||
2. **Implementation** — `@deepseek-ai/dsh-compact-basic`: a concrete `BasicCompactService` that owns the entire algorithm — token estimation (chars per token — the `charsPerToken` config, default 4 — + per-block overhead), the tail→head retention walk, summarization via `ctx.llm.stream()`, the surface replacement, the lock, and the `agent/pre-step` auto-compaction listener. A tokenizer-based or template-based backend is a sibling package (or a subclass overriding the two protected estimation/summarization hooks).
|
||||
3. **Consumer** — deferred. A `/compact` tool and slash command will `inject: ['compact']` and call the contract; they are intentionally out of scope here so the seam settles first.
|
||||
|
||||
### The contract depends on `dsh-session` and `dsh-llm` — a deliberate deviation
|
||||
|
||||
@@ -6,7 +6,7 @@ Status: proposed
|
||||
|
||||
The [fs seam split](../../implemented/simplification/2026-06-26-fsspec-style-fs-seam.md) moved read routing and policy out of the backend into `dsh-tool-fs` and `dsh-fs-policy`. Four pieces of surface kept the pre-split shape — populated on every call, read by nobody:
|
||||
|
||||
1. **`STREAM_MIN_SIZE` + `FsIoInternals.streamMinSize` in `dsh-fs-local`** (`packages/fs/fs-local/src/fsio.ts`, re-exported from `packages/fs/fs-local/src/index.ts`): zero readers anywhere, including fs-local's own source and tests. The backend has no read routing — `readWholeText`/`streamWholeText` are separate primitives the caller chooses between — and the real routing constant lives in the consumer (`packages/fs/tool-fs/src/read.ts`, compared against `info.size`). Two mirrors of the 10 MiB fact; the backend's is dead, and the knob's JSDoc claims a "read routing" override that does not exist.
|
||||
1. **`STREAM_MIN_SIZE` + `FsIoInternals.streamMinSize` in `dsh-fs-local`** — *already removed by the no-hardcoded-tunables audit (the routing bound became `dsh-tool-fs`'s `readStreamMinSize` config); listed here for the record of the full prune, no work remains.* Originally (`packages/fs/fs-local/src/fsio.ts`, re-exported from `packages/fs/fs-local/src/index.ts`): zero readers anywhere, including fs-local's own source and tests. The backend has no read routing — `readWholeText`/`streamWholeText` are separate primitives the caller chooses between — and the real routing constant lives in the consumer (`packages/fs/tool-fs/src/read.ts`, compared against `info.size`). Two mirrors of the 10 MiB fact; the backend's is dead, and the knob's JSDoc claims a "read routing" override that does not exist.
|
||||
2. **`FsTarget.inputPath`** (`packages/fs/fs/src/types.ts`): every backend and every test fake must fabricate a "diagnostics only" value with zero production readers — the policy plugin and every error message use `targetKey`/`displayPath`. The `listDir` producer exposes the semantic wobble: directory children get the bare entry name, which was nobody's "input".
|
||||
3. **`FsEditOutcome.replacements` + `.replaceAll`** (`packages/fs/fs/src/types.ts`): `replacements` has zero production readers (the single-match policy itself stays — it is enforced by the `FS_AMBIGUOUS_EDIT`/`FS_EDIT_NOT_FOUND` throws inside the backend, whose error message keeps the internal count); `replaceAll` is read only by `formatEditOutput` in `packages/fs/tool-fs/src/edit.ts` — as an echo of the `replace_all` argument the tool already holds. Shrunk, `FsEditOutcome` becomes `{ version, before, after }`, parallel to `FsWriteOutcome`'s genuinely backend-discovered fields.
|
||||
4. **`FileReadOutcome.limit` + `.version`** (`packages/fs/tool-fs/src/read-render.ts`): populated by the read tool, but `formatReadOutput` renders `offset`/`lines`/`totalLines`/`truncatedByBytes` only, and the `fs/observed` emit uses `info.version` directly rather than the outcome copy.
|
||||
|
||||
@@ -12,6 +12,7 @@ Local-subprocess implementation of the `@deepseek-ai/dsh-bash` executor seam: `L
|
||||
timeoutMs: 120000 # default foreground timeout
|
||||
maxTimeoutMs: 600000 # cap for per-call overrides
|
||||
maxOutputBytes: 64000 # per-stream in-memory cap; overflow spills to disk
|
||||
graceMs: 3000 # SIGTERM→SIGKILL escalation grace on kills
|
||||
```
|
||||
|
||||
## Behavior (and where it came from)
|
||||
@@ -19,7 +20,7 @@ Local-subprocess implementation of the `@deepseek-ai/dsh-bash` executor seam: `L
|
||||
Design surveyed against the bash tools of Claude Code, OpenCode, Codex, and pi; the notable choices:
|
||||
|
||||
- **Spawn per call, no shell state** — every call is a fresh non-login `bash -c` (deterministic; no rc files). All four surveyed tools spawn per call. `XXX(stateful-shell)` in `src/run.ts` records the two proven stateful designs (Claude Code's cwd-only persistence; Codex's PTY exec sessions) for when real workflows demand them.
|
||||
- **Process-group kills with escalation** — children are spawned `detached` (own process group); kills send SIGTERM to the group, then SIGKILL after a 3s grace (OpenCode's escalation; pipelines and subshells die with the parent). ESRCH is tolerated; daemons that re-parent away from the group can still survive — same caveat as the surveyed tools.
|
||||
- **Process-group kills with escalation** — children are spawned `detached` (own process group); kills send SIGTERM to the group, then SIGKILL after the `graceMs` grace (default 3s — OpenCode's escalation; pipelines and subshells die with the parent). ESRCH is tolerated; daemons that re-parent away from the group can still survive — same caveat as the surveyed tools.
|
||||
- **Tail-keep truncation + spill files** — output beyond `maxOutputBytes` keeps the in-memory TAIL (errors/results cluster at the end — pi/OpenCode rationale) while the FULL stream is appended to a temp file whose path is reported when available. If the final spill close reports a delayed writeback failure, the executor still returns the tail but withholds the path rather than advertising a possibly incomplete file.
|
||||
- **Model-friendly env + credential scrub** — `process.env` minus credential-shaped vars (`*KEY*`/`*SECRET*`/`*TOKEN*`), then `NO_COLOR=1 TERM=dumb PAGER=cat GIT_PAGER=cat` (Codex's hardcoded set) so pagers and ANSI color don't garble results. This scrub is the security control that keeps the harness's *ambient* credentials out of a spawned command. A spec's `env` is merged LAST (after the scrub), so a caller's explicit entry — a value it already holds — wins even on a credential-shaped name. The spec's `stdin`, when supplied, is written to the child and closed; with none supplied, fd 0 is `/dev/null` — the exact pre-seam default, so a command that probes stdin's file type is unaffected. Both `env`/`stdin` are set by in-process plugins (the hooks bridges); the model-facing tool doesn't expose them. See [the bash-stdin-env RFC](../../../docs/rfc/implemented/architecture/2026-06-30-bash-stdin-env-trusted-plugin-surface.md).
|
||||
- **Background tasks** — `start()` returns immediately, no timeout applies (Claude Code detaches timeouts when backgrounding), `readOutput()` is incremental with whole-stream byte offsets, and disposal kills everything. The spec's opaque `owner` token is stored on the tracked task and returned by `ownerOf(id)` — the executor never interprets it (the consumer's access policy does), and because it lives with the task here it survives a `tool-bash` HMR reload.
|
||||
|
||||
@@ -17,7 +17,7 @@ import { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import { BashExecutor, BashTaskId } from '@deepseek-ai/dsh-bash'
|
||||
import type { BashExecRequest, BashExecSpec, BashRunResult, BashTask, BashTaskRead, OwnerToken } from '@deepseek-ai/dsh-bash'
|
||||
import { runBash } from './run.ts'
|
||||
import { DEFAULT_GRACE_MS, runBash } from './run.ts'
|
||||
import type { RunInternals, RunningBash } from './run.ts'
|
||||
|
||||
export { DEFAULT_GRACE_MS, ENV_OVERRIDES, killGroup, OutputCollector, runBash } from './run.ts'
|
||||
@@ -33,6 +33,8 @@ export interface Config {
|
||||
maxTimeoutMs?: number
|
||||
/** Per-stream in-memory output cap; overflow spills to a temp file. */
|
||||
maxOutputBytes?: number
|
||||
/** Grace period between the SIGTERM and the SIGKILL escalation on a kill. */
|
||||
graceMs?: number
|
||||
}
|
||||
|
||||
/** The shape after schemastery applied the defaults (cwd has none). */
|
||||
@@ -57,7 +59,7 @@ interface TrackedTask extends BashTask {
|
||||
* Local-subprocess bash executor. Defaults follow the agent-tool survey
|
||||
* consensus: 120s default / 600s max timeout (Claude Code, OpenCode), 64KB
|
||||
* in-memory output with full-stream spill files (pi, OpenCode),
|
||||
* process-group SIGTERM→SIGKILL kills (OpenCode).
|
||||
* process-group SIGTERM→SIGKILL kills with a 3s grace (OpenCode).
|
||||
*/
|
||||
export class LocalBashExecutor extends BashExecutor {
|
||||
static Config: z<Config> = z.object({
|
||||
@@ -65,11 +67,12 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
timeoutMs: z.number().default(120_000),
|
||||
maxTimeoutMs: z.number().default(600_000),
|
||||
maxOutputBytes: z.number().default(64_000),
|
||||
graceMs: z.number().default(DEFAULT_GRACE_MS),
|
||||
})
|
||||
|
||||
private tasks = new Map<BashTaskId, TrackedTask>()
|
||||
private nextTaskId = 1
|
||||
/** Test seam: timer/spill knobs forwarded to runBash. */
|
||||
/** Test seam: spill knobs forwarded to runBash. */
|
||||
internals: RunInternals = {}
|
||||
|
||||
/** Validated config (schemastery applied the defaults before construction). */
|
||||
@@ -83,6 +86,7 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
assertPositiveFinite('timeoutMs', this.config.timeoutMs)
|
||||
assertPositiveFinite('maxTimeoutMs', this.config.maxTimeoutMs)
|
||||
assertPositiveFinite('maxOutputBytes', this.config.maxOutputBytes)
|
||||
assertPositiveFinite('graceMs', this.config.graceMs)
|
||||
ctx.effect(() => async () => {
|
||||
// Kill every live process group and WAIT for the processes to close so
|
||||
// nothing outlives the fiber (HMR safety) — a TERM-trapping child is
|
||||
@@ -132,6 +136,7 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
cwd: spec.workdir,
|
||||
timeoutMs: spec.timeoutMs,
|
||||
maxOutputBytes: this.config.maxOutputBytes,
|
||||
graceMs: this.config.graceMs,
|
||||
signal: spec.signal,
|
||||
stdin: spec.stdin,
|
||||
env: spec.env,
|
||||
@@ -150,6 +155,7 @@ export class LocalBashExecutor extends BashExecutor {
|
||||
cwd: spec.workdir,
|
||||
timeoutMs: 0,
|
||||
maxOutputBytes: this.config.maxOutputBytes,
|
||||
graceMs: this.config.graceMs,
|
||||
signal: spec.signal,
|
||||
stdin: spec.stdin,
|
||||
env: spec.env,
|
||||
|
||||
@@ -73,6 +73,8 @@ export interface SpawnSpec {
|
||||
timeoutMs: number
|
||||
/** Per-stream in-memory cap; overflow spills to disk (tail kept in memory). */
|
||||
maxOutputBytes: number
|
||||
/** Grace period between the SIGTERM and the SIGKILL escalation on a kill. */
|
||||
graceMs: number
|
||||
/** Abort signal — kills the process group when fired. */
|
||||
signal?: AbortSignal | undefined
|
||||
/**
|
||||
@@ -100,15 +102,13 @@ export interface SpawnOutcome {
|
||||
stderr: CollectedOutput
|
||||
}
|
||||
|
||||
/** Injectable knobs so tests can exercise escalation/spill without long waits. */
|
||||
/** Injectable knobs so tests can exercise spill behavior without the OS tmpdir. */
|
||||
export interface RunInternals {
|
||||
/** Grace period between SIGTERM and SIGKILL on the process group. */
|
||||
graceMs?: number
|
||||
/** Directory for spill files (defaults to the OS temp dir). */
|
||||
spillDir?: string
|
||||
}
|
||||
|
||||
/** Default SIGTERM→SIGKILL grace period (matches OpenCode's 3s). */
|
||||
/** Default SIGTERM→SIGKILL grace period (the `graceMs` config; matches OpenCode's 3s). */
|
||||
export const DEFAULT_GRACE_MS = 3_000
|
||||
|
||||
let spillCounter = 0
|
||||
@@ -292,7 +292,6 @@ export interface RunningBash {
|
||||
* no inherited shell state); revisit when real workflows demand it.
|
||||
*/
|
||||
export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningBash {
|
||||
const graceMs = internals.graceMs ?? DEFAULT_GRACE_MS
|
||||
const spillDir = internals.spillDir ?? privateSpillDir()
|
||||
|
||||
if (spec.signal?.aborted) {
|
||||
@@ -331,7 +330,7 @@ export function runBash(spec: SpawnSpec, internals: RunInternals = {}): RunningB
|
||||
const kill = (): void => {
|
||||
if (graceTimer !== undefined) return // escalation already in flight
|
||||
killGroup(pid, 'SIGTERM')
|
||||
graceTimer = setTimeout(() => { killGroup(pid, 'SIGKILL') }, graceMs)
|
||||
graceTimer = setTimeout(() => { killGroup(pid, 'SIGKILL') }, spec.graceMs)
|
||||
}
|
||||
|
||||
if (spec.timeoutMs > 0) {
|
||||
|
||||
@@ -11,9 +11,10 @@ const spillDir = mkdtempSync(join(tmpdir(), 'dsh-bash-exec-spec-'))
|
||||
|
||||
async function setup(config: ConstructorParameters<typeof LocalBashExecutor>[1] = {}) {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LocalBashExecutor, config)
|
||||
// A short kill grace via the REAL config path, so escalation tests stay fast.
|
||||
await ctx.plugin(LocalBashExecutor, { graceMs: 200, ...config })
|
||||
const bash = ctx.bash as LocalBashExecutor
|
||||
bash.internals = { spillDir, graceMs: 200 }
|
||||
bash.internals = { spillDir }
|
||||
return { ctx, bash }
|
||||
}
|
||||
|
||||
@@ -80,12 +81,22 @@ describe('LocalBashExecutor.run', () => {
|
||||
await expect(setup({ timeoutMs: Number.NaN })).rejects.toThrow(/timeoutMs/)
|
||||
await expect(setup({ maxTimeoutMs: 0 })).rejects.toThrow(/maxTimeoutMs/)
|
||||
await expect(setup({ maxOutputBytes: -1 })).rejects.toThrow(/maxOutputBytes/)
|
||||
await expect(setup({ graceMs: 0 })).rejects.toThrow(/graceMs/)
|
||||
|
||||
const { bash } = await setup()
|
||||
expect(() => bash.resolve({ command: 'true', timeoutMs: Number.NaN })).toThrow(/request\.timeoutMs/)
|
||||
expect(() => bash.resolve({ command: 'true', timeoutMs: -1 })).toThrow(/request\.timeoutMs/)
|
||||
})
|
||||
|
||||
it('kill escalation uses the configured graceMs (a TERM-trapping task dies by SIGKILL)', async () => {
|
||||
const { bash } = await setup() // setup pins graceMs: 200 via config
|
||||
const task = bash.start(bash.resolve({ command: 'trap \'\' TERM; sleep 60' }))
|
||||
await new Promise(resolve => setTimeout(resolve, 100))
|
||||
bash.kill(task.id)
|
||||
await task.done
|
||||
expect(task.signal).toBe('SIGKILL')
|
||||
})
|
||||
|
||||
it('per-call timeout takes precedence under the cap and kills on expiry', async () => {
|
||||
const { bash } = await setup({ timeoutMs: 60_000 })
|
||||
const result = await bash.run(bash.resolve({ command: 'sleep 60', timeoutMs: 100 }))
|
||||
@@ -271,9 +282,9 @@ describe('LocalBashExecutor background tasks', () => {
|
||||
|
||||
it('disposing with already-finished tasks only kills the running ones', async () => {
|
||||
const ctx = new Context()
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, {})
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as LocalBashExecutor
|
||||
bash.internals = { spillDir, graceMs: 200 }
|
||||
bash.internals = { spillDir }
|
||||
|
||||
const finished = bash.start(bash.resolve({ command: 'true' }))
|
||||
await finished.done
|
||||
@@ -288,9 +299,9 @@ describe('LocalBashExecutor background tasks', () => {
|
||||
|
||||
it('disposing the executor fiber kills running tasks (no orphans)', async () => {
|
||||
const ctx = new Context()
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, {})
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as LocalBashExecutor
|
||||
bash.internals = { spillDir, graceMs: 200 }
|
||||
bash.internals = { spillDir }
|
||||
const listener = vi.fn()
|
||||
bash.onTaskDone(listener)
|
||||
|
||||
@@ -337,9 +348,9 @@ describe('review fixes: lifecycle hardening', () => {
|
||||
|
||||
it('dispose AWAITS a TERM-trapping process (SIGKILL escalation included)', async () => {
|
||||
const ctx = new Context()
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, {})
|
||||
const fiber = await ctx.plugin(LocalBashExecutor, { graceMs: 200 })
|
||||
const bash = ctx.bash as LocalBashExecutor
|
||||
bash.internals = { spillDir, graceMs: 200 }
|
||||
bash.internals = { spillDir }
|
||||
|
||||
const task = bash.start(bash.resolve({ command: 'trap \'\' TERM; sleep 60' }))
|
||||
await new Promise(resolve => setTimeout(resolve, 100))
|
||||
|
||||
@@ -28,6 +28,7 @@ function spec(command: string, overrides: Partial<Parameters<typeof runBash>[0]>
|
||||
cwd: process.cwd(),
|
||||
timeoutMs: 0,
|
||||
maxOutputBytes: 64_000,
|
||||
graceMs: 3_000,
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
@@ -106,7 +107,7 @@ describe('runBash', () => {
|
||||
})
|
||||
|
||||
it('escalates to SIGKILL when SIGTERM is trapped', async () => {
|
||||
const running = runBash(spec('trap \'\' TERM; echo ready; sleep 60'), { graceMs: 200 })
|
||||
const running = runBash(spec('trap \'\' TERM; echo ready; sleep 60', { graceMs: 200 }))
|
||||
await waitForStdout(running, 'ready\n')
|
||||
running.kill()
|
||||
const result = await running.done
|
||||
|
||||
@@ -21,8 +21,8 @@ async function setup() {
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(AgentRegistry)
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir, graceMs: 200 }
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000, graceMs: 200 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir }
|
||||
await ctx.plugin(ToolBash)
|
||||
return ctx
|
||||
}
|
||||
@@ -184,8 +184,8 @@ describe('bash tool', () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(LocalBashExecutor, { maxOutputBytes: 100 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir, graceMs: 200 }
|
||||
await ctx.plugin(LocalBashExecutor, { maxOutputBytes: 100, graceMs: 200 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir }
|
||||
await ctx.plugin(ToolBash)
|
||||
const result = await call(ctx, 'bash', { command: 'for i in $(seq 1 100); do printf "line-%04d\\n" $i; done', description: 'test command' })
|
||||
expect(text(result)).toContain('[output truncated; full output: ')
|
||||
@@ -316,8 +316,8 @@ describe('background tools', () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(LocalBashExecutor, { maxOutputBytes: 100 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir, graceMs: 200 }
|
||||
await ctx.plugin(LocalBashExecutor, { maxOutputBytes: 100, graceMs: 200 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir }
|
||||
await ctx.plugin(ToolBash)
|
||||
|
||||
const started = await call(ctx, 'bash', { command: 'for i in $(seq 1 200); do printf "line-%04d\\n" $i; done', description: 'test command', run_in_background: true })
|
||||
@@ -568,8 +568,8 @@ describe('background task ownership (cross-session isolation)', () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir, graceMs: 200 }
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000, graceMs: 200 })
|
||||
;(ctx.bash as LocalBashExecutor).internals = { spillDir }
|
||||
const fiber = await ctx.plugin(ToolBash)
|
||||
|
||||
const a = fakeAgent('sess-a')
|
||||
|
||||
@@ -5,7 +5,7 @@ A three-package capability seam (see [capability seams](../../docs/rfc/implement
|
||||
| Package | Role | ctx key |
|
||||
|---|---|---|
|
||||
| `compact/` | Abstract compaction seam (interface + `compact/*` events + `CompactionResult`) | `ctx.compact` |
|
||||
| `compact-basic/` | A backend: char/4 estimation + token-budget retention + `llm.stream()` summarization | (registers `ctx.compact`) |
|
||||
| `compact-basic/` | A backend: chars-per-token estimation (`charsPerToken`, default 4) + token-budget retention + `llm.stream()` summarization | (registers `ctx.compact`) |
|
||||
| `tool-compact/` (deferred) | Model-facing `/compact` tool over `ctx.compact` | (registers on `ctx.tools`) |
|
||||
|
||||
The interface lives at `compact/compact/`, the backend at `compact/compact-basic/`. Unlike the bash seam, it depends on `dsh-session` and `dsh-llm` — its verbs are defined over a `Session` and its output is the `ContentBlock` vocabulary, so the contract cannot be expressed without naming them. That deviation from the "interface depends only on cordis" guidance is intentional and recorded in the [compaction capability-seam RFC](../../docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md). A tokenizer- or template-based backend would replace `compact-basic` without touching the interface or the tool.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# @deepseek-ai/dsh-compact-basic
|
||||
|
||||
The **basic compaction backend**: a `BasicCompactService` implementing the `@deepseek-ai/dsh-compact` seam with a char/4 token heuristic, token-budget retention, and summarization routed through the agent request pipeline.
|
||||
The **basic compaction backend**: a `BasicCompactService` implementing the `@deepseek-ai/dsh-compact` seam with a chars-per-token heuristic (the `charsPerToken` config, default 4), token-budget retention, and summarization routed through the agent request pipeline.
|
||||
|
||||
This is the implementation tier of the compaction capability — see the [interface package](../compact/README.md) for the seam and the [capability-seam RFC](../../../docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md) for the design.
|
||||
|
||||
@@ -8,7 +8,7 @@ This is the implementation tier of the compaction capability — see the [interf
|
||||
|
||||
The abstract contract states only WHAT compaction does; this backend owns every HOW decision:
|
||||
|
||||
- **Token estimation** — `estimateContentTokens()`: char/4 with per-block structural overhead (`text`/`reasoning` = `ceil(len/4) + 4`, `tool-call` from name + arguments, `tool-result` recursive, `image` = 85, unknown blocks via JSON length).
|
||||
- **Token estimation** — `estimateContentTokens()`: chars divided by the `charsPerToken` config (default 4) with per-block structural overhead (`text`/`reasoning` = `ceil(len/charsPerToken) + 4`, `tool-call` from name + arguments, `tool-result` recursive, `image` = 85, unknown blocks via JSON length).
|
||||
- **Retention policy** — `compactIfNeeded()` walks the surface nodes tail→head summing per-node token estimates, and retains the smallest tail-run of WHOLE units (a closed step, or a single no-step node such as a pre-step `user/message` or inter-step `steering/message`) whose total reaches `retainTokens`; everything older is compacted. Retention is **turn-agnostic** — turn boundaries play no role, so a single runaway turn that alone exceeds the window compacts its OWN early closed steps rather than being retained verbatim (the failure mode that motivated dropping turn-protection: a tool-heavy turn must stay compactable or the harness dies exactly when compaction is needed). The only structural guard is **tool-pairing balance**: the compacted region's edges are balanced cuts on the surface (no unanswered tool-call crosses either edge), so it never splits a step's `assistant/message` tool-calls from their `tool/result`s. When the only compactable content left is an un-splittable open tail step, it declines (returns `null`) and retries once an older step closes. **Single-unit overflow is out of scope, by design**: if one retained unit (a single closed step, or a large pasted `user/message`) ALONE exceeds the budget, compaction cannot help and the call may go out over-budget — bounding an individual unit's size is a separate concern. `compactRegion()` enforces tool-pairing balance strictly, throwing on a boundary that would split a step. `dsh-session` exports `isToolPairingBalanced` for the check.
|
||||
- **Dynamic convergence** — no static summary-length config pretends to bound what the model will write. If framing/estimator/system overhead leaves the compacted surface above threshold, `compactIfNeeded()` re-compacts the head checkpoint up to `compactionRetries` extra times; if it still cannot get below threshold, it throws. A summary whose estimated stored size is not smaller than the shadowed content fails closed before it mutates the surface.
|
||||
- **Summarization** — `summarize()`: a `GenerateOptions` request assembled via `BlockAssembler` with a fixed system prompt that asks for a structured checkpoint (Primary Request and Intent · Key Technical Concepts · Files and Code · Errors and Fixes · Pending Tasks · Current Work · Next Step · Critical Context), every section mandatory, exact paths/commands/identifiers preserved. The request runs through the `agent/request` waterfall before `ctx.llm.stream()`, so router agents that choose the concrete model there also route compaction summaries. `maxTokens` is the provider-side generation cap; only text blocks from the model's reply are kept before the checkpoint is stored (reasoning is dropped so private chain-of-thought never leaks into the durable summary, and a stray `tool-call` is dropped so the synthesized `user/message` summary cannot land an orphaned call with no matching `tool-result`). The compacted region is flattened to a plain-text transcript first: text and reasoning contribute their text, and every non-text block (image, tool-call, tool-result, plugin-added types) contributes a type-tagged placeholder (`[image]`, `[tool-call: name(args)]`, …) so the summarizer is told what existed rather than silently dropping it.
|
||||
@@ -32,6 +32,7 @@ Every knob is **required** except `auto` — there is no concrete data yet to ju
|
||||
| `maxTokens` | yes | Provider generation cap for the summarization call; may include reasoning tokens. |
|
||||
| `compactionRetries` | yes | Extra compaction attempts after the first if the compacted surface remains over threshold. |
|
||||
| `auto` | no (default `true`) | Register the `agent/pre-step` auto-compaction listener. Set `false` for manual-only. |
|
||||
| `charsPerToken` | no (default `4`) | Token-estimator text density (estimated tokens = chars / `charsPerToken`; may be fractional). The default suits English text; CJK-heavy deployments should set ~1-2 or the estimate undershoots several-fold and compaction fires too late. |
|
||||
|
||||
## Usage
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@deepseek-ai/dsh-compact-basic",
|
||||
"description": "Basic compaction backend (char/4 token estimation + token-budget retention + llm.generate() summarization) for the DeepSeek Harness",
|
||||
"description": "Basic compaction backend (chars-per-token estimation + token-budget retention + llm.generate() summarization) for the DeepSeek Harness",
|
||||
"version": "0.0.1",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
|
||||
@@ -2,7 +2,8 @@
|
||||
* `BasicCompactService`: the first implementation of the
|
||||
* `@deepseek-ai/dsh-compact` seam. It owns the entire compaction strategy:
|
||||
*
|
||||
* - **Token estimation** — char/4 heuristic with per-block structural overhead.
|
||||
* - **Token estimation** — chars/`charsPerToken` heuristic (config, default 4)
|
||||
* with per-block structural overhead.
|
||||
* - **Retention policy** — walk surface nodes tail→head, keep recent nodes up
|
||||
* to a token budget, compact everything older. The cutoff is snapped forward
|
||||
* to the next balanced tool-pairing boundary so a compacted region never
|
||||
@@ -148,9 +149,11 @@ function finishError(finish: FinishReason): Error | undefined {
|
||||
}
|
||||
|
||||
/**
|
||||
* Basic, dependency-light compaction backend. Defaults target a 128K context
|
||||
* window, compacting at 80% utilization and retaining ~20K tokens of recent
|
||||
* context.
|
||||
* Basic, dependency-light compaction backend: estimates the surface's token
|
||||
* footprint, summarizes the stale prefix through the model, and shadows it
|
||||
* behind a durable checkpoint. Every threshold/budget knob is required config
|
||||
* ({@link BasicCompactConfig}); the estimator's text density is the
|
||||
* `charsPerToken` knob.
|
||||
*/
|
||||
export class BasicCompactService extends CompactService {
|
||||
static inject = ['llm']
|
||||
@@ -207,24 +210,27 @@ export class BasicCompactService extends CompactService {
|
||||
|
||||
// ---- Token estimation (overridable hooks) ----
|
||||
|
||||
// TODO: char/4 is a coarse heuristic. Replace with an exact count — a real
|
||||
// tokenizer, or the provider's post-response `usage` (input tokens) fed back
|
||||
// as a correction — so threshold decisions match the model's actual budget.
|
||||
// TODO: chars/charsPerToken is a coarse heuristic. Replace with an exact
|
||||
// count — a real tokenizer, or the provider's post-response `usage` (input
|
||||
// tokens) fed back as a correction — so threshold decisions match the
|
||||
// model's actual budget.
|
||||
/**
|
||||
* Estimate the token count of content blocks — char/4 with per-block
|
||||
* overhead. Override in a subclass to plug in a real tokenizer.
|
||||
* Estimate the token count of content blocks — chars divided by the
|
||||
* `charsPerToken` config, with per-block overhead. Override in a subclass to
|
||||
* plug in a real tokenizer.
|
||||
*/
|
||||
estimateContentTokens(blocks: readonly ContentBlock[]): number {
|
||||
const { charsPerToken } = this.config
|
||||
let tokens = 0
|
||||
for (const block of blocks) {
|
||||
switch (block.type) {
|
||||
case 'text':
|
||||
case 'reasoning':
|
||||
tokens += Math.ceil(block.text.length / 4) + BLOCK_OVERHEAD
|
||||
tokens += Math.ceil(block.text.length / charsPerToken) + BLOCK_OVERHEAD
|
||||
break
|
||||
case 'tool-call':
|
||||
tokens += Math.ceil(block.name.length / 4)
|
||||
+ Math.ceil(block.arguments.length / 4)
|
||||
tokens += Math.ceil(block.name.length / charsPerToken)
|
||||
+ Math.ceil(block.arguments.length / charsPerToken)
|
||||
+ BLOCK_OVERHEAD
|
||||
break
|
||||
case 'tool-result':
|
||||
@@ -236,7 +242,7 @@ export class BasicCompactService extends CompactService {
|
||||
default:
|
||||
// Unknown block types (merge-extensible ContentBlockMap):
|
||||
// estimate conservatively via JSON stringify.
|
||||
tokens += BLOCK_OVERHEAD + Math.ceil(JSON.stringify(block).length / 4)
|
||||
tokens += BLOCK_OVERHEAD + Math.ceil(JSON.stringify(block).length / charsPerToken)
|
||||
}
|
||||
}
|
||||
return tokens
|
||||
@@ -266,7 +272,7 @@ export class BasicCompactService extends CompactService {
|
||||
total += this.estimateContentTokens(msg.content)
|
||||
total += ROLE_OVERHEAD
|
||||
}
|
||||
if (systemPrompt) total += Math.ceil(systemPrompt.length / 4)
|
||||
if (systemPrompt) total += Math.ceil(systemPrompt.length / this.config.charsPerToken)
|
||||
return total
|
||||
}
|
||||
|
||||
|
||||
@@ -10,10 +10,12 @@
|
||||
*/
|
||||
|
||||
/**
|
||||
* Backend configuration. Every knob is REQUIRED except `auto`: there is no
|
||||
* concrete data yet to justify default thresholds/budgets, so a consumer must
|
||||
* state each value explicitly rather than inherit a guessed default. `auto`
|
||||
* alone defaults to `true` (auto-compaction is the intended posture).
|
||||
* Backend configuration. Every knob is REQUIRED except `auto` and
|
||||
* `charsPerToken`: there is no concrete data yet to justify default
|
||||
* thresholds/budgets, so a consumer must state each value explicitly rather
|
||||
* than inherit a guessed default. `auto` alone defaults to `true`
|
||||
* (auto-compaction is the intended posture), and `charsPerToken` defaults to
|
||||
* the English-text heuristic its estimator was calibrated on.
|
||||
*/
|
||||
export interface BasicCompactConfig {
|
||||
/** Context window size in tokens. */
|
||||
@@ -30,13 +32,21 @@ export interface BasicCompactConfig {
|
||||
compactionRetries: number
|
||||
/** Enable automatic compaction on the `agent/pre-step` seam (default true). */
|
||||
auto?: boolean
|
||||
/**
|
||||
* Text density for the token estimator: estimated tokens = chars /
|
||||
* `charsPerToken`. Defaults to 4 (typical English text). A CJK-heavy
|
||||
* deployment should set ~1-2 — CJK runs at roughly 1-2 chars per token, so
|
||||
* the default UNDERestimates several-fold and compaction fires far too late.
|
||||
* May be fractional.
|
||||
*/
|
||||
charsPerToken?: number
|
||||
}
|
||||
|
||||
/** Resolved config with `auto` defaulted. */
|
||||
/** Resolved config with `auto` and `charsPerToken` defaulted. */
|
||||
export type ResolvedConfig = Required<BasicCompactConfig>
|
||||
|
||||
/**
|
||||
* Default `auto` when unset and reject nonsensical numeric knobs.
|
||||
* Default `auto`/`charsPerToken` when unset and reject nonsensical numeric knobs.
|
||||
*
|
||||
* Convergence is not a static config invariant: provider generation caps can be
|
||||
* spent on hidden or surfaced reasoning tokens, and the model may emit a summary
|
||||
@@ -46,13 +56,14 @@ export type ResolvedConfig = Required<BasicCompactConfig>
|
||||
* throwing if the surface still exceeds the threshold.
|
||||
*/
|
||||
export function resolveConfig(config: BasicCompactConfig): ResolvedConfig {
|
||||
const resolved: ResolvedConfig = { auto: true, ...config }
|
||||
const resolved: ResolvedConfig = { auto: true, charsPerToken: 4, ...config }
|
||||
|
||||
assertPositiveInteger('contextWindow', resolved.contextWindow)
|
||||
assertRatio('thresholdRatio', resolved.thresholdRatio)
|
||||
assertNonNegativeInteger('retainTokens', resolved.retainTokens)
|
||||
assertPositiveInteger('maxTokens', resolved.maxTokens)
|
||||
assertNonNegativeInteger('compactionRetries', resolved.compactionRetries)
|
||||
assertPositiveFinite('charsPerToken', resolved.charsPerToken)
|
||||
if (typeof resolved.summarizationModel !== 'string') {
|
||||
throw new Error('BasicCompactConfig: summarizationModel must be a string.')
|
||||
}
|
||||
@@ -74,6 +85,12 @@ function assertNonNegativeInteger(name: string, value: number): void {
|
||||
}
|
||||
}
|
||||
|
||||
function assertPositiveFinite(name: string, value: number): void {
|
||||
if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) {
|
||||
throw new Error(`BasicCompactConfig: ${name} (${value}) must be a positive finite number.`)
|
||||
}
|
||||
}
|
||||
|
||||
function assertRatio(name: string, value: number): void {
|
||||
if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0 || value > 1) {
|
||||
throw new Error(`BasicCompactConfig: ${name} (${value}) must be a number in (0, 1].`)
|
||||
|
||||
@@ -820,6 +820,19 @@ describe('BasicCompactService token estimation (char/4 heuristic)', () => {
|
||||
const svc = new BasicCompactService(new Context(), cfg({ auto: false }))
|
||||
expect(svc.estimateContentTokens([])).toBe(0)
|
||||
})
|
||||
|
||||
it('honors a configured charsPerToken (fractional densities included)', () => {
|
||||
// 'this is a somewhat longer text block' = 36 chars.
|
||||
const blocks: ContentBlock[] = [{ type: 'text', text: 'this is a somewhat longer text block' }]
|
||||
// charsPerToken 2: ceil(36/2)+4 = 22 — a CJK-density config doubles the estimate.
|
||||
const dense = new BasicCompactService(new Context(), cfg({ auto: false, charsPerToken: 2 }))
|
||||
expect(dense.estimateContentTokens(blocks)).toBe(22)
|
||||
// Fractional density is legal: ceil(36/1.5)+4 = 28.
|
||||
const fractional = new BasicCompactService(new Context(), cfg({ auto: false, charsPerToken: 1.5 }))
|
||||
expect(fractional.estimateContentTokens(blocks)).toBe(28)
|
||||
// The system-prompt term scales with the same knob: 36-char prompt at density 2 → ceil(36/2) = 18.
|
||||
expect(dense.estimateTokens([], 'this is a somewhat longer text block')).toBe(18)
|
||||
})
|
||||
})
|
||||
|
||||
describe('BasicCompactService HMR safety', () => {
|
||||
@@ -862,6 +875,10 @@ describe('BasicCompactService config validation', () => {
|
||||
)).toThrow(/summarizationModel must be a string/)
|
||||
expect(() => new BasicCompactService(new Context(), cfg({ auto: 'no' } as unknown as Partial<BasicCompactConfig>)))
|
||||
.toThrow(/auto must be a boolean/)
|
||||
expect(() => new BasicCompactService(new Context(), cfg({ auto: false, charsPerToken: 0 })))
|
||||
.toThrow(/charsPerToken .* positive finite number/)
|
||||
expect(() => new BasicCompactService(new Context(), cfg({ auto: false, charsPerToken: Number.NaN })))
|
||||
.toThrow(/charsPerToken .* positive finite number/)
|
||||
})
|
||||
|
||||
it('accepts a large retain budget because convergence is enforced dynamically', () => {
|
||||
|
||||
@@ -7,7 +7,7 @@ This package is the interface tier of the compaction capability, split so each c
|
||||
| Package | Role |
|
||||
|---|---|
|
||||
| `@deepseek-ai/dsh-compact` (this) | the interface: abstract service + `compact/*` events + `CompactionResult` |
|
||||
| `@deepseek-ai/dsh-compact-basic` (deferred) | a backend: char/4 estimation + token-budget retention + `llm.stream()` summarization |
|
||||
| `@deepseek-ai/dsh-compact-basic` (deferred) | a backend: chars-per-token estimation (`charsPerToken`, default 4) + token-budget retention + `llm.stream()` summarization |
|
||||
| `@deepseek-ai/dsh-tool-compact` (deferred) | the model-facing `/compact` tool over `ctx.compact` |
|
||||
|
||||
Unlike the bash seam, this interface depends on `@deepseek-ai/dsh-session` and `@deepseek-ai/dsh-llm` — the contract's verbs are defined over a `Session` and its output is the `ContentBlock` vocabulary, so they cannot be expressed without naming those packages. That deviation from the "interface depends only on cordis" guidance is intentional and recorded in the [compaction capability-seam RFC](../../../docs/rfc/implemented/feature/2026-06-18-compaction-capability-seam.md).
|
||||
|
||||
@@ -26,9 +26,6 @@ import { basename, dirname, join, resolve } from 'node:path'
|
||||
import { TextDecoder } from 'node:util'
|
||||
import { FsError, FsTargetKey, FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
|
||||
/** Files at or above this size stream their text; smaller files read whole. */
|
||||
export const STREAM_MIN_SIZE = 10 * 1024 * 1024
|
||||
|
||||
const BINARY_SAMPLE_BYTES = 8192
|
||||
|
||||
function isENOENT(error: unknown): boolean {
|
||||
@@ -85,13 +82,10 @@ function versionOf(info: Stats): FsVersion {
|
||||
}
|
||||
|
||||
/**
|
||||
* Test seam: lets specs force the streaming read path (via a small
|
||||
* `streamMinSize`) and pin the temp-file name (to prove exclusive-open
|
||||
* behavior) without a 10 MB fixture or a name race.
|
||||
* Test seam: lets specs pin the temp-file name (to prove exclusive-open
|
||||
* behavior) without a name race.
|
||||
*/
|
||||
export interface FsIoInternals {
|
||||
/** Override {@link STREAM_MIN_SIZE} for read routing. */
|
||||
streamMinSize?: number
|
||||
/** Override the generated private staging-dir name (relative to the target dir). */
|
||||
tempDirName?: (writePath: string) => string
|
||||
/** Override the generated temp-file name (relative to the private staging dir). */
|
||||
|
||||
@@ -41,7 +41,6 @@ import {
|
||||
import type { FsIoInternals } from './fsio.ts'
|
||||
|
||||
export {
|
||||
STREAM_MIN_SIZE,
|
||||
applyLiteralEdit,
|
||||
listDirectory,
|
||||
probe,
|
||||
|
||||
@@ -11,11 +11,22 @@ await ctx.plugin(ToolFs) // this package — re
|
||||
|
||||
`@deepseek-ai/dsh-fs-policy` is **optional**: omit it and the tools run against the bare provider (unconditional write/overwrite/edit, no observed-state). A deployment that loads these tools is expected to also load it, so the behavior is read-before-write/edit.
|
||||
|
||||
## Config
|
||||
|
||||
All keys are optional; the defaults are the shipped read caps.
|
||||
|
||||
| Key | Default | Meaning |
|
||||
|---|---|---|
|
||||
| `readLimit` | `2000` | Default and maximum lines returned by one `read` call (the tool schema advertises it as the `limit` default). |
|
||||
| `readMaxLineLength` | `2000` | Characters kept per line before truncation (the suffix names the cap). |
|
||||
| `readMaxBytes` | `51200` | Byte cap on one `read` call's selected lines; overflow ends the window with a "capped" footer. |
|
||||
| `readStreamMinSize` | `10485760` | Files at or above this size (or with unknown size) stream instead of loading whole into memory. |
|
||||
|
||||
## Tools (schemas per [the filesystem tool schemas RFC](../../../docs/rfc/implemented/feature/2026-06-17-filesystem-tool-schemas.md))
|
||||
|
||||
| Tool | Arguments | Behavior |
|
||||
|---|---|---|
|
||||
| `read` | `file_path`, `offset?`, `limit?` | Line-numbered UTF-8 content with a pagination footer. `offset` is 1-based; `limit` defaults to and caps at 2000 lines. |
|
||||
| `read` | `file_path`, `offset?`, `limit?` | Line-numbered UTF-8 content with a pagination footer. `offset` is 1-based; `limit` defaults to and caps at the configured `readLimit` (2000). |
|
||||
| `write` | `file_path`, `content` | Create or fully replace a file. With the policy plugin: overwriting an existing file requires a prior `read` at the unchanged version; creating a new file does not. Without it: unconditional. |
|
||||
| `edit` | `file_path`, non-empty `old_string`, `new_string`, `replace_all?` | Literal replacement; unique match required unless `replace_all` is true. With the policy plugin: requires a prior `read` (any window) and the file unchanged since. Without it: unconditional. |
|
||||
|
||||
|
||||
@@ -22,7 +22,8 @@
|
||||
],
|
||||
"license": "BSD-3-Clause",
|
||||
"dependencies": {
|
||||
"diff": "^9.0.0"
|
||||
"diff": "^9.0.0",
|
||||
"schemastery": "^3.18.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@deepseek-ai/dsh-fs": "^0.0.1",
|
||||
|
||||
@@ -23,11 +23,14 @@
|
||||
*/
|
||||
|
||||
import type { Context } from 'cordis'
|
||||
import { applyReadTool } from './read.ts'
|
||||
import z from 'schemastery'
|
||||
import { applyReadTool, READ_LIMIT, STREAM_MIN_SIZE } from './read.ts'
|
||||
import { applyWriteTool } from './write.ts'
|
||||
import { applyEditTool } from './edit.ts'
|
||||
import { READ_MAX_BYTES, READ_MAX_LINE_LENGTH } from './read-render.ts'
|
||||
|
||||
export { READ_LIMIT, STREAM_MIN_SIZE, applyReadTool, parseReadArgs } from './read.ts'
|
||||
export type { ReadToolCaps } from './read.ts'
|
||||
export { applyWriteTool, formatWriteOutput, parseWriteArgs } from './write.ts'
|
||||
export { applyEditTool, formatEditOutput, parseEditArgs } from './edit.ts'
|
||||
export { READ_MAX_BYTES, READ_MAX_LINE_LENGTH, buildWindow, formatReadOutput } from './read-render.ts'
|
||||
@@ -41,9 +44,49 @@ export const name = 'tool-fs'
|
||||
/** Services required by the filesystem tool suite. */
|
||||
export const inject = ['tools', 'fs', 'systemPrompt']
|
||||
|
||||
/** Plugin config (all optional — `Config` supplies the defaults). */
|
||||
export interface Config {
|
||||
/** Default and maximum number of lines returned by one `read` call. */
|
||||
readLimit?: number
|
||||
/** Maximum characters returned for a single line before truncation. */
|
||||
readMaxLineLength?: number
|
||||
/** Maximum bytes returned for the selected lines of one `read` call. */
|
||||
readMaxBytes?: number
|
||||
/** Files at or above this size stream instead of loading whole into memory. */
|
||||
readStreamMinSize?: number
|
||||
}
|
||||
|
||||
export const Config: z<Config> = z.object({
|
||||
readLimit: z.number().default(READ_LIMIT),
|
||||
readMaxLineLength: z.number().default(READ_MAX_LINE_LENGTH),
|
||||
readMaxBytes: z.number().default(READ_MAX_BYTES),
|
||||
readStreamMinSize: z.number().default(STREAM_MIN_SIZE),
|
||||
})
|
||||
|
||||
/** The shape after schemastery applied the defaults. */
|
||||
type ResolvedConfig = Required<Config>
|
||||
|
||||
/** Every read cap counts lines/chars/bytes — a positive integer, or windowing arithmetic misbehaves silently. */
|
||||
function assertPositiveInteger(name: string, value: number): void {
|
||||
if (!Number.isInteger(value) || value < 1) {
|
||||
throw new Error(`tool-fs: ${name} must be a positive integer`)
|
||||
}
|
||||
}
|
||||
|
||||
/** Register the full `read`/`write`/`edit` filesystem tool suite. */
|
||||
export function apply(ctx: Context): void {
|
||||
applyReadTool(ctx)
|
||||
export function apply(ctx: Context, config: Config): void {
|
||||
// schemastery (Config) has already filled every defaulted field.
|
||||
const resolved = config as ResolvedConfig
|
||||
assertPositiveInteger('readLimit', resolved.readLimit)
|
||||
assertPositiveInteger('readMaxLineLength', resolved.readMaxLineLength)
|
||||
assertPositiveInteger('readMaxBytes', resolved.readMaxBytes)
|
||||
assertPositiveInteger('readStreamMinSize', resolved.readStreamMinSize)
|
||||
applyReadTool(ctx, {
|
||||
limit: resolved.readLimit,
|
||||
maxLineLength: resolved.readMaxLineLength,
|
||||
maxBytes: resolved.readMaxBytes,
|
||||
streamMinSize: resolved.readStreamMinSize,
|
||||
})
|
||||
applyWriteTool(ctx)
|
||||
applyEditTool(ctx)
|
||||
}
|
||||
|
||||
@@ -19,21 +19,22 @@
|
||||
import { FsError } from '@deepseek-ai/dsh-fs'
|
||||
import type { FsVersion } from '@deepseek-ai/dsh-fs'
|
||||
|
||||
/** Maximum characters returned for a single line. */
|
||||
/** Default maximum characters returned for a single line (the `readMaxLineLength` config). */
|
||||
export const READ_MAX_LINE_LENGTH = 2000
|
||||
|
||||
/** Maximum bytes returned for selected file lines. */
|
||||
/** Default maximum bytes returned for selected file lines (the `readMaxBytes` config). */
|
||||
export const READ_MAX_BYTES = 50 * 1024
|
||||
|
||||
const READ_MAX_LINE_SUFFIX = `... (line truncated to ${READ_MAX_LINE_LENGTH} chars)`
|
||||
const LINE_BUFFER_CAP = READ_MAX_LINE_LENGTH + 1
|
||||
|
||||
/** Resolved read window. The consumer applies its defaults/caps before calling. */
|
||||
export interface ReadWindow {
|
||||
/** 1-based first line to return. */
|
||||
offset: number
|
||||
/** Maximum number of lines to return. */
|
||||
limit: number
|
||||
/** Maximum characters returned for a single line; overflow is truncated with a suffix. */
|
||||
maxLineLength: number
|
||||
/** Maximum bytes of selected output; overflow stops the scan and marks `truncatedByBytes`. */
|
||||
maxBytes: number
|
||||
}
|
||||
|
||||
/** One line returned from a text file. */
|
||||
@@ -82,8 +83,8 @@ function newAccumulator(): WindowAccumulator {
|
||||
return { lines: [], totalLines: 0, outputBytes: 0, truncatedByBytes: false, done: false }
|
||||
}
|
||||
|
||||
function truncateLine(line: string): string {
|
||||
return line.length > READ_MAX_LINE_LENGTH ? `${line.substring(0, READ_MAX_LINE_LENGTH)}${READ_MAX_LINE_SUFFIX}` : line
|
||||
function truncateLine(line: string, maxLineLength: number): string {
|
||||
return line.length > maxLineLength ? `${line.substring(0, maxLineLength)}... (line truncated to ${maxLineLength} chars)` : line
|
||||
}
|
||||
|
||||
function lineByteSize(line: string, currentLineCount: number): number {
|
||||
@@ -94,9 +95,9 @@ function consumeLine(acc: WindowAccumulator, rawLine: string, request: ReadWindo
|
||||
acc.totalLines += 1
|
||||
if (acc.totalLines < request.offset || acc.lines.length >= request.limit) return
|
||||
|
||||
const text = truncateLine(rawLine)
|
||||
const text = truncateLine(rawLine, request.maxLineLength)
|
||||
const bytes = lineByteSize(text, acc.lines.length)
|
||||
if (acc.outputBytes + bytes > READ_MAX_BYTES) {
|
||||
if (acc.outputBytes + bytes > request.maxBytes) {
|
||||
acc.truncatedByBytes = true
|
||||
acc.done = true
|
||||
return
|
||||
@@ -121,7 +122,7 @@ function finish(acc: WindowAccumulator, request: ReadWindow, displayPath: string
|
||||
* Accepts an `AsyncIterable<string>` (a chunked `streamText`) or an
|
||||
* `Iterable<string>` (a whole-file `readText` wrapped as `[text]`), so one code
|
||||
* path serves both. Scans for newlines with a capped line buffer (a newline-free
|
||||
* giant line is truncated, never buffered past {@link READ_MAX_LINE_LENGTH}),
|
||||
* giant line is truncated, never buffered past `request.maxLineLength`),
|
||||
* enforces the byte cap, and throws `FS_NOT_FOUND` for an offset past EOF.
|
||||
*/
|
||||
export async function buildWindow(
|
||||
@@ -130,12 +131,14 @@ export async function buildWindow(
|
||||
displayPath: string,
|
||||
): Promise<WindowResult> {
|
||||
const acc = newAccumulator()
|
||||
// One char past the truncation point is enough to prove a line overflows.
|
||||
const lineBufferCap = request.maxLineLength + 1
|
||||
let lineBuffer = ''
|
||||
|
||||
function appendToLineBuffer(segment: string): void {
|
||||
if (lineBuffer.length >= LINE_BUFFER_CAP) return
|
||||
if (lineBuffer.length >= lineBufferCap) return
|
||||
lineBuffer += segment
|
||||
if (lineBuffer.length > LINE_BUFFER_CAP) lineBuffer = lineBuffer.slice(0, LINE_BUFFER_CAP)
|
||||
if (lineBuffer.length > lineBufferCap) lineBuffer = lineBuffer.slice(0, lineBufferCap)
|
||||
}
|
||||
|
||||
function flushLine(): void {
|
||||
|
||||
@@ -23,12 +23,27 @@ import { buildWindow, formatReadOutput } from './read-render.ts'
|
||||
import type { FileReadOutcome } from './read-render.ts'
|
||||
import { sessionCwd } from './session-cwd.ts'
|
||||
|
||||
/** Default and maximum number of lines returned by one `read` call. */
|
||||
/** Default and maximum number of lines returned by one `read` call (the `readLimit` config). */
|
||||
export const READ_LIMIT = 2000
|
||||
|
||||
/** Files at or above this size stream; smaller files read whole into memory. */
|
||||
/**
|
||||
* Default streaming threshold (the `readStreamMinSize` config): files at or
|
||||
* above this size stream; smaller files read whole into memory.
|
||||
*/
|
||||
export const STREAM_MIN_SIZE = 10 * 1024 * 1024
|
||||
|
||||
/** Resolved read-tool caps — plugin config after defaulting (see `Config` in index.ts). */
|
||||
export interface ReadToolCaps {
|
||||
/** Default and maximum number of lines returned by one call. */
|
||||
limit: number
|
||||
/** Maximum characters returned for a single line. */
|
||||
maxLineLength: number
|
||||
/** Maximum bytes returned for selected file lines. */
|
||||
maxBytes: number
|
||||
/** Files at or above this size stream; smaller files read whole into memory. */
|
||||
streamMinSize: number
|
||||
}
|
||||
|
||||
/** Validated `read` arguments after defaulting. */
|
||||
interface ReadInput {
|
||||
filePath: string
|
||||
@@ -43,17 +58,17 @@ function parsePositiveInteger(value: number, name: string): number {
|
||||
return value
|
||||
}
|
||||
|
||||
/** Validate value constraints the schema DSL can't express. */
|
||||
export function parseReadArgs(args: { file_path: string; offset?: number; limit?: number }): ReadInput {
|
||||
/** Validate value constraints the schema DSL can't express. `maxLimit` is the deployment's line cap. */
|
||||
export function parseReadArgs(args: { file_path: string; offset?: number; limit?: number }, maxLimit: number): ReadInput {
|
||||
if (args.file_path.trim().length === 0) throw new Error('file_path must be a non-empty string')
|
||||
const offset = args.offset === undefined ? 1 : parsePositiveInteger(args.offset, 'offset')
|
||||
const limit = args.limit === undefined ? READ_LIMIT : parsePositiveInteger(args.limit, 'limit')
|
||||
if (limit > READ_LIMIT) throw new Error(`limit must be less than or equal to ${READ_LIMIT}`)
|
||||
const limit = args.limit === undefined ? maxLimit : parsePositiveInteger(args.limit, 'limit')
|
||||
if (limit > maxLimit) throw new Error(`limit must be less than or equal to ${maxLimit}`)
|
||||
return { filePath: args.file_path, offset, limit }
|
||||
}
|
||||
|
||||
/** Register the `read` tool and its system-prompt guidance. */
|
||||
export function applyReadTool(ctx: Context): void {
|
||||
export function applyReadTool(ctx: Context, caps: ReadToolCaps): void {
|
||||
ctx.systemPrompt.section({
|
||||
name: 'tool:read',
|
||||
order: 100,
|
||||
@@ -66,10 +81,10 @@ export function applyReadTool(ctx: Context): void {
|
||||
parameters: {
|
||||
file_path: { type: 'string', required: true, description: 'Path to read, resolved by the filesystem backend.' },
|
||||
offset: { type: 'number', description: '1-based first line to return. Defaults to 1.' },
|
||||
limit: { type: 'number', description: `Maximum number of lines to return. Defaults to ${READ_LIMIT}.` },
|
||||
limit: { type: 'number', description: `Maximum number of lines to return. Defaults to ${caps.limit}.` },
|
||||
},
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const input = parseReadArgs(args)
|
||||
const input = parseReadArgs(args, caps.limit)
|
||||
const cwd = sessionCwd(exec)
|
||||
const target = await ctx.fs.resolve(input.filePath, cwd !== undefined ? { cwd } : undefined)
|
||||
|
||||
@@ -83,10 +98,14 @@ export function applyReadTool(ctx: Context): void {
|
||||
|
||||
// Stream when the file is large OR size is unknown, so a size-less backend
|
||||
// never buffers an arbitrarily large file.
|
||||
const chunks = info.size === undefined || info.size >= STREAM_MIN_SIZE
|
||||
const chunks = info.size === undefined || info.size >= caps.streamMinSize
|
||||
? await ctx.fs.streamText(target, exec.signal)
|
||||
: [await ctx.fs.readText(target, exec.signal)]
|
||||
const window = await buildWindow(chunks, { offset: input.offset, limit: input.limit }, target.displayPath)
|
||||
const window = await buildWindow(
|
||||
chunks,
|
||||
{ offset: input.offset, limit: input.limit, maxLineLength: caps.maxLineLength, maxBytes: caps.maxBytes },
|
||||
target.displayPath,
|
||||
)
|
||||
|
||||
const outcome: FileReadOutcome = {
|
||||
offset: input.offset,
|
||||
@@ -106,7 +125,8 @@ export function applyReadTool(ctx: Context): void {
|
||||
// appended (`Read foo.txt (5 - 8)`), `read` kind (icon), and a follow-along
|
||||
// location whose line is the read's offset (defaulting to 1). The window is
|
||||
// derived from the RAW args (offset/limit as the model passed them), NOT the
|
||||
// tool's defaulted 1/READ_LIMIT, so an unbounded read shows a bare title.
|
||||
// tool's defaulted 1/configured limit, so an unbounded read shows a bare
|
||||
// title (and the presenter stays a pure function of args, config-free).
|
||||
presentCall(args): GenericCallView {
|
||||
const { offset, limit } = args
|
||||
const window = limit !== undefined && limit > 0
|
||||
|
||||
@@ -6,10 +6,11 @@
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildWindow, READ_MAX_LINE_LENGTH } from '@deepseek-ai/dsh-tool-fs'
|
||||
import { buildWindow, READ_MAX_BYTES, READ_MAX_LINE_LENGTH } from '@deepseek-ai/dsh-tool-fs'
|
||||
import type { ReadWindow } from '@deepseek-ai/dsh-tool-fs'
|
||||
|
||||
const READ_ALL: ReadWindow = { offset: 1, limit: 2000 }
|
||||
const DEFAULT_CAPS = { maxLineLength: READ_MAX_LINE_LENGTH, maxBytes: READ_MAX_BYTES }
|
||||
const READ_ALL: ReadWindow = { offset: 1, limit: 2000, ...DEFAULT_CAPS }
|
||||
|
||||
/** Yield `text` as one chunk (whole-file read shape). */
|
||||
async function* whole(text: string): AsyncIterable<string> {
|
||||
@@ -34,7 +35,7 @@ describe('buildWindow', () => {
|
||||
})
|
||||
|
||||
it('applies offset/limit', async () => {
|
||||
const result = await buildWindow(whole('one\ntwo\nthree\nfour'), { offset: 2, limit: 2 }, 'f')
|
||||
const result = await buildWindow(whole('one\ntwo\nthree\nfour'), { offset: 2, limit: 2, ...DEFAULT_CAPS }, 'f')
|
||||
expect(result.lines.map(l => l.number)).toEqual([2, 3])
|
||||
expect(result.totalLines).toBe(4)
|
||||
})
|
||||
@@ -62,7 +63,7 @@ describe('buildWindow', () => {
|
||||
})
|
||||
|
||||
it('rejects an offset past EOF', async () => {
|
||||
await expect(buildWindow(whole('one\ntwo'), { offset: 9, limit: 1 }, 'f')).rejects.toMatchObject({ code: 'FS_NOT_FOUND' })
|
||||
await expect(buildWindow(whole('one\ntwo'), { offset: 9, limit: 1, ...DEFAULT_CAPS }, 'f')).rejects.toMatchObject({ code: 'FS_NOT_FOUND' })
|
||||
})
|
||||
|
||||
it('flushes a final line with no trailing newline', async () => {
|
||||
@@ -76,9 +77,22 @@ describe('buildWindow', () => {
|
||||
expect(result.totalLines).toBe(2)
|
||||
})
|
||||
|
||||
describe('caps are per-request (the plugin config reaches the window)', () => {
|
||||
it('truncates lines at a custom maxLineLength and names it in the suffix', async () => {
|
||||
const result = await buildWindow(whole('abcdefghij'), { offset: 1, limit: 10, maxLineLength: 5, maxBytes: READ_MAX_BYTES }, 'f')
|
||||
expect(result.lines[0]?.text).toBe('abcde... (line truncated to 5 chars)')
|
||||
})
|
||||
|
||||
it('caps output at a custom maxBytes', async () => {
|
||||
const result = await buildWindow(whole('aaaa\nbbbb\ncccc'), { offset: 1, limit: 10, maxLineLength: 2000, maxBytes: 9 }, 'f')
|
||||
expect(result.lines.map(l => l.text)).toEqual(['aaaa', 'bbbb'])
|
||||
expect(result.truncatedByBytes).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('chunked input (streamed read shape)', () => {
|
||||
it('windows identically when text arrives in small chunks', async () => {
|
||||
const result = await buildWindow(chunked('one\ntwo\nthree', 2), { offset: 2, limit: 1 }, 'f')
|
||||
const result = await buildWindow(chunked('one\ntwo\nthree', 2), { offset: 2, limit: 1, ...DEFAULT_CAPS }, 'f')
|
||||
expect(result.lines).toEqual([{ number: 2, text: 'two' }])
|
||||
expect(result.totalLines).toBe(3)
|
||||
})
|
||||
|
||||
@@ -495,3 +495,71 @@ describe('result-time contextual diff (meta + presentResult)', () => {
|
||||
expect(view).toEqual({ card: 'diff', title: 'Write a.txt', diffs: [{ path: 'a.txt', oldText: null, newText: 'y' }] })
|
||||
})
|
||||
})
|
||||
|
||||
describe('read caps are plugin config', () => {
|
||||
async function setupWith(config: ToolFs.Config) {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(FakeFs)
|
||||
await ctx.plugin(FsPolicy)
|
||||
await ctx.plugin(ToolFs, config)
|
||||
return { ctx, fs: ctx.fs as FakeFs }
|
||||
}
|
||||
|
||||
it('a configured readLimit is both the default and the cap, and the schema names it', async () => {
|
||||
const { ctx, fs } = await setupWith({ readLimit: 2 })
|
||||
fs.files.set('key:a.txt', 'one\ntwo\nthree\nfour')
|
||||
const result = await call(ctx, 'read', { file_path: 'a.txt' })
|
||||
expect(text(result)).toContain('(Showing lines 1-2 of 4. Use offset=3 to continue.)')
|
||||
const overCap = await call(ctx, 'read', { file_path: 'a.txt', limit: 3 })
|
||||
expect(overCap.isError).toBe(true)
|
||||
expect(text(overCap)).toContain('less than or equal to 2')
|
||||
const readSchema = ctx.tools.schemas().find(s => s.name === 'read')
|
||||
expect(JSON.stringify(readSchema)).toContain('Defaults to 2.')
|
||||
})
|
||||
|
||||
it('a configured readMaxLineLength truncates lines at the configured length', async () => {
|
||||
const { ctx, fs } = await setupWith({ readMaxLineLength: 4 })
|
||||
fs.files.set('key:a.txt', 'abcdefgh')
|
||||
const result = await call(ctx, 'read', { file_path: 'a.txt' })
|
||||
expect(text(result)).toContain('1: abcd... (line truncated to 4 chars)')
|
||||
})
|
||||
|
||||
it('a configured readMaxBytes caps the window at the configured bytes', async () => {
|
||||
const { ctx, fs } = await setupWith({ readMaxBytes: 9 })
|
||||
fs.files.set('key:a.txt', 'aaaa\nbbbb\ncccc')
|
||||
const result = await call(ctx, 'read', { file_path: 'a.txt' })
|
||||
expect(text(result)).toContain('Output capped.')
|
||||
expect(text(result)).not.toContain('cccc')
|
||||
})
|
||||
|
||||
it('a configured readStreamMinSize routes smaller files to the streaming path', async () => {
|
||||
const { ctx, fs } = await setupWith({ readStreamMinSize: 5 })
|
||||
fs.files.set('key:a.txt', 'alpha\nbeta')
|
||||
const readSpy = vi.spyOn(fs, 'readText')
|
||||
const streamSpy = vi.spyOn(fs, 'streamText')
|
||||
const result = await call(ctx, 'read', { file_path: 'a.txt' })
|
||||
expect(result.isError).toBe(false)
|
||||
expect(streamSpy).toHaveBeenCalled()
|
||||
expect(readSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['readLimit', { readLimit: 0 }],
|
||||
['readLimit', { readLimit: 2.5 }],
|
||||
['readMaxLineLength', { readMaxLineLength: -1 }],
|
||||
['readMaxBytes', { readMaxBytes: Number.NaN }],
|
||||
['readStreamMinSize', { readStreamMinSize: 0 }],
|
||||
] as const)('rejects a non-positive or fractional %s at load', async (name, config) => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(FakeFs)
|
||||
await expect(ctx.plugin(ToolFs, config)).rejects.toThrow(new RegExp(`tool-fs: ${name} must be a positive integer`))
|
||||
})
|
||||
|
||||
it('has no default export (namespace plugin export shape)', () => {
|
||||
expect('default' in ToolFs).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -8,6 +8,7 @@
|
||||
"references": [
|
||||
{ "path": "../../../vendor/cosmokit" },
|
||||
{ "path": "../../../vendor/cordis" },
|
||||
{ "path": "../../../vendor/schemastery" },
|
||||
{ "path": "../../llm/llm" },
|
||||
{ "path": "../../core/tools" },
|
||||
{ "path": "../../core/system-prompt" },
|
||||
|
||||
@@ -58,6 +58,18 @@ export function appendHookInvoked(session: Session, invocation: HookInvocation):
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Truncate a hook's stderr for {@link HookResultRecord.stderrSummary}: trimmed,
|
||||
* `undefined` when empty, cut at `maxChars` with an ellipsis when over. The
|
||||
* bound is a parameter — like `runHook`'s `defaultTimeoutMs`, each bridge owns
|
||||
* the config default and passes it in.
|
||||
*/
|
||||
export function summarizeStderr(stderr: string, maxChars: number): string | undefined {
|
||||
const t = stderr.trim()
|
||||
if (t.length === 0) return undefined
|
||||
return t.length > maxChars ? t.slice(0, maxChars) + '…' : t
|
||||
}
|
||||
|
||||
/** Append a `hook/result` outcome event to `session` (pairs with a prior `hook/invoked`). */
|
||||
export function appendHookResult(session: Session, record: HookResultRecord): void {
|
||||
session.append('hook/result', {
|
||||
|
||||
@@ -34,5 +34,5 @@ export { runHook } from './runner.ts'
|
||||
export type { RunHookOptions, RunHookResult } from './runner.ts'
|
||||
export { mergeHookOutputs } from './merge.ts'
|
||||
export type { MergedDecision, MergedHookOutcome } from './merge.ts'
|
||||
export { appendHookInvoked, appendHookResult } from './events.ts'
|
||||
export { appendHookInvoked, appendHookResult, summarizeStderr } from './events.ts'
|
||||
export type { HookInvocation, HookResultRecord } from './events.ts'
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { Session, SessionId } from '@deepseek-ai/dsh-session'
|
||||
import { appendHookInvoked, appendHookResult } from '@deepseek-ai/dsh-hook-protocol'
|
||||
import { appendHookInvoked, appendHookResult, summarizeStderr } from '@deepseek-ai/dsh-hook-protocol'
|
||||
|
||||
describe('hook/* session events', () => {
|
||||
it('appendHookInvoked records a log-only hook/invoked (with matcher when present)', () => {
|
||||
@@ -59,3 +59,20 @@ describe('hook/* session events', () => {
|
||||
expect(result?.type === 'hook/result' && result.data.handlerId).toBe('pair-1')
|
||||
})
|
||||
})
|
||||
|
||||
describe('summarizeStderr', () => {
|
||||
it('returns undefined for empty/whitespace stderr', () => {
|
||||
expect(summarizeStderr('', 500)).toBeUndefined()
|
||||
expect(summarizeStderr(' \n\t ', 500)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('passes through a summary at or under the cap, trimmed', () => {
|
||||
expect(summarizeStderr(' blocked: bad tool ', 500)).toBe('blocked: bad tool')
|
||||
expect(summarizeStderr('abc', 3)).toBe('abc')
|
||||
})
|
||||
|
||||
it('truncates past the cap with an ellipsis', () => {
|
||||
expect(summarizeStderr('abcdef', 4)).toBe('abcd…')
|
||||
expect(summarizeStderr('x'.repeat(600), 500)).toBe('x'.repeat(500) + '…')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -13,6 +13,7 @@ const config: Config = {
|
||||
pluginRoot: '/path/to/plugin', // optional: replaces ${CLAUDE_PLUGIN_ROOT} in command strings
|
||||
projectDir: '/path/to/project', // optional: replaces ${CLAUDE_PROJECT_DIR} AND sets the hook env var; defaults to the session cwd when omitted
|
||||
defaultTimeoutMs: 600_000, // optional: per-hook timeout when a hook sets none (CC default)
|
||||
stderrSummaryMaxChars: 500, // optional: char cap on the hook/result event's persisted stderr summary
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
matchesMatcher,
|
||||
mergeHookOutputs,
|
||||
runHook,
|
||||
summarizeStderr,
|
||||
type HookOutput,
|
||||
type MatcherGroup,
|
||||
type MergedHookOutcome,
|
||||
@@ -73,6 +74,8 @@ export interface Config {
|
||||
projectDir?: string
|
||||
/** Default per-hook timeout in ms when a hook sets none (CC default: 600000). */
|
||||
defaultTimeoutMs?: number
|
||||
/** Character cap for the `hook/result` event's persisted stderr summary. */
|
||||
stderrSummaryMaxChars?: number
|
||||
}
|
||||
|
||||
export const Config: z<Config> = z.object({
|
||||
@@ -80,6 +83,7 @@ export const Config: z<Config> = z.object({
|
||||
pluginRoot: z.string(),
|
||||
projectDir: z.string(),
|
||||
defaultTimeoutMs: z.number().default(600_000),
|
||||
stderrSummaryMaxChars: z.number().default(500),
|
||||
})
|
||||
|
||||
/** A stable per-handler id so an invoked/result pair correlates in the log. */
|
||||
@@ -91,14 +95,18 @@ function nextHandlerId(point: string): string {
|
||||
/** The `{kind:'plugin'}` source stamped on every context this bridge injects. */
|
||||
const PLUGIN_SOURCE: MessageSource = { kind: 'plugin', plugin: 'hooks-claude' }
|
||||
|
||||
/** Truncate a stderr blob for the `hook/result` summary field. */
|
||||
function summarize(stderr: string): string | undefined {
|
||||
const t = stderr.trim()
|
||||
if (t.length === 0) return undefined
|
||||
return t.length > 500 ? t.slice(0, 500) + '…' : t
|
||||
/** The summary cap bounds a persisted event field — a positive integer or the slice misbehaves silently. */
|
||||
function assertPositiveInteger(name: string, value: number): void {
|
||||
if (!Number.isInteger(value) || value < 1) {
|
||||
throw new Error(`hooks-claude: ${name} must be a positive integer`)
|
||||
}
|
||||
}
|
||||
|
||||
export function apply(ctx: Context, config: Config): void {
|
||||
// Validate the cap BEFORE the config-file parse: a bad value must fail the
|
||||
// load loudly, not be skipped by the parse-failure early return.
|
||||
const stderrSummaryMaxChars = config.stderrSummaryMaxChars ?? 500
|
||||
assertPositiveInteger('stderrSummaryMaxChars', stderrSummaryMaxChars)
|
||||
// --- Parse the config ONCE at load. A read/parse failure is contained: the
|
||||
// bridge logs and registers nothing rather than crashing boot (a typo'd path
|
||||
// must not take the agent down). ---
|
||||
@@ -182,7 +190,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
ctx.logger.warn(`hooks-claude: ${point} hook emitted a systemMessage, which is not yet surfaced (ignored)`)
|
||||
}
|
||||
if (session && opts.turn !== undefined) {
|
||||
const stderrSummary = summarize(output.stderr)
|
||||
const stderrSummary = summarizeStderr(output.stderr, stderrSummaryMaxChars)
|
||||
appendHookResult(session, {
|
||||
turn: opts.turn, point, handlerId,
|
||||
decision: output.decision ?? (output.continue === false ? 'stop' : 'pass'),
|
||||
|
||||
@@ -27,7 +27,7 @@ function hooks(d: string, h: unknown): string {
|
||||
writeFileSync(join(d, 'hooks.json'), JSON.stringify({ hooks: h })); return join(d, 'hooks.json')
|
||||
}
|
||||
|
||||
type HarnessOpts = { pluginRoot?: string; projectDir?: string }
|
||||
type HarnessOpts = { pluginRoot?: string; projectDir?: string; stderrSummaryMaxChars?: number }
|
||||
async function harness(configPath: string, adapter: MockAdapter, opts: HarnessOpts = {}): Promise<Context> {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
@@ -139,6 +139,31 @@ describe('hooks-claude coverage — empty/no-op outcomes and no-agent paths', ()
|
||||
await waitForIdle(ctx, agent)
|
||||
const res = events(agent).find(e => e.type === 'hook/result')
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary?.endsWith('…')).toBe(true)
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary?.length).toBe(501) // default 500-char cap + ellipsis
|
||||
})
|
||||
|
||||
it('rejects a non-positive or fractional stderrSummaryMaxChars at load', async () => {
|
||||
const d = dir()
|
||||
const path = hooks(d, {})
|
||||
for (const bad of [0, -5, 1.5, Number.NaN]) {
|
||||
const adapter = new MockAdapter([])
|
||||
await expect(harness(path, adapter, { stderrSummaryMaxChars: bad }))
|
||||
.rejects.toThrow(/hooks-claude: stderrSummaryMaxChars must be a positive integer/)
|
||||
}
|
||||
})
|
||||
|
||||
it('the stderr summary cap is plugin config (stderrSummaryMaxChars)', async () => {
|
||||
const d = dir()
|
||||
const s = sh(d, 'long.sh', '#!/usr/bin/env bash\nprintf "x%.0s" {1..600} >&2\nexit 2\n')
|
||||
const path = hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: s }] }] })
|
||||
const adapter = new MockAdapter([toolCallResponse('c1', 'echo', {}), textResponse('done')])
|
||||
const ctx = await harness(path, adapter, { stderrSummaryMaxChars: 40 })
|
||||
ctx.tools.register(defineTool({ name: 'echo', description: 'e', parameters: {}, async execute() { return [{ type: 'text', text: 'ok' }] } }))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
agent.send([{ type: 'text', text: 'go' }])
|
||||
await waitForIdle(ctx, agent)
|
||||
const res = events(agent).find(e => e.type === 'hook/result')
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary).toBe('x'.repeat(40) + '…')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ const config: Config = {
|
||||
configPath: '/path/to/.codex/hooks.json', // required
|
||||
model: 'deepseek-v4', // optional: stamped on every payload (Codex includes `model`)
|
||||
defaultTimeoutMs: 600_000, // optional: per-hook timeout when a hook sets none
|
||||
stderrSummaryMaxChars: 500, // optional: char cap on the hook/result event's persisted stderr summary
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
matchesMatcher,
|
||||
mergeHookOutputs,
|
||||
runHook,
|
||||
summarizeStderr,
|
||||
type HookOutput,
|
||||
type MatcherGroup,
|
||||
type MergedHookOutcome,
|
||||
@@ -49,12 +50,15 @@ export interface Config {
|
||||
model?: string
|
||||
/** Default per-hook timeout in ms when a hook sets none (Codex default: 600000). */
|
||||
defaultTimeoutMs?: number
|
||||
/** Character cap for the `hook/result` event's persisted stderr summary. */
|
||||
stderrSummaryMaxChars?: number
|
||||
}
|
||||
|
||||
export const Config: z<Config> = z.object({
|
||||
configPath: z.string().required(),
|
||||
model: z.string().default(''),
|
||||
defaultTimeoutMs: z.number().default(600_000),
|
||||
stderrSummaryMaxChars: z.number().default(500),
|
||||
})
|
||||
|
||||
let handlerCounter = 0
|
||||
@@ -64,13 +68,18 @@ function nextHandlerId(point: string): string {
|
||||
|
||||
const PLUGIN_SOURCE: MessageSource = { kind: 'plugin', plugin: 'hooks-codex' }
|
||||
|
||||
function summarize(stderr: string): string | undefined {
|
||||
const t = stderr.trim()
|
||||
if (t.length === 0) return undefined
|
||||
return t.length > 500 ? t.slice(0, 500) + '…' : t
|
||||
/** The summary cap bounds a persisted event field — a positive integer or the slice misbehaves silently. */
|
||||
function assertPositiveInteger(name: string, value: number): void {
|
||||
if (!Number.isInteger(value) || value < 1) {
|
||||
throw new Error(`hooks-codex: ${name} must be a positive integer`)
|
||||
}
|
||||
}
|
||||
|
||||
export function apply(ctx: Context, config: Config): void {
|
||||
// Validate the cap BEFORE the config-file parse: a bad value must fail the
|
||||
// load loudly, not be skipped by the parse-failure early return.
|
||||
const stderrSummaryMaxChars = config.stderrSummaryMaxChars ?? 500
|
||||
assertPositiveInteger('stderrSummaryMaxChars', stderrSummaryMaxChars)
|
||||
let parsed: CodexHookConfig = {}
|
||||
try {
|
||||
const raw: unknown = JSON.parse(readFileSync(config.configPath, 'utf8'))
|
||||
@@ -140,7 +149,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
ctx.logger.warn(`hooks-codex: ${point} hook emitted a systemMessage, which is not yet surfaced (ignored)`)
|
||||
}
|
||||
if (session && opts.turn !== undefined) {
|
||||
const stderrSummary = summarize(output.stderr)
|
||||
const stderrSummary = summarizeStderr(output.stderr, stderrSummaryMaxChars)
|
||||
appendHookResult(session, {
|
||||
turn: opts.turn, point, handlerId,
|
||||
decision: output.decision ?? (output.continue === false ? 'stop' : 'pass'),
|
||||
|
||||
@@ -23,12 +23,12 @@ function hooks(d: string, h: unknown): string {
|
||||
writeFileSync(join(d, 'hooks.json'), JSON.stringify({ hooks: h })); return join(d, 'hooks.json')
|
||||
}
|
||||
|
||||
async function harness(configPath: string, adapter: MockAdapter): Promise<Context> {
|
||||
async function harness(configPath: string, adapter: MockAdapter, opts: { stderrSummaryMaxChars?: number } = {}): Promise<Context> {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService); await ctx.plugin(SessionStore); await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry); await ctx.plugin(AgentRegistry); await ctx.plugin(AgentLoop, { agents: [] })
|
||||
await ctx.plugin(LocalBashExecutor, { timeoutMs: 10_000 })
|
||||
await ctx.plugin(HooksCodex, { configPath, model: 'm' })
|
||||
await ctx.plugin(HooksCodex, { configPath, model: 'm', ...opts })
|
||||
ctx.llm.registerAdapter(['mock'], adapter)
|
||||
return ctx
|
||||
}
|
||||
@@ -204,6 +204,29 @@ describe('hooks-codex coverage — decision mapping paths', () => {
|
||||
agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent)
|
||||
const res = events(agent).find(e => e.type === 'hook/result')
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary?.endsWith('…')).toBe(true)
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary?.length).toBe(501) // default 500-char cap + ellipsis
|
||||
})
|
||||
|
||||
it('rejects a non-positive or fractional stderrSummaryMaxChars at load', async () => {
|
||||
const d = dir()
|
||||
hooks(d, {})
|
||||
for (const bad of [0, -5, 1.5, Number.NaN]) {
|
||||
const adapter = new MockAdapter([])
|
||||
await expect(harness(join(d, 'hooks.json'), adapter, { stderrSummaryMaxChars: bad }))
|
||||
.rejects.toThrow(/hooks-codex: stderrSummaryMaxChars must be a positive integer/)
|
||||
}
|
||||
})
|
||||
|
||||
it('the stderr summary cap is plugin config (stderrSummaryMaxChars)', async () => {
|
||||
const d = dir()
|
||||
hooks(d, { PreToolUse: [{ hooks: [{ type: 'command', command: sh(d, 'l.sh', '#!/usr/bin/env bash\nprintf "x%.0s" {1..600} >&2\nexit 2\n') }] }] })
|
||||
const adapter = new MockAdapter([toolCallResponse('c1', 'Bash', { command: 'x' }), textResponse('done')])
|
||||
const ctx = await harness(join(d, 'hooks.json'), adapter, { stderrSummaryMaxChars: 40 })
|
||||
ctx.tools.register(defineTool({ name: 'Bash', description: 'b', parameters: { command: { type: 'string' } }, async execute() { return [{ type: 'text', text: 'ok' }] } }))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent)
|
||||
const res = events(agent).find(e => e.type === 'hook/result')
|
||||
expect(res?.type === 'hook/result' && res.data.stderrSummary).toBe('x'.repeat(40) + '…')
|
||||
})
|
||||
|
||||
it('warns on a skipped async hook and a direct apply() defaults the timeout', async () => {
|
||||
|
||||
@@ -8,7 +8,7 @@ A SQLite durable session-persistence backend — a second `SessionPersistence` i
|
||||
|
||||
Each `SessionEvent` maps 1:1 onto a row in an `events` table `(session_id, seq, type, time, data, source_event_seqs, surface_op)` — `data` is the event payload as JSON text, so the row shape is the event verbatim (including `assistant/chunk`, keeping `seq` contiguous). The two `TEXT` columns `source_event_seqs` and `surface_op` are nullable; they store the event's optional surface-metadata fields (see [session surface](../../../docs/rfc/implemented/architecture/2026-06-18-session-surface.md)). Out-of-log metadata (`SessionHeader`) lives in a `sessions` row. A `sessions` row is written only by the first `append` — its existence is the lazy-materialization signal (`list` reports exactly the sessions that have a row), so no separate column is needed.
|
||||
|
||||
The repo targets Node ≥ 24 (the root `engines` field), which includes the stable `node:sqlite` module. The database opens with `foreign_keys = ON` (so `ON DELETE CASCADE` drops a session's events with its row) and `journal_mode = WAL`. The table-layout version is stored in `PRAGMA user_version` and checked on open: a fresh database is stamped with the current `SCHEMA_VERSION`; a database written by any other, incompatible build (a non-current `user_version`, older or newer) is rejected rather than opened against an unknown layout — there is no migration (unreleased software).
|
||||
The repo targets Node ≥ 24 (the root `engines` field), which includes the stable `node:sqlite` module. The database opens with `foreign_keys = ON` (so `ON DELETE CASCADE` drops a session's events with its row) and the configured `journal_mode` (default `wal`; pick a rollback-journal mode like `delete` on filesystems where WAL's shared-memory files do not work, e.g. network mounts). The table-layout version is stored in `PRAGMA user_version` and checked on open: a fresh database is stamped with the current `SCHEMA_VERSION`; a database written by any other, incompatible build (a non-current `user_version`, older or newer) is rejected rather than opened against an unknown layout — there is no migration (unreleased software).
|
||||
|
||||
## Contract semantics over rows
|
||||
|
||||
@@ -21,6 +21,7 @@ The repo targets Node ≥ 24 (the root `engines` field), which includes the stab
|
||||
```ts
|
||||
interface Config {
|
||||
path: string // SQLite database file path, or ':memory:' for an in-process DB
|
||||
journalMode?: 'wal' | 'delete' | 'truncate' | 'persist' // journal_mode pragma; default 'wal'
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
@@ -28,7 +28,7 @@ import {
|
||||
} from '@deepseek-ai/dsh-session-persistence'
|
||||
import type { Session, SessionEvent, SurfaceEventType, SessionId, SessionHeader } from '@deepseek-ai/dsh-session'
|
||||
import {
|
||||
openDatabase, rowToMeta, scanRows, type EventRow, type SessionRow,
|
||||
type JournalMode, openDatabase, rowToMeta, scanRows, type EventRow, type SessionRow,
|
||||
} from './schema.ts'
|
||||
|
||||
export { SCHEMA_VERSION } from './schema.ts'
|
||||
@@ -54,6 +54,13 @@ export interface Config {
|
||||
* dirs) on construction.
|
||||
*/
|
||||
path: string
|
||||
/**
|
||||
* SQLite `journal_mode` pragma. `wal` (the default) is the recorded
|
||||
* durability model; pick a rollback-journal mode (`delete`/`truncate`/
|
||||
* `persist`) on filesystems where WAL's shared-memory files do not work
|
||||
* (network mounts). See {@link JournalMode}.
|
||||
*/
|
||||
journalMode?: JournalMode
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -66,6 +73,7 @@ export class SessionPersistenceSqlite extends SessionPersistence implements Pers
|
||||
|
||||
static Config: z<Config> = z.object({
|
||||
path: z.string().required(),
|
||||
journalMode: z.union(['wal', 'delete', 'truncate', 'persist'] as const).default('wal'),
|
||||
})
|
||||
|
||||
/**
|
||||
@@ -83,18 +91,19 @@ export class SessionPersistenceSqlite extends SessionPersistence implements Pers
|
||||
super(ctx)
|
||||
// Open the database asynchronously (the parent directory may need creating);
|
||||
// every hook awaits `ready` first. Opening synchronously would force a sync
|
||||
// mkdir and block plugin apply.
|
||||
this.ready = this.openDb(config.path)
|
||||
// mkdir and block plugin apply. schemastery (static Config) has already
|
||||
// filled `journalMode`; the cast records that runtime fact.
|
||||
this.ready = this.openDb(config.path, (config as Required<Config>).journalMode)
|
||||
this.coordinator = new PersistenceCoordinator<number>(this.ctx, this)
|
||||
}
|
||||
|
||||
private async openDb(path: string): Promise<void> {
|
||||
private async openDb(path: string, journalMode: JournalMode): Promise<void> {
|
||||
if (path !== ':memory:') {
|
||||
const abs = resolve(path)
|
||||
await mkdir(dirname(abs), { recursive: true, mode: 0o700 })
|
||||
this.db = openDatabase(abs)
|
||||
this.db = openDatabase(abs, journalMode)
|
||||
} else {
|
||||
this.db = openDatabase(path)
|
||||
this.db = openDatabase(path, journalMode)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -45,10 +45,21 @@ export interface EventRow {
|
||||
surface_op: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Journal modes the backend will run under. `wal` is the default and the
|
||||
* durability model the persistence ADR records; the rollback-journal modes
|
||||
* (`delete`/`truncate`/`persist`) exist for filesystems where WAL's
|
||||
* shared-memory files do not work (network mounts). `memory`/`off` are
|
||||
* excluded: dropping journal durability silently contradicts what this
|
||||
* backend promises.
|
||||
*/
|
||||
export type JournalMode = 'wal' | 'delete' | 'truncate' | 'persist'
|
||||
|
||||
/**
|
||||
* Open the database at `path` and apply the schema + pragmas. `foreign_keys`
|
||||
* makes `ON DELETE CASCADE` drop a session's events with its row; `journal_mode
|
||||
* = WAL` matches the durability model the ADR records (the row shape maps 1:1
|
||||
* makes `ON DELETE CASCADE` drop a session's events with its row; the
|
||||
* `journal_mode` pragma is set from the plugin's `journalMode` config (`wal`
|
||||
* default — the durability model the ADR records; the row shape maps 1:1
|
||||
* onto `SessionEvent`; opencode runs this exact shape on SQLite/WAL).
|
||||
*
|
||||
* The table-layout version is persisted in SQLite's `PRAGMA user_version` and
|
||||
@@ -66,10 +77,12 @@ export interface EventRow {
|
||||
* makes the version check reject both sibling v3 databases instead of opening
|
||||
* one against columns it does not have.
|
||||
*/
|
||||
export function openDatabase(path: string): DatabaseSync {
|
||||
export function openDatabase(path: string, journalMode: JournalMode): DatabaseSync {
|
||||
const db = new DatabaseSync(path)
|
||||
db.exec('PRAGMA foreign_keys = ON')
|
||||
db.exec('PRAGMA journal_mode = WAL')
|
||||
// journalMode is a closed in-code union (validated by the plugin Config), not
|
||||
// user-controlled SQL — safe to interpolate (PRAGMA takes no bound params).
|
||||
db.exec(`PRAGMA journal_mode = ${journalMode.toUpperCase()}`)
|
||||
// `PRAGMA user_version` always returns exactly one row { user_version }.
|
||||
const { user_version: onDisk } = db.prepare('PRAGMA user_version').get() as { user_version: number }
|
||||
if (onDisk !== 0 && onDisk !== SCHEMA_VERSION) {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { Context } from 'cordis'
|
||||
import { existsSync } from 'node:fs'
|
||||
import { mkdtemp, rm } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
@@ -53,7 +54,7 @@ runCoordinatorContract('sqlite', async (): Promise<CoordinatorFixture> => {
|
||||
// A row past the committed region whose `data` does not parse: scanRows
|
||||
// bounds the preserved prefix at it and returns its seq as tornFrom, which
|
||||
// the backend surfaces to the coordinator as the tornMarker to delete from.
|
||||
const db = openDatabase(path)
|
||||
const db = openDatabase(path, 'wal')
|
||||
const next = (db.prepare('SELECT COALESCE(MAX(seq), -1) + 1 AS n FROM events WHERE session_id = ?')
|
||||
.get(id) as { n: number }).n
|
||||
db.prepare('INSERT INTO events (session_id, seq, type, time, data) VALUES (?, ?, ?, ?, ?)')
|
||||
@@ -192,7 +193,7 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
await b1.ctx.sessionPersistence.append(m.id, oneTurnLog()) // seqs 0..5
|
||||
await b1.dispose()
|
||||
// Hand-write an interrupted turn (turn/start seq 6, no turn/end).
|
||||
const db = openDatabase(path)
|
||||
const db = openDatabase(path, 'wal')
|
||||
db.prepare('INSERT INTO events (session_id, seq, type, time, data) VALUES (?, 6, ?, 7, ?)')
|
||||
.run(m.id, 'turn/start', JSON.stringify({ turn: 2, trigger: { kind: 'message', source: { kind: 'user' } } }))
|
||||
db.close()
|
||||
@@ -204,7 +205,7 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
expect(loaded.events.at(-1)!.type).toBe('turn/end')
|
||||
// load() is mutating: the synthetic turn/end MUST be on disk so the stored log
|
||||
// is balanced and the cursor is truthful (contract: load closes, not defers).
|
||||
const probe = openDatabase(path)
|
||||
const probe = openDatabase(path, 'wal')
|
||||
const stored = probe.prepare('SELECT seq, type FROM events WHERE session_id = ? ORDER BY seq').all(m.id) as { seq: number; type: string }[]
|
||||
probe.close()
|
||||
expect(stored.map(r => r.seq)).toEqual([0, 1, 2, 3, 4, 5, 6, 7])
|
||||
@@ -237,21 +238,21 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
|
||||
it('rejects opening a database whose schema version is not the current build (newer OR older)', async () => {
|
||||
const path = await freshDbPath()
|
||||
openDatabase(path).close() // stamp user_version = SCHEMA_VERSION
|
||||
openDatabase(path, 'wal').close() // stamp user_version = SCHEMA_VERSION
|
||||
// Bump user_version past what this build supports.
|
||||
const dbNewer = openDatabase(path)
|
||||
const dbNewer = openDatabase(path, 'wal')
|
||||
dbNewer.exec(`PRAGMA user_version = ${SCHEMA_VERSION + 1}`)
|
||||
dbNewer.close()
|
||||
expect(() => openDatabase(path)).toThrow(/incompatible with this build/)
|
||||
expect(() => openDatabase(path, 'wal')).toThrow(/incompatible with this build/)
|
||||
|
||||
// A stale OLDER version (e.g. a pre-summary-drop v1 DB) is also rejected —
|
||||
// we do not migrate (unreleased software, no backward-compat).
|
||||
const olderPath = await freshDbPath()
|
||||
openDatabase(olderPath).close()
|
||||
const dbOlder = openDatabase(olderPath)
|
||||
openDatabase(olderPath, 'wal').close()
|
||||
const dbOlder = openDatabase(olderPath, 'wal')
|
||||
dbOlder.exec('PRAGMA user_version = 1')
|
||||
dbOlder.close()
|
||||
expect(() => openDatabase(olderPath)).toThrow(/incompatible with this build/)
|
||||
expect(() => openDatabase(olderPath, 'wal')).toThrow(/incompatible with this build/)
|
||||
})
|
||||
|
||||
it('rejects a sibling v3 database (the merge-collided version) rather than opening it against missing columns', async () => {
|
||||
@@ -261,11 +262,11 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
// of this build's columns, so it MUST be rejected, not opened. Stamp a v3
|
||||
// database and confirm the version check refuses it.
|
||||
const path = await freshDbPath()
|
||||
openDatabase(path).close() // creates + stamps user_version = SCHEMA_VERSION (4)
|
||||
const db = openDatabase(path)
|
||||
openDatabase(path, 'wal').close() // creates + stamps user_version = SCHEMA_VERSION (4)
|
||||
const db = openDatabase(path, 'wal')
|
||||
db.exec('PRAGMA user_version = 3')
|
||||
db.close()
|
||||
expect(() => openDatabase(path)).toThrow(/schema version 3, incompatible with this build/)
|
||||
expect(() => openDatabase(path, 'wal')).toThrow(/schema version 3, incompatible with this build/)
|
||||
})
|
||||
|
||||
it('a corrupt-JSON row in the uncommitted tail is discarded on load, not unloadable', async () => {
|
||||
@@ -281,7 +282,7 @@ describe('SessionPersistenceSqlite: durability and crash semantics', () => {
|
||||
// unloadable; a torn tail must be discarded. scanRows finds the last
|
||||
// turn/end on the seq+type columns (never parsing tail `data`), so the
|
||||
// unparsable row after it bounds the preserved prefix and is deleted by load.
|
||||
const db = openDatabase(path)
|
||||
const db = openDatabase(path, 'wal')
|
||||
db.prepare('INSERT INTO events (session_id, seq, type, time, data) VALUES (?, 6, ?, 7, ?)')
|
||||
.run(m.id, 'turn/start', '{not valid json')
|
||||
db.close()
|
||||
@@ -370,6 +371,29 @@ describe('SessionPersistenceSqlite: edge cases', () => {
|
||||
await b2.dispose()
|
||||
})
|
||||
|
||||
it('journalMode config reaches the database (default wal, rollback modes selectable)', async () => {
|
||||
// :memory: databases always report journal_mode=memory, so probe file DBs.
|
||||
const walPath = await freshDbPath()
|
||||
const bWal = await backend(walPath)
|
||||
await bWal.ctx.sessionPersistence.create(meta('jm-wal'))
|
||||
expect((openDatabase(walPath, 'wal').prepare('PRAGMA journal_mode').get() as { journal_mode: string }).journal_mode).toBe('wal')
|
||||
await bWal.dispose()
|
||||
|
||||
const deletePath = await freshDbPath()
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const fiber = await ctx.plugin(SessionPersistenceSqlite, { path: deletePath, journalMode: 'delete' })
|
||||
await ctx.sessionPersistence.create(meta('jm-delete'))
|
||||
// Probe through a second connection: journal_mode=delete is a per-database
|
||||
// property only insofar as no WAL files exist — assert the world, not the
|
||||
// backend's self-report (no -wal sidecar after writes in delete mode).
|
||||
const db = openDatabase(deletePath, 'delete')
|
||||
expect((db.prepare('PRAGMA journal_mode').get() as { journal_mode: string }).journal_mode).toBe('delete')
|
||||
db.close()
|
||||
expect(existsSync(`${deletePath}-wal`)).toBe(false)
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
it('HMR: a DIFFERENT session colliding with a materialized on-disk id is rejected', async () => {
|
||||
const path = await freshDbPath()
|
||||
// Instance 1 materializes a session and disposes.
|
||||
|
||||
@@ -25,6 +25,8 @@ Unlike the in-process backends, the child does NOT share this cordis context —
|
||||
| `cwd` | string | parent cwd | Working directory for the child process and its ACP session. |
|
||||
| `permission` | `'allow' \| 'reject'` | `reject` | How to auto-answer the child's `session/request_permission` prompts. `reject` declines every prompt (answer `cancelled`); `allow` approves via the first allow-shaped option. The first cut surfaces no prompt to a human. |
|
||||
| `env` | Record<string,string> | `{}` | Extra env vars for the child (e.g. its own `DEEPSEEK_API_KEY`). Forwarded on top of a credential-scrubbed copy of the parent env, so an explicit key reaches the child while ambient secrets do not leak implicitly. |
|
||||
| `disposeEofGraceMs` | number | `6000` | Dispose ladder tier 1: how long the child gets to quiesce on its own after stdin EOF (flush persistence, tear down its nested subprocesses) before SIGTERM. |
|
||||
| `disposeGraceMs` | number | `3000` | Dispose ladder tier 2: grace between SIGTERM and the SIGKILL escalation. |
|
||||
|
||||
```yaml
|
||||
- id: subagent-acp
|
||||
|
||||
@@ -21,7 +21,7 @@
|
||||
import type { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import type { SubagentCapabilities, SubagentProvider, SubagentStartRequest } from '@deepseek-ai/dsh-subagent'
|
||||
import { type AcpRunSpec, type PermissionPolicy, startAcpRun } from './run.ts'
|
||||
import { type AcpRunSpec, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, type PermissionPolicy, startAcpRun } from './run.ts'
|
||||
|
||||
export const name = 'subagent-acp'
|
||||
export const inject = ['subagents']
|
||||
@@ -52,6 +52,14 @@ export interface Config {
|
||||
* ambient secrets do not leak implicitly.
|
||||
*/
|
||||
env: Record<string, string>
|
||||
/**
|
||||
* Grace period (ms) for the child's EOF-driven quiesce on dispose — its
|
||||
* window to flush persistence and tear down its own nested subprocesses
|
||||
* before the parent escalates to a signal.
|
||||
*/
|
||||
disposeEofGraceMs?: number
|
||||
/** Grace period (ms) between `SIGTERM` and the `SIGKILL` escalation on dispose. */
|
||||
disposeGraceMs?: number
|
||||
}
|
||||
|
||||
export const Config: z<Config> = z.object({
|
||||
@@ -61,8 +69,20 @@ export const Config: z<Config> = z.object({
|
||||
cwd: z.string(),
|
||||
permission: z.union(['allow', 'reject'] as const).default('reject'),
|
||||
env: z.dict(z.string()).default({}),
|
||||
disposeEofGraceMs: z.number().default(DEFAULT_DISPOSE_EOF_GRACE_MS),
|
||||
disposeGraceMs: z.number().default(DEFAULT_DISPOSE_GRACE_MS),
|
||||
})
|
||||
|
||||
/** A dispose grace must be a positive finite number (it bounds the teardown wait). */
|
||||
function assertPositiveFinite(name: string, value: number): void {
|
||||
if (!Number.isFinite(value) || value <= 0) {
|
||||
throw new Error(`subagent-acp: ${name} must be a positive finite number`)
|
||||
}
|
||||
}
|
||||
|
||||
/** The shape after schemastery applied the defaults (cwd has none). */
|
||||
type ResolvedConfig = Required<Omit<Config, 'cwd'>> & Pick<Config, 'cwd'>
|
||||
|
||||
/**
|
||||
* The ACP provider. Advertises NO start-time capabilities: an out-of-process
|
||||
* child cannot honor `outputSchema`/`maxDepth`/`toolFilter` (the service rejects
|
||||
@@ -71,7 +91,7 @@ export const Config: z<Config> = z.object({
|
||||
class AcpProvider implements SubagentProvider {
|
||||
readonly capabilities: SubagentCapabilities = { outputSchema: false, depthLimit: false, toolFilter: false }
|
||||
|
||||
constructor(readonly name: string, private readonly ctx: Context, private readonly config: Config) {}
|
||||
constructor(readonly name: string, private readonly ctx: Context, private readonly config: ResolvedConfig) {}
|
||||
|
||||
start(request: SubagentStartRequest) {
|
||||
const spec: AcpRunSpec = {
|
||||
@@ -80,6 +100,8 @@ class AcpProvider implements SubagentProvider {
|
||||
cwd: this.config.cwd ?? process.cwd(),
|
||||
permission: this.config.permission,
|
||||
env: this.config.env,
|
||||
disposeEofGraceMs: this.config.disposeEofGraceMs,
|
||||
disposeGraceMs: this.config.disposeGraceMs,
|
||||
onError: (error, stopReason) => {
|
||||
// The seam forbids `result` rejecting, so a child-level failure is
|
||||
// flattened to a stop reason — preserve it here rather than losing it.
|
||||
@@ -91,5 +113,9 @@ class AcpProvider implements SubagentProvider {
|
||||
}
|
||||
|
||||
export function apply(ctx: Context, config: Config): void {
|
||||
ctx.subagents.registerProvider(new AcpProvider(config.providerName, ctx, config))
|
||||
// schemastery (Config) has already filled every defaulted field.
|
||||
const resolved = config as ResolvedConfig
|
||||
assertPositiveFinite('disposeEofGraceMs', resolved.disposeEofGraceMs)
|
||||
assertPositiveFinite('disposeGraceMs', resolved.disposeGraceMs)
|
||||
ctx.subagents.registerProvider(new AcpProvider(resolved.providerName, ctx, resolved))
|
||||
}
|
||||
|
||||
@@ -73,16 +73,16 @@ export interface AcpRunSpec {
|
||||
/**
|
||||
* Grace period (ms) for the child's EOF-driven quiesce in
|
||||
* {@link SubagentRun.dispose} — the window to flush persistence and tear down
|
||||
* its OWN nested subprocesses before the parent escalates to a signal. Defaults
|
||||
* to {@link DEFAULT_DISPOSE_EOF_GRACE_MS}; a test injects a small value.
|
||||
* its OWN nested subprocesses before the parent escalates to a signal. The
|
||||
* plugin fills this from its `disposeEofGraceMs` config.
|
||||
*/
|
||||
disposeEofGraceMs?: number
|
||||
disposeEofGraceMs: number
|
||||
/**
|
||||
* Grace period (ms) between `SIGTERM` and the `SIGKILL` escalation in
|
||||
* {@link SubagentRun.dispose}. Defaults to {@link DEFAULT_DISPOSE_GRACE_MS};
|
||||
* a test injects a small value to exercise the escalation without a long wait.
|
||||
* {@link SubagentRun.dispose}. The plugin fills this from its
|
||||
* `disposeGraceMs` config.
|
||||
*/
|
||||
disposeGraceMs?: number
|
||||
disposeGraceMs: number
|
||||
/**
|
||||
* Sink for a child-level failure that the run flattened into a stop reason
|
||||
* (the seam contract forbids `result` rejecting). The driver calls this with
|
||||
@@ -94,19 +94,20 @@ export interface AcpRunSpec {
|
||||
}
|
||||
|
||||
/**
|
||||
* Default grace for the child's EOF-driven quiesce on dispose — the window for it
|
||||
* to flush persistence and tear down its OWN nested subprocesses (which may run
|
||||
* their own `SIGTERM`→`SIGKILL` escalation) before the parent escalates to a
|
||||
* signal. Deliberately LARGER than {@link DEFAULT_DISPOSE_GRACE_MS}: a cooperative
|
||||
* child whose teardown is itself waiting on a signal-trapping grandchild (e.g. a
|
||||
* bash subprocess in its own ~3s SIGTERM→SIGKILL grace) plus a final flush needs
|
||||
* MORE than a single signal-grace of headroom, or the parent's SIGTERM cuts it off
|
||||
* exactly as it reaches its own SIGKILL+flush. The child is an arbitrary ACP agent,
|
||||
* so this is a standalone generous default, NOT derived from any child's internals.
|
||||
* Default grace for the child's EOF-driven quiesce on dispose (the
|
||||
* `disposeEofGraceMs` config) — the window for it to flush persistence and tear
|
||||
* down its OWN nested subprocesses (which may run their own `SIGTERM`→`SIGKILL`
|
||||
* escalation) before the parent escalates to a signal. Deliberately LARGER than
|
||||
* {@link DEFAULT_DISPOSE_GRACE_MS}: a cooperative child whose teardown is itself
|
||||
* waiting on a signal-trapping grandchild (e.g. a bash subprocess in its own ~3s
|
||||
* SIGTERM→SIGKILL grace) plus a final flush needs MORE than a single
|
||||
* signal-grace of headroom, or the parent's SIGTERM cuts it off exactly as it
|
||||
* reaches its own SIGKILL+flush. The child is an arbitrary ACP agent, so this is
|
||||
* a standalone generous default, NOT derived from any child's internals.
|
||||
*/
|
||||
export const DEFAULT_DISPOSE_EOF_GRACE_MS = 6_000
|
||||
|
||||
/** Default grace between SIGTERM and SIGKILL on dispose (mirrors the bash executor). */
|
||||
/** Default grace between SIGTERM and SIGKILL on dispose (the `disposeGraceMs` config; mirrors the bash executor). */
|
||||
export const DEFAULT_DISPOSE_GRACE_MS = 3_000
|
||||
|
||||
/**
|
||||
@@ -372,8 +373,8 @@ export function startAcpRun(request: SubagentStartRequest, spec: AcpRunSpec): Su
|
||||
// Reach quiescence, not merely request it (dispose must AWAIT the child
|
||||
// actually stopping). If the child is already gone, nothing to do.
|
||||
if (child.exitCode !== null || child.signalCode !== null) return
|
||||
const eofGraceMs = spec.disposeEofGraceMs ?? DEFAULT_DISPOSE_EOF_GRACE_MS
|
||||
const graceMs = spec.disposeGraceMs ?? DEFAULT_DISPOSE_GRACE_MS
|
||||
const eofGraceMs = spec.disposeEofGraceMs
|
||||
const graceMs = spec.disposeGraceMs
|
||||
// 1. Graceful: end the ACP request stream (stdin EOF) and let the child
|
||||
// quiesce ON ITS OWN. Our acp-agent has NO SIGTERM handler in a normal
|
||||
// session — it tears down via the server bridge's connection-close path
|
||||
|
||||
@@ -8,7 +8,7 @@ import { fileURLToPath } from 'node:url'
|
||||
import SubagentService from '@deepseek-ai/dsh-subagent'
|
||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||
import * as acp from '../src/index.ts'
|
||||
import { acpStopReason, acpContentText, buildChildEnv, SENSITIVE_ENV_PATTERN, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts'
|
||||
import { acpStopReason, acpContentText, buildChildEnv, DEFAULT_DISPOSE_EOF_GRACE_MS, DEFAULT_DISPOSE_GRACE_MS, SENSITIVE_ENV_PATTERN, startAcpRun, toAcpPrompt, type AcpRunSpec } from '../src/run.ts'
|
||||
|
||||
/**
|
||||
* Keyless integration tests for the ACP subagent backend. Each spawns a REAL
|
||||
@@ -171,7 +171,7 @@ describe('dsh-subagent-acp', () => {
|
||||
const run = startAcpRun(
|
||||
{ prompt: [{ type: 'text', text: 'p' }], parent: fakeParent, signal: controller.signal },
|
||||
// `touch <sentinel>` — runs only if the process is actually spawned.
|
||||
{ command: 'touch', args: [sentinel], cwd: tmp, permission: 'reject', env: {} },
|
||||
{ command: 'touch', args: [sentinel], cwd: tmp, permission: 'reject', env: {}, disposeEofGraceMs: DEFAULT_DISPOSE_EOF_GRACE_MS, disposeGraceMs: DEFAULT_DISPOSE_GRACE_MS },
|
||||
)
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('aborted')
|
||||
@@ -390,7 +390,7 @@ describe('dsh-subagent-acp', () => {
|
||||
// absent-sink branch).
|
||||
const run = startAcpRun(
|
||||
{ prompt: [{ type: 'text', text: 'p' }], parent: fakeParent },
|
||||
{ command: '/nonexistent/acp-agent-binary', args: [], cwd: process.cwd(), permission: 'reject', env: {} },
|
||||
{ command: '/nonexistent/acp-agent-binary', args: [], cwd: process.cwd(), permission: 'reject', env: {}, disposeEofGraceMs: DEFAULT_DISPOSE_EOF_GRACE_MS, disposeGraceMs: DEFAULT_DISPOSE_GRACE_MS },
|
||||
)
|
||||
const result = await run.result
|
||||
// The seam contract: a child-level failure resolves error, never rejects.
|
||||
@@ -398,6 +398,47 @@ describe('dsh-subagent-acp', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('plugin-config dispose graces reach the run (SIGKILL escalation through the provider)', async () => {
|
||||
// Same trap scenario as the direct startAcpRun escalation test, but the
|
||||
// graces arrive via the PLUGIN CONFIG through the registered provider — so a
|
||||
// regression that stops threading config into AcpRunSpec (falling back to
|
||||
// the 6s/3s defaults) blows past the 4000ms bound and fails loud.
|
||||
const tmp = mkdtempSync(join(tmpdir(), 'acp-cfg-trap-'))
|
||||
const ready = join(tmp, 'trap-armed')
|
||||
try {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SubagentService)
|
||||
await ctx.plugin(acp, {
|
||||
providerName: 'acp',
|
||||
command: process.execPath,
|
||||
args: ['--import', tsxLoader, mockServer],
|
||||
permission: 'reject',
|
||||
env: { MOCK_TRAP_SIGTERM: '1', MOCK_TEXT: 'x', MOCK_READY_FILE: ready, TSX_TSCONFIG_PATH: repoTsconfig },
|
||||
disposeEofGraceMs: 150,
|
||||
disposeGraceMs: 150,
|
||||
})
|
||||
const run = ctx.subagents.start('acp', { prompt: [{ type: 'text', text: 'p' }], parent: fakeParent })
|
||||
await waitForFile(ready)
|
||||
await expect(Promise.race([
|
||||
run.dispose(),
|
||||
new Promise((_r, reject) => { setTimeout(() => { reject(new Error('dispose did not return — config graces not threaded to the run')) }, 4000) }),
|
||||
])).resolves.toBeUndefined()
|
||||
await ctx.fiber.dispose()
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('rejects a non-positive dispose grace at load', async () => {
|
||||
for (const bad of [{ disposeEofGraceMs: 0 }, { disposeGraceMs: -1 }, { disposeEofGraceMs: Number.NaN }]) {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SubagentService)
|
||||
await expect(ctx.plugin(acp, { providerName: 'acp', command: 'true', args: [], permission: 'reject', env: {}, ...bad }))
|
||||
.rejects.toThrow(/subagent-acp: dispose(?:Eof)?GraceMs must be a positive finite number/)
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
|
||||
it('resolves error via the provider (real load path) when the command does not exist', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SubagentService)
|
||||
@@ -428,6 +469,8 @@ describe('dsh-subagent-acp', () => {
|
||||
cwd: process.cwd(),
|
||||
permission: 'reject',
|
||||
env: {},
|
||||
disposeEofGraceMs: DEFAULT_DISPOSE_EOF_GRACE_MS,
|
||||
disposeGraceMs: DEFAULT_DISPOSE_GRACE_MS,
|
||||
onError: (error, stopReason) => { errors.push({ message: error.message, stopReason }) },
|
||||
},
|
||||
)
|
||||
|
||||
@@ -8,7 +8,7 @@ Each tool is registered independently; a product that wants only one disables th
|
||||
|
||||
| Tool | Args | Behavior |
|
||||
|---|---|---|
|
||||
| `web_search` | `query` (string) | Discovery. Returns an optional answer plus source URLs. `max_results` is **not** model-facing — the tool sets the bound (`WEB_SEARCH_MAX_RESULTS = 8`) and passes it to the seam. |
|
||||
| `web_search` | `query` (string) | Discovery. Returns an optional answer plus source URLs. `max_results` is **not** model-facing — the tool sets the bound (the `searchMaxResults` config, default 8) and passes it to the seam. |
|
||||
| `web_fetch` | `url` (string), `timeout_ms` (number, optional) | Retrieves a specific URL. HTML bodies are rendered to markdown-ish text; text bodies pass through. A non-2xx status is reported, not an error. |
|
||||
|
||||
## Config
|
||||
@@ -17,6 +17,7 @@ Each tool is registered independently; a product that wants only one disables th
|
||||
|---|---|---|
|
||||
| `search` | `true` | Register `web_search`. |
|
||||
| `fetch` | `true` | Register `web_fetch`. |
|
||||
| `searchMaxResults` | `8` | Upper bound on sources returned by one `web_search` call (the seam truncates a longer provider list and flags it). |
|
||||
|
||||
```yaml
|
||||
- id: tool-web
|
||||
|
||||
@@ -20,7 +20,7 @@
|
||||
import type { Context } from 'cordis'
|
||||
import z from 'schemastery'
|
||||
import type {} from '@deepseek-ai/dsh-web'
|
||||
import { applyWebSearchTool } from './search.ts'
|
||||
import { applyWebSearchTool, WEB_SEARCH_MAX_RESULTS } from './search.ts'
|
||||
import { applyWebFetchTool } from './fetch.ts'
|
||||
|
||||
export { WEB_SEARCH_MAX_RESULTS, applyWebSearchTool, formatSearchOutput, parseSearchArgs, presentSearchCall } from './search.ts'
|
||||
@@ -38,13 +38,26 @@ export interface Config {
|
||||
search?: boolean
|
||||
/** Register `web_fetch`. Defaults to true. */
|
||||
fetch?: boolean
|
||||
/** Upper bound on sources returned by one `web_search` call. */
|
||||
searchMaxResults?: number
|
||||
}
|
||||
|
||||
export const Config: z<Config> = z.object({
|
||||
search: z.boolean().default(true),
|
||||
fetch: z.boolean().default(true),
|
||||
searchMaxResults: z.number().default(WEB_SEARCH_MAX_RESULTS),
|
||||
})
|
||||
|
||||
/** The shape after schemastery applies its defaults to every field. */
|
||||
type ResolvedConfig = Required<Config>
|
||||
|
||||
/** The result cap must be a positive integer (it bounds a provider's source list). */
|
||||
function assertPositiveInteger(name: string, value: number): void {
|
||||
if (!Number.isInteger(value) || value < 1) {
|
||||
throw new Error(`tool-web: ${name} must be a positive integer`)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Register the enabled web tools. `search`/`fetch` default to true; a product
|
||||
* that wants only one disables the other in config. The tools' disposers are
|
||||
@@ -52,6 +65,9 @@ export const Config: z<Config> = z.object({
|
||||
* teardown is needed.
|
||||
*/
|
||||
export function apply(ctx: Context, config: Config): void {
|
||||
if (config.search !== false) applyWebSearchTool(ctx)
|
||||
if (config.fetch !== false) applyWebFetchTool(ctx)
|
||||
// schemastery (Config) has already filled every defaulted field.
|
||||
const resolved = config as ResolvedConfig
|
||||
assertPositiveInteger('searchMaxResults', resolved.searchMaxResults)
|
||||
if (resolved.search) applyWebSearchTool(ctx, resolved.searchMaxResults)
|
||||
if (resolved.fetch) applyWebFetchTool(ctx)
|
||||
}
|
||||
|
||||
@@ -13,10 +13,10 @@ import type { WebSearchResult } from '@deepseek-ai/dsh-web'
|
||||
import type {} from '@deepseek-ai/dsh-system-prompt'
|
||||
|
||||
/**
|
||||
* Default upper bound on returned sources. Owned by the consumer (not the
|
||||
* provider or model), mirroring `dsh-tool-fs`'s `READ_LIMIT`/`GREP_LIMIT`. The
|
||||
* model just asks a question; the product controls how much context returns.
|
||||
* The default `8` aligns with OpenCode's Exa default.
|
||||
* Default upper bound on returned sources (the `searchMaxResults` config).
|
||||
* Owned by the consumer (not the provider or model), mirroring `dsh-tool-fs`'s
|
||||
* `READ_LIMIT`. The model just asks a question; the product controls how much
|
||||
* context returns. The default `8` aligns with OpenCode's Exa default.
|
||||
*/
|
||||
export const WEB_SEARCH_MAX_RESULTS = 8
|
||||
|
||||
@@ -67,8 +67,8 @@ export function presentSearchCall(args: { query: string }): GenericCallView {
|
||||
return { card: 'generic', title: args.query, kind: 'search', rawInput: args.query }
|
||||
}
|
||||
|
||||
/** Register the `web_search` tool and its system-prompt guidance. */
|
||||
export function applyWebSearchTool(ctx: Context): void {
|
||||
/** Register the `web_search` tool and its system-prompt guidance. `maxResults` is the deployment's source cap. */
|
||||
export function applyWebSearchTool(ctx: Context, maxResults: number): void {
|
||||
ctx.systemPrompt.section({
|
||||
name: 'tool:web_search',
|
||||
order: 110,
|
||||
@@ -84,7 +84,7 @@ export function applyWebSearchTool(ctx: Context): void {
|
||||
async execute(args, exec): Promise<ContentBlock[]> {
|
||||
const input = parseSearchArgs(args)
|
||||
const result = await ctx.web.search(
|
||||
{ query: input.query, maxResults: WEB_SEARCH_MAX_RESULTS },
|
||||
{ query: input.query, maxResults },
|
||||
exec.signal ? { signal: exec.signal } : undefined,
|
||||
)
|
||||
return [{ type: 'text', text: formatSearchOutput(result) }]
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
presentFetchCall,
|
||||
renderBody,
|
||||
htmlToMarkdown,
|
||||
WEB_SEARCH_MAX_RESULTS,
|
||||
} from '@deepseek-ai/dsh-tool-web'
|
||||
|
||||
const available: WebProviderStatus = { available: true }
|
||||
@@ -279,3 +280,48 @@ describe('tool-web execution through the real registry', () => {
|
||||
await fiber.dispose()
|
||||
})
|
||||
})
|
||||
|
||||
describe('searchMaxResults is plugin config', () => {
|
||||
it('forwards the default cap to the seam when unconfigured', async () => {
|
||||
const seen: { maxResults?: number | undefined } = {}
|
||||
const provider: WebSearchProvider = {
|
||||
id: 'stub-search',
|
||||
status: () => available,
|
||||
search: (request) => { seen.maxResults = request.maxResults; return Promise.resolve({ providerId: 'stub-search', query: 'q', sources: [], truncated: false }) },
|
||||
}
|
||||
const { fiber, call } = await mountTools({ webConfig: { searchProvider: 'stub-search' }, search: provider })
|
||||
await call('web_search', { query: 'q' })
|
||||
expect(seen.maxResults).toBe(WEB_SEARCH_MAX_RESULTS)
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
it('forwards a configured cap to the seam, which enforces it', async () => {
|
||||
const sources = Array.from({ length: 5 }, (_, i) => ({ url: `https://s${i}.test` }))
|
||||
const provider: WebSearchProvider = {
|
||||
id: 'stub-search',
|
||||
status: () => available,
|
||||
search: request => Promise.resolve({ providerId: 'stub-search', query: request.query, sources, truncated: false }),
|
||||
}
|
||||
const { fiber, call } = await mountTools({ config: { searchMaxResults: 2 }, webConfig: { searchProvider: 'stub-search' }, search: provider })
|
||||
const out = await call('web_search', { query: 'q' })
|
||||
expect(out.isError).toBe(false)
|
||||
const body = out.content.map(b => b.text).join('')
|
||||
expect(body).toContain('https://s1.test')
|
||||
expect(body).not.toContain('https://s2.test')
|
||||
expect(body).toContain('Showing the first 2 sources.')
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
it.each([
|
||||
['zero', 0],
|
||||
['negative', -3],
|
||||
['fractional', 1.5],
|
||||
])('rejects a %s searchMaxResults at load', async (_label, value) => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SystemPrompt)
|
||||
await ctx.plugin(ToolRegistry)
|
||||
await ctx.plugin(WebService, {})
|
||||
await expect(ctx.plugin(ToolWeb, { searchMaxResults: value }))
|
||||
.rejects.toThrow(/tool-web: searchMaxResults must be a positive integer/)
|
||||
})
|
||||
})
|
||||
|
||||
3
pnpm-lock.yaml
generated
3
pnpm-lock.yaml
generated
@@ -323,6 +323,9 @@ importers:
|
||||
diff:
|
||||
specifier: ^9.0.0
|
||||
version: 9.0.0
|
||||
schemastery:
|
||||
specifier: ^3.18.0
|
||||
version: 3.18.0
|
||||
devDependencies:
|
||||
'@deepseek-ai/dsh-agent':
|
||||
specifier: workspace:^
|
||||
|
||||
Reference in New Issue
Block a user