fix: address codex review round 1
- spill-policy validates maxInlineBytes as a non-negative integer at LOAD, so a bad config fails the deployment instead of letting a negative value reach TextRetainer and turn every oversized-result call into an isError. - Document the spill seam vocabulary in docs/core-data-structures/spill.md (SaveTextSpill/SpillOwner/SpillSource/SpillRef/SpillPath, verbatim + type-equiv gated) and index it from core.md, matching the other capability seams.
This commit is contained in:
@@ -24,6 +24,7 @@ Everything else is documented on a **sub-page**, not here. The rule that draws t
|
||||
| [compaction.md](compaction.md) | the compaction seam: the `compact/*` session events, `CompactionResult`, the `CompactService` interface |
|
||||
| [subagent.md](subagent.md) | the subagent seam: the named-provider registry, `SubagentStartRequest`/`Result`/`Run`, the start-time-vs-runtime capability split |
|
||||
| [web.md](web.md) | the web access seam: `WebSearchRequest`/`Result`, `WebFetchRequest`/`Result`, `WebFetchBody`, provider/capability status, `WebError` |
|
||||
| [spill.md](spill.md) | the spill storage seam: `SaveTextSpill`, `SpillOwner`/`SpillSource`, `SpillRef`, the branded `SpillPath` |
|
||||
|
||||
> Type definitions on this page are pasted **verbatim** from source and drift-checked by `pnpm run verify-type-equiv` (see [development.md](../development.md#documenting-types-verbatim-ts-type-equiv)). Inline JSDoc is omitted for readability; follow the source link for the full contracts.
|
||||
|
||||
|
||||
55
docs/core-data-structures/spill.md
Normal file
55
docs/core-data-structures/spill.md
Normal file
@@ -0,0 +1,55 @@
|
||||
# Spill Storage
|
||||
|
||||
The spill storage seam — a [capability seam](../rfc/implemented/architecture/2026-07-08-tool-output-spill-files.md) that persists a tool's oversized text to a session-scoped path the model can later `read`, split across packages: interface ([dsh-spill](../../packages/spill/spill), `ctx.spillFiles`), implementation ([dsh-spill-local](../../packages/spill/spill-local), private session-scoped files on the host filesystem), and consumer ([dsh-spill-policy](../../packages/spill/spill-policy), the `tools/post-execute` policy). Spill is **one optional capability**, not part of the agent-loop spine — so its vocabulary lives here, not in [core.md](core.md). Preview mechanics stay in [dsh-retention](../../packages/util/retention); this seam only saves the final text the policy hands it.
|
||||
|
||||
Source: [`packages/spill/spill/src/types.ts`](../../packages/spill/spill/src/types.ts)
|
||||
|
||||
## The save request
|
||||
|
||||
`saveText` is the whole seam: persist `content` verbatim, return a readable path plus the exact byte count. The request carries WHO the artifact belongs to (`owner`), WHERE it came from (`source`, descriptive provenance for the filename and future cleanup — not access control), and a `suggestedName` the backend sanitizes to one safe path segment before use (it is a hint, never a path).
|
||||
|
||||
```ts type-equiv
|
||||
interface SaveTextSpill {
|
||||
owner: SpillOwner
|
||||
source: SpillSource
|
||||
suggestedName: string
|
||||
content: string
|
||||
}
|
||||
```
|
||||
|
||||
```ts type-equiv
|
||||
interface SpillOwner {
|
||||
sessionId: SessionId
|
||||
}
|
||||
```
|
||||
|
||||
`SpillOwner` scopes storage to a `SessionId` — spill is inherently session-scoped (its directory layout and future cleanup unit are per session), so the seam imports `dsh-session`'s `SessionId` directly rather than minting a decoupled token like the bash executor's cross-session `OwnerToken` ([bash.md](bash.md)).
|
||||
|
||||
```ts type-equiv
|
||||
interface SpillSource {
|
||||
toolName: string
|
||||
callId: CallId
|
||||
label: string
|
||||
}
|
||||
```
|
||||
|
||||
## The result
|
||||
|
||||
```ts type-equiv
|
||||
interface SpillRef {
|
||||
path: SpillPath
|
||||
bytes: number
|
||||
}
|
||||
```
|
||||
|
||||
`SpillPath` is a [branded](core.md#branded-ids) local filesystem path returned by the backend and intended for the model's `read` tool. The brand records that the path came from the spill seam (a runtime artifact, not a workspace file the model authored); it is still rendered to the model as an ordinary path string in v1. A future remote or virtual backend may replace it with a `spill://…` URI plus a read-only filesystem bridge, so consumers treat it as opaque.
|
||||
|
||||
```ts type-equiv
|
||||
type SpillPath = Branded<'SpillPath'>
|
||||
```
|
||||
|
||||
## The service
|
||||
|
||||
`SpillFiles` (`ctx.spillFiles`, defined in [`packages/spill/spill/src/index.ts`](../../packages/spill/spill/src/index.ts)) is a one-method abstract service: `saveText(input) → Promise<SpillRef>`. It persists the FULL `content`, chooses a private (not world-readable) location and a collision-free name derived from — never equal to — `suggestedName`, and REJECTS on a real storage failure (permissions, ENOSPC, backend unavailable). The seam owns storage only: no retention policy, no tool-result replacement, no file inspection.
|
||||
|
||||
The local backend ([dsh-spill-local](../../packages/spill/spill-local)) writes under `<root>/session-<hash>/<random>-<safeName>` — a configured or lazily-created private (0700) root, a `sha256(sessionId)` session subdir, and an exclusive owner-only (`open(path, 'wx', 0o600)`) write so a planted symlink cannot redirect it. The policy consumer ([dsh-spill-policy](../../packages/spill/spill-policy)) replaces an over-`maxInlineBytes` plain-text final result with a retention-library head/tail preview plus the spill path, best-effort: a save failure keeps the original inline result rather than turning a successful call into an `isError`.
|
||||
@@ -8,7 +8,7 @@ This plugin registers **no service** and owns no storage or preview mechanics: p
|
||||
|
||||
| Key | Default | Meaning |
|
||||
|---|---|---|
|
||||
| `maxInlineBytes` | *(omitted)* | Model-facing context cap for a plain-text result, in UTF-8 bytes. **Omitted disables the policy entirely** (the plugin registers nothing). When set, a larger result is spilled and replaced with a preview derived from the same budget (head/tail split). |
|
||||
| `maxInlineBytes` | *(omitted)* | Model-facing context cap for a plain-text result, in UTF-8 bytes (a non-negative integer; validated at load). **Omitted disables the policy entirely** (the plugin registers nothing). When set, a larger result is spilled and replaced with a preview derived from the same budget (head/tail split). |
|
||||
|
||||
## Behavior
|
||||
|
||||
|
||||
@@ -101,6 +101,12 @@ export function apply(ctx: Context, config: Config): void {
|
||||
const maxInlineBytes = config.maxInlineBytes
|
||||
// Omitted ⇒ no automatic spill policy: register nothing at all.
|
||||
if (maxInlineBytes === undefined) return
|
||||
// Validate at LOAD, not per call: a negative/fractional cap would reach
|
||||
// TextRetainer's assertBudget and throw, turning every oversized-result call
|
||||
// into an isError. A bad config must fail the deployment, not the tool.
|
||||
if (!Number.isInteger(maxInlineBytes) || maxInlineBytes < 0) {
|
||||
throw new Error(`spill-policy: maxInlineBytes must be a non-negative integer (got ${maxInlineBytes})`)
|
||||
}
|
||||
|
||||
ctx.on('tools/post-execute', async (exec, result, next): Promise<PostToolDecision> => {
|
||||
// Delegate first so a downstream listener (e.g. a hook) settles the result;
|
||||
|
||||
@@ -82,6 +82,16 @@ describe('disabled mode', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('config validation', () => {
|
||||
it('rejects a negative maxInlineBytes at load', async () => {
|
||||
await expect(setup({ maxInlineBytes: -1 })).rejects.toThrow(/non-negative integer/)
|
||||
})
|
||||
|
||||
it('rejects a fractional maxInlineBytes at load', async () => {
|
||||
await expect(setup({ maxInlineBytes: 1.5 })).rejects.toThrow(/non-negative integer/)
|
||||
})
|
||||
})
|
||||
|
||||
describe('oversized plain-text replacement', () => {
|
||||
it('spills the full text and replaces the result with a preview + path', async () => {
|
||||
const { ctx, spill } = await setup({ maxInlineBytes: 20 })
|
||||
|
||||
@@ -77,6 +77,12 @@
|
||||
{ "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchRequest", "source": "packages/web/web/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchResult", "source": "packages/web/web/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/web.md", "symbol": "WebFetchBody", "source": "packages/web/web/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/web.md", "symbol": "WebProviderStatus", "source": "packages/web/web/src/types.ts" }
|
||||
{ "doc": "docs/core-data-structures/web.md", "symbol": "WebProviderStatus", "source": "packages/web/web/src/types.ts" },
|
||||
|
||||
{ "doc": "docs/core-data-structures/spill.md", "symbol": "SaveTextSpill", "source": "packages/spill/spill/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/spill.md", "symbol": "SpillOwner", "source": "packages/spill/spill/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/spill.md", "symbol": "SpillSource", "source": "packages/spill/spill/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/spill.md", "symbol": "SpillRef", "source": "packages/spill/spill/src/types.ts" },
|
||||
{ "doc": "docs/core-data-structures/spill.md", "symbol": "SpillPath", "source": "packages/spill/spill/src/types.ts" }
|
||||
]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user