diff --git a/docs/dispatch_model.md b/docs/dispatch_model.md index d8ab60385..80baa0e54 100644 --- a/docs/dispatch_model.md +++ b/docs/dispatch_model.md @@ -100,7 +100,7 @@ Admission is checked *before* a delivery rather than after, so the admitted acti Both limits count each delivery once. An `activity_dispatched` size is the whole `get_activity` response, so the techniques and resources it bundled eagerly are already inside it and their own observability events are not added again; what counts on top is only what the worker went back for lazily. Counting the bundled entries twice inflated one activity of the main workflow by 48% and a run of three by 70%, which made a nominal 280,000-character budget bind at 164,540. -`get_activity` reports where a context stands in `_meta.batch` (`activities`, `max_activities`, `delivered_chars`, `budget_chars`, `may_continue`), so the ordinary end of a batch is the worker stopping. Asking past the bound is refused with the payload undelivered and a `batch_refused` history event naming the limit — recorded once per scope, activity and limit, so the tally counts how often a limit bound rather than how often a worker retried. That tally is what the starting settings are revised from. +`get_activity` reports where a context stands (`activities`, `max_activities`, `delivered_chars`, `budget_chars`, `may_continue`) in two places: a `batch:` block leading the response text, and `_meta.batch`. The text is the surface a worker is certain to read — the same reason `artifact_prefix` rides there — and the standing is what makes the ordinary end of a batch the worker stopping. The counts describe where the context stands once this delivery lands, so the activity being delivered is already counted among the ones it has taken. Asking past the bound is refused with the payload undelivered and a `batch_refused` history event naming the limit — recorded once per scope, activity and limit, so the tally counts how often a limit bound rather than how often a worker retried. That tally is what the starting settings are revised from. `may_continue` is answered as of that delivery, and the worker then fetches techniques and resources lazily while it runs the activity, drawing down the same budget. So a batch reported as having room can still be refused at the next boundary — the delivered and budget counts on the same response are what a reader compares to see how close it was. The refusal is an expected outcome rather than an error, and the orchestrator handles it by releasing the identity and dispatching a replacement — which must carry a **new** `agent_id`, since the bound is keyed on the identity and a fresh context under a used one would receive markers for content it does not hold. diff --git a/docs/resource_resolution_model.md b/docs/resource_resolution_model.md index 4733216da..897bea672 100644 --- a/docs/resource_resolution_model.md +++ b/docs/resource_resolution_model.md @@ -140,7 +140,7 @@ The server resolves the reference: An optional `#section` anchor (a GitHub-style heading slug) narrows the result to that section and its body — used to fetch just the template a technique references without the whole file. The content is loaded from `workflows/{workflow}/resources/{slug}.md` and returned alongside the resource `id` and `version`. -Under `context_mode: "persistent"`, a byte-identical refetch of the same exact `resource_id` (including any `#section`) returns a short `{ delivery: "unchanged", content_hash }` marker instead of the body — the same reference-delivery contract as `get_technique` (see [Reference Delivery](#11-reference-delivery)). Bare and sectioned ids are independent ledger keys. Pass `full: true` to force the full body when the calling context no longer holds the earlier delivery. Fresh/default sessions always receive the full resource body. Each call still appends a `resource_fetched` history event (observability only), including when the answer is an unchanged marker. +A byte-identical refetch of the same exact `resource_id` (including any `#section`) to a context that already holds it returns a short `{ delivery: "unchanged", content_hash }` marker instead of the body — the same reference-delivery contract as `get_technique`, on the grounds set out in [Reference Delivery](#11-reference-delivery). Bare and sectioned ids are independent ledger keys. Pass `full: true` to force the full body when the calling context no longer holds the earlier delivery. Each call still appends a `resource_fetched` history event (observability only), including when the answer is an unchanged marker. ### Benefits @@ -150,9 +150,17 @@ Under `context_mode: "persistent"`, a byte-identical refetch of the same exact ` ## 11. Reference Delivery -By default the server sends every payload in full, every time. A freshly spawned worker starts with an empty context, so that repetition is what gives it the content at all. +The server sends a payload in full to a context that does not hold it. A freshly spawned worker starts with an empty context, so that first delivery is what gives it the content at all. -An agent that already holds a payload can ask for **reference delivery** instead. The server replaces that payload with a short marker — `{ delivery: "unchanged", content_hash }` — and the agent reuses what it has. +Where a context already holds the bytes, the server sends a short marker instead — `{ delivery: "unchanged", content_hash }` — and the agent reuses what it has. Three things establish that it holds them, and each governs a different call: + +| Ground | What collapses | +|---|---| +| **Reference delivery** — `context_mode: "persistent"`, or `bundle: "reference"` on one call | anything this scope's ledger records | +| **The same response** — an earlier `step_techniques` entry of this `get_activity` carries the block in full | the shared contract and rules blocks a response repeats, on a full delivery too | +| **A named context asking again** — this `agent_id`'s ledger records the payload | a repeat `get_technique` or `get_resource`, whatever mode the call declares | + +The second and third need no opt-in, because in both cases the bytes demonstrably reached the asking context: in one they are above the marker in the same payload, in the other the ledger says this identity received them. The third requires a caller that names its context — with `agent_id` omitted the scope falls back to the session's own identity, which sibling workers share, so nothing collapses there. ### What counts as "already holds" @@ -189,7 +197,7 @@ The orchestrator mints an `agent_id` per dispatch and reuses it verbatim for as - **`get_resource`** — a byte-identical refetch of the same `resource_id` returns `delivery: unchanged` and a `content_hash` instead of the body. The key is the caller's exact `resource_id`, anchor included, so `pr-description` and `pr-description#templates` occupy independent slots. - **`get_workflow`** — under `context_mode: "persistent"` the orchestrator ops bundle (everything above the `---` separator) is keyed under `workflow_bundle:`. On a resume where the agent already holds it, the whole bundle collapses to a single marker, while the workflow summary below the separator stays full. -`get_technique` and `get_resource` collapse under either `bundle: "reference"` or a session-wide `context_mode: "persistent"`. Fresh and default sessions always receive full bodies. +`get_technique` and `get_resource` collapse a repeat to any caller that named its context, and under `bundle: "reference"` or a session-wide `context_mode: "persistent"` besides. A caller that named no context receives full bodies however often it asks, and `full: true` overrides every ground. ### Blocks inside a technique @@ -197,6 +205,8 @@ Collapsing can go finer than a whole technique. Techniques sharing a workflow co So when a technique is new to the context but one of its shared blocks already arrived with a sibling technique, that block becomes a marker in place while the technique-specific core arrives in full. This happens both on the `get_technique` full-delivery path and inside each eagerly inlined `get_activity` `step_techniques` entry. +Inside one `get_activity` response this holds on a full delivery too, and it is where most of the repetition is: every composed technique of a delivery carries the contract and rules blocks it inherits from its container, so a response bundling five of them would carry five copies. The first copy ships in full and the rest are markers, whose bytes are above them in the same payload — readable by a context holding no prior delivery at all. Measured over the batch benchmark's three-activity run, a fresh worker per activity receives 213,476 characters where five copies apiece cost 225,617. Where any block collapses this way the response carries a `bundle_note` pointing the reader at the earliest entry showing it. + Hashing the content is what keeps this from going stale: a block annotated with binding-seam provenance hashes differently, so it correctly arrives in full. ### Forcing full delivery diff --git a/src/tools/resource-tools.ts b/src/tools/resource-tools.ts index d0252e7e5..6d5fab6ff 100644 --- a/src/tools/resource-tools.ts +++ b/src/tools/resource-tools.ts @@ -79,6 +79,39 @@ function withSessionStoreErrors, R>( }; } +/** + * Whether a fetch is a context asking again for content it already holds (#404 W9). + * + * A ledger entry says a scope received a payload in full, so a second ask for the same bytes is + * answered with a marker whatever delivery mode the call declares — the content is in the asking + * context already, and re-sending it buys nothing. + * + * Three conditions make that sound. + * + * `full: true` always overrides, which is the escape hatch for a context that summarized the content + * away. + * + * The caller must NAME its context, because with `agent_id` omitted the scope falls back to the + * session's own identity and a marker could reach a context that never received the bytes. + * + * And the name must not BE the session's own identity, which is the one name known to be shared by + * construction: `dispatch_child` defaults it to `"worker"`, so two sibling workers can each pass it + * without either having received what the other did. That scope keeps its earlier behaviour — it + * collapses only where the caller asked for reference delivery, which is a claim about one context + * rather than an inference from a name. A solo walk, which legitimately owns that identity, declares + * `context_mode: "persistent"` and so collapses on that ground instead. + * + * A distinct name shared by two contexts anyway defeats this, as it defeats reference delivery today; + * minting one identity per dispatch is what the corpus requires, and `full: true` recovers. + */ +function isRepeatToNamedContext( + agentId: string | undefined, + full: boolean | undefined, + sessionAgentId: string, +): boolean { + return full !== true && agentId !== undefined && agentId !== sessionAgentId; +} + export function registerResourceTools(server: McpServer, config: ServerConfig): void { const traceOpts = config.traceStore ? { traceStore: config.traceStore } : undefined; // Process-level engineering root (may be install multi-root). Per-session @@ -602,7 +635,7 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): server.tool( 'get_technique', 'Load one fully composed technique (step-bound when `step_id` is set; otherwise the activity\'s or workflow\'s first). ' + - 'Under `context_mode: "persistent"` or `bundle: "reference"`, a byte-identical refetch to the SAME `agent_id` scope may return an unchanged-reference; pass `full: true` when earlier content was summarized away. ' + + 'A byte-identical refetch to a named `agent_id` scope returns an unchanged-reference — that context already holds the bytes — as does any refetch under `context_mode: "persistent"` or `bundle: "reference"`; pass `full: true` when earlier content was summarized away. ' + 'A fresh worker context must not ask for reference delivery — it holds no prior delivery to reference.', { ...sessionIndexParam, @@ -762,7 +795,8 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): && (bundle ?? (state.contextMode === 'persistent' ? 'reference' : 'full')) === 'reference'; const ledgerKey = `technique:${techniqueId}`; const hash = contentHash(text); - if (referenceMode && deliveredHash(state, ledgerKey, scope) === hash) { + if ((referenceMode || isRepeatToNamedContext(agent_id, full, state.agentId)) + && deliveredHash(state, ledgerKey, scope) === hash) { const next = advanceSession(state, (draft) => { draft.currentTechnique = techniqueId as string; recordFirstArrival(draft); @@ -825,7 +859,7 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): server.tool( 'get_resource', 'Load a resource by id (optional `#section`). Bare slug = session workflow; `workflow/slug` = cross-workflow. ' + - 'Under `context_mode: "persistent"` or `bundle: "reference"`, a byte-identical refetch to the SAME `agent_id` scope may return an unchanged-reference; pass `full: true` when content was summarized away. ' + + 'A byte-identical refetch to a named `agent_id` scope returns an unchanged-reference — that context already holds the bytes — as does any refetch under `context_mode: "persistent"` or `bundle: "reference"`; pass `full: true` when content was summarized away. ' + 'A freshly spawned worker must not ask for reference delivery — it holds no prior delivery to reference.', { ...sessionIndexParam, @@ -899,7 +933,8 @@ export function registerResourceTools(server: McpServer, config: ServerConfig): const hash = contentHash(fullText); const referenceMode = full !== true && (bundle ?? (state.contextMode === 'persistent' ? 'reference' : 'full')) === 'reference'; - if (referenceMode && deliveredHash(state, ledgerKey, scope) === hash) { + if ((referenceMode || isRepeatToNamedContext(agent_id, full, state.agentId)) + && deliveredHash(state, ledgerKey, scope) === hash) { const next = advanceSession(state, (draft) => { recordFirstArrival(draft); recordFetch(draft, 'unchanged', fullText.length); diff --git a/src/tools/workflow-tools.ts b/src/tools/workflow-tools.ts index 9a0a2e58b..3b8d0886c 100644 --- a/src/tools/workflow-tools.ts +++ b/src/tools/workflow-tools.ts @@ -18,7 +18,7 @@ import { buildProducerIndex, provenanceContextFor, decorateTechniqueProvenance } import { withAuditLog, logInfo, logWarn } from '../logging.js'; import { applyVariableWrites } from '../utils/variable-seed.js'; import { stringifyForResponse } from '../utils/serialization.js'; -import { contentHash, deliveredHash, dedupTechniqueBlocks, deliveryScope, recordDeliveries, unchangedMarker } from '../utils/delivery.js'; +import { contentHash, countCollapsedBlocks, deliveredHash, dedupTechniqueBlocks, deliveryScope, recordDeliveries, unchangedMarker } from '../utils/delivery.js'; import { dispatchKind, hasDispatch, priorDeliveryScope, recordDispatch, recordRedelivery } from '../utils/dispatch.js'; import { batchBound, batchRefusal, batchRefusalMessage, batchState, recordBatchRefusal } from '../utils/batch.js'; import { extractResourceIds, qualifyResourceId } from '../utils/resource-ref.js'; @@ -914,6 +914,8 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): const resourceRefIds: string[] = []; const linkedResourceIds = new Set(); const bundlingWarnings: string[] = []; + /** Shared blocks a full delivery dropped because a sibling of this same response carried them. */ + let intraResponseCollapses = 0; const bundleConfig = (activity as Activity | undefined)?.bundleTechniques; // maxChars: 0 is the explicit opt-out sentinel; any other declared value is a per-technique // size cap. Absent config means no per-technique cap (only the cumulative budget applies). @@ -1000,11 +1002,14 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): newDeliveries[ledgerKey] = hash; // The arrival marker leads the block; the composed technique fields follow at the same // level, so a bundled entry reads exactly like a get_technique fetch with a step header. - // Under reference mode, collapse any shared contract/rules block already delivered (by a - // sibling bundled step or an earlier fetch) to a marker while the core stays full. - const projected = referenceMode - ? dedupTechniqueBlocks(projectTechnique(technique), state, newDeliveries, scope) - : projectTechnique(technique); + // Shared contract and rules blocks collapse to a marker while the core stays full. Under + // reference mode a block this context received on an earlier call collapses too; under + // full delivery only a block a sibling of THIS response already carried does, so the + // bytes a marker stands for are always above it in the same payload. + const projected = dedupTechniqueBlocks( + projectTechnique(technique), state, newDeliveries, scope, { readLedger: referenceMode }, + ); + if (!referenceMode) intraResponseCollapses += countCollapsedBlocks(projected); bundledStepTechniques[step.id!] = { marker: stepMarker, ...projected }; } // Collect linked resource ids from the full composed technique text even when this @@ -1146,7 +1151,15 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): bundle_note: 'Entries marked { delivery: "unchanged", content_hash } are already in your context — reuse them. A marker may also replace a single inherited_inputs/inherited_outputs/rules block inside an otherwise-full step_techniques entry (that shared block came from a sibling technique). Re-fetch a technique with get_technique { step_id, full: true }, or get_activity { bundle: "full" } to re-deliver the whole bundle.', ...bundleData, } - : bundleData; + : intraResponseCollapses > 0 + ? { + // A full delivery repeats the contract and rules blocks its techniques share. The + // second and later copies arrive as markers whose bytes are above them in this same + // response, so a freshly spawned worker holding no prior delivery still reads them. + bundle_note: 'A shared inherited_inputs/inherited_outputs/rules block inside a step_techniques entry may arrive as { delivery: "unchanged", content_hash } because an EARLIER entry of THIS response carries it in full. Read that block from the earliest step_techniques entry that shows it — the entries are in step order and the content is identical. Every other field of every entry arrives in full.', + ...bundleData, + } + : bundleData; const opsSection = stringifyForResponse(opsData) + '\n\n---\n\n'; // artifactPrefix is server-computed from the activity filename and is NOT in @@ -1208,7 +1221,7 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): // Assembled before the save so the dispatch event can record what this dispatch actually // cost — `chars` on an activity's fresh and resume events is the before/after measurement. - const responseText = `${opsSection}${header}\n\n${activityRulesBlock}${enforcementBlock}${activityBodyWithArtifacts}`; + const responseBody = `${opsSection}${header}\n\n${activityRulesBlock}${enforcementBlock}${activityBodyWithArtifacts}`; // Persist against a FRESH load, not the snapshot captured before composition: the session // store is last-writer-wins over the whole file, and composition awaits dozens of FS reads — @@ -1226,6 +1239,29 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): const priorScope = hasDispatch(reloaded.state, scope, activity_id) ? undefined : priorDeliveryScope(reloaded.state, scope, activity_id); + + // Where this context stands against its bound once this delivery lands, read off the history it + // will be saved against plus the delivery itself. A worker reads `may_continue` to decide + // whether to ask for the next activity, so the ordinary end of a batch is the worker stopping + // and the refusal above is the backstop; the counts make that answer auditable from the + // response. It rides in the response TEXT as well as `_meta`, for the same reason + // `artifact_prefix` does: the text is the surface a worker is certain to read. + const stand = batchState(reloaded.state, scope, bound, { chars: responseBody.length, activityId: activity_id }); + const batch = { + activities: stand.activities.length, + max_activities: bound.maxActivities, + delivered_chars: stand.chars, + budget_chars: bound.budgetChars, + may_continue: stand.mayContinue, + }; + // The numbers only. What to do with `may_continue` is owned by the worker role technique every + // activity bundle carries (`workflow-engine::activity-worker`, batch-ends-where-the-server-says), + // and restating it here would put the same instruction on a third surface and cost every + // delivery the characters. `delivered_chars` counts this response's payload; the block's own + // length is charged to the recorded dispatch, which is the figure a measurement reads. + const batchBlock = `${stringifyForResponse({ batch })}\n\n`; + const responseText = `${batchBlock}${responseBody}`; + const next = advanceSession(reloaded.state, (draft) => { recordDeliveries(draft, scope, newDeliveries); recordDispatch(draft, { scope, kind: dispatch, activityId: activity_id, chars: responseText.length }); @@ -1260,19 +1296,6 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): }); await saveSessionForTool(reloaded, next); - // Where this context stands against its bound, this delivery included. A worker reads - // `may_continue` to decide whether to ask for the next activity, so the ordinary end of a batch - // is the worker stopping and the refusal above is the backstop. The counts make that answer - // auditable from the response. - const stand = batchState(next, scope, bound); - const batch = { - activities: stand.activities.length, - max_activities: bound.maxActivities, - delivered_chars: stand.chars, - budget_chars: bound.budgetChars, - may_continue: stand.mayContinue, - }; - // What this delivery cost to build and to send, on one line. `resolved_techniques` is the // distinct bound ops the producer scan read for the whole request and `provenance_passes` the // steps decorated from that one scan, so the two together say whether resolve work is being @@ -1286,6 +1309,7 @@ export function registerWorkflowTools(server: McpServer, config: ServerConfig): bundled_steps: bundledSteps.length, bundled_steps_collapsed: bundledSteps.filter((b) => b.delivery === 'unchanged').length, bundled_resources: bundledResourceDeliveries.length, + shared_blocks_collapsed_in_response: intraResponseCollapses, spent_chars: spentChars, eager_budget_chars: Math.floor(eagerBudgetChars), response_chars: responseText.length, diff --git a/src/utils/batch.ts b/src/utils/batch.ts index 9898559ea..5c4d5c950 100644 --- a/src/utils/batch.ts +++ b/src/utils/batch.ts @@ -145,10 +145,23 @@ export function batchBound( * * `mayContinue` is answered before the lazy fetches of the activity just taken draw down the same * budget, so `true` can still become a refusal at the next boundary. + * + * `pending` describes a delivery on its way out that the history does not record yet, so a response + * can carry the standing it produces without being assembled twice: `chars` is what that response + * costs, and `activityId` is the activity it carries, counted as taken when the scope does not + * already hold it. */ -export function batchState(state: SessionFile, scope: string, bound: BatchBound): BatchState { +export function batchState( + state: SessionFile, + scope: string, + bound: BatchBound, + pending: { chars?: number; activityId?: string } = {}, +): BatchState { const activities = batchActivities(state, scope); - const chars = deliveredChars(state, scope); + if (pending.activityId !== undefined && !activities.includes(pending.activityId)) { + activities.push(pending.activityId); + } + const chars = deliveredChars(state, scope) + (pending.chars ?? 0); // The session's own agent owns the whole walk, and a scope with no activity yet has no batch to be // past the end of — the reading the refusal takes too. const exempt = scope === state.agentId || activities.length === 0; diff --git a/src/utils/delivery.ts b/src/utils/delivery.ts index 5873529ad..04ee2beff 100644 --- a/src/utils/delivery.ts +++ b/src/utils/delivery.ts @@ -7,12 +7,25 @@ import { stringifyForResponse } from './serialization.js'; * * The session file carries a delivery ledger (`deliveredContent`): per * delivery scope (see `deliveryScope`), a map of content key → hash of the - * payload last delivered in full. When reference delivery is active (session - * `contextMode: 'persistent'` or a per-call opt-in), a payload whose hash - * matches the ledger is replaced by a short `{ delivery: 'unchanged', - * content_hash }` marker — the receiving context already holds the bytes. - * This is the one canonical unchanged-marker shape, emitted identically by - * the `get_activity` bundle path, `get_technique`, and `get_resource`. + * payload last delivered in full. A payload whose hash matches is replaced by + * a short `{ delivery: 'unchanged', content_hash }` marker — the receiving + * context already holds the bytes. This is the one canonical unchanged-marker + * shape, emitted identically by the `get_activity` bundle path, + * `get_technique`, and `get_resource`. + * + * Three things make a marker readable, and each governs where one is emitted: + * - Reference delivery (session `contextMode: 'persistent'` or a per-call + * opt-in) — the caller states that this context holds its earlier payloads. + * - The same response — a sibling entry above the marker carries the block in + * full, so a full delivery to a fresh context can still collapse the + * repeats (`dedupTechniqueBlocks` with `readLedger: false`). + * - A named context asking again — the ledger records that THIS `agent_id` + * received the bytes, so a repeat `get_technique` / `get_resource` is + * answered with a marker whatever mode the call declares. A caller that + * omits `agent_id` scopes to the session's own identity, which siblings + * share, so no repeat collapses there. + * + * `full: true` overrides all three, for a context that summarized content away. * * Content keys are namespaced by delivery channel so the composition paths * never cross-reference each other's payloads: @@ -97,6 +110,30 @@ export const DEDUP_BLOCKS = ['inherited_inputs', 'inherited_outputs', 'rules', ' /** Inherited blocks whose `note` is content-keyed separately from `items`. */ const INHERITED_SPLIT_BLOCKS = ['inherited_inputs', 'inherited_outputs'] as const; +/** Whether a value is an unchanged-marker rather than content. */ +function isMarker(value: unknown): boolean { + return !!value && typeof value === 'object' && (value as { delivery?: string }).delivery === 'unchanged'; +} + +/** + * Shared blocks of a projected technique that came back as markers — whole blocks, and the `note` / + * `items` halves of an inherited block that collapsed independently. Counted so a delivery can report + * how much repetition it dropped. + */ +export function countCollapsedBlocks(projected: Record): number { + let collapsed = 0; + for (const block of DEDUP_BLOCKS) { + const value = projected[block]; + if (isMarker(value)) { collapsed += 1; continue; } + if (value && typeof value === 'object' && !Array.isArray(value)) { + const rec = value as Record; + if (isMarker(rec['note'])) collapsed += 1; + if (isMarker(rec['items'])) collapsed += 1; + } + } + return collapsed; +} + /** * Content-key a field: collapse to an unchanged-marker when already delivered, * otherwise stage the hash. When `assignFull` is true, also write the full value @@ -111,10 +148,12 @@ function stageField( scope: string, keyPrefix: string, assignFull = false, + readLedger = true, ): void { const hash = contentHash(stringifyForResponse({ [field]: value })); const key = `${keyPrefix}:${hash}`; - if (deliveredHash(state, key, scope) === hash || newDeliveries[key] === hash) { + const held = readLedger && deliveredHash(state, key, scope) === hash; + if (held || newDeliveries[key] === hash) { out[field] = unchangedMarker(hash); } else { newDeliveries[key] = hash; @@ -130,21 +169,38 @@ function stageField( * whole-value candidates. Returns a shallow copy (input not mutated); newly-delivered * hashes are staged into `newDeliveries` for the caller to commit. * + * Two reasons a block may collapse, and `readLedger` selects which apply: + * + * - **The ledger** — this scope received the bytes on an earlier call. Only sound where the caller + * asked for reference delivery, since a marker is unreadable to a context that never received the + * bytes. + * - **This response** — a sibling in the SAME response carried the block in full, which + * `newDeliveries` records. Sound on any delivery, including a full one to a freshly spawned + * worker: the bytes are in the response the marker arrives in, above the marker. + * + * So `readLedger: false` is the full-delivery mode. It collapses the second and later copies of a + * block a response repeats — the contract and rules blocks every technique of a workflow shares — + * while the first copy ships in full. + * * @param projected `projectTechnique` output. * @param state session, for the delivery-ledger lookup. * @param newDeliveries accumulator of block-hashes to record. * @param scope delivery scope to look up (default: the session's agent). + * @param opts.readLedger consult this scope's earlier deliveries as well as this response's + * (default true). */ export function dedupTechniqueBlocks( projected: Record, state: SessionFile, newDeliveries: Record, scope: string = state.agentId, + opts: { readLedger?: boolean } = {}, ): Record { const out = { ...projected }; + const readLedger = opts.readLedger ?? true; if (out['provenance_note'] !== undefined) { - stageField(out, 'provenance_note', out['provenance_note'], state, newDeliveries, scope, 'technique:provenance_note', true); + stageField(out, 'provenance_note', out['provenance_note'], state, newDeliveries, scope, 'technique:provenance_note', true, readLedger); } for (const block of INHERITED_SPLIT_BLOCKS) { @@ -154,28 +210,29 @@ export function dedupTechniqueBlocks( const rec = value as Record; const next: Record = { ...rec }; if (rec['note'] !== undefined) { - stageField(next, 'note', rec['note'], state, newDeliveries, scope, `technique:${block}.note`); + stageField(next, 'note', rec['note'], state, newDeliveries, scope, `technique:${block}.note`, false, readLedger); } if (rec['items'] !== undefined) { - stageField(next, 'items', rec['items'], state, newDeliveries, scope, `technique:${block}.items`); + stageField(next, 'items', rec['items'], state, newDeliveries, scope, `technique:${block}.items`, false, readLedger); } // Whole-block key still recorded when both halves are full (first delivery), so a // reader that only understands whole-block markers keeps working. const wholeHash = contentHash(stringifyForResponse({ [block]: value })); const wholeKey = `technique:${block}:${wholeHash}`; - if (deliveredHash(state, wholeKey, scope) === wholeHash || newDeliveries[wholeKey] === wholeHash) { + const heldWhole = readLedger && deliveredHash(state, wholeKey, scope) === wholeHash; + if (heldWhole || newDeliveries[wholeKey] === wholeHash) { out[block] = unchangedMarker(wholeHash); } else { newDeliveries[wholeKey] = wholeHash; out[block] = next; } } else { - stageField(out, block, value, state, newDeliveries, scope, `technique:${block}`, true); + stageField(out, block, value, state, newDeliveries, scope, `technique:${block}`, true, readLedger); } } if (out['rules'] !== undefined) { - stageField(out, 'rules', out['rules'], state, newDeliveries, scope, 'technique:rules', true); + stageField(out, 'rules', out['rules'], state, newDeliveries, scope, 'technique:rules', true, readLedger); } return out; diff --git a/tests/reference-delivery.test.ts b/tests/reference-delivery.test.ts index 1fdb4591b..a83442704 100644 --- a/tests/reference-delivery.test.ts +++ b/tests/reference-delivery.test.ts @@ -39,6 +39,19 @@ function responseText(result: any): string { return (result.content[0] as { type: 'text'; text: string }).text; } +/** + * A get_activity response with the leading `batch:` block removed — the definitions and operations + * it delivers, without the live standing that leads it. The standing reports what this context has + * taken and been delivered, so it moves between two otherwise identical calls. + */ +function payloadOf(text: string): string { + if (!text.startsWith('batch:')) return text; + const lines = text.split('\n'); + let at = 1; + while (at < lines.length && (lines[at]!.startsWith(' ') || lines[at] === '')) at += 1; + return lines.slice(at).join('\n'); +} + describe('delivery ledger helpers', () => { it('contentHash is deterministic and 16 hex chars', () => { expect(contentHash('abc')).toBe(contentHash('abc')); @@ -167,8 +180,8 @@ describe('reference-not-repeat delivery (B1)', () => { return result; } - describe('get_activity default mode is unchanged', () => { - it('repeats the full bundle on every call and never emits markers', async () => { + describe('get_activity default mode delivers every technique in full', () => { + it('never references content from an earlier call, and repeats its payload byte for byte', async () => { const session = await startSession({ workflow_id: 'work-package', agent_id: 'w1' }); const idx = session['session_index'] as string; await enterActivity(idx, 'start-work-package'); @@ -180,12 +193,17 @@ describe('reference-not-repeat delivery (B1)', () => { expect(parsed.bundle['bundle_mode']).toBeUndefined(); const techniques = parsed.bundle['techniques'] as Record; expect(Object.keys(techniques).length).toBeGreaterThan(0); + // Every composed technique arrives whole. A marker inside one of them stands for a block an + // earlier entry of the SAME response carries, which is a different thing (#404 W7) and is + // covered by tests/send-once.test.ts. for (const value of Object.values(techniques)) { expect(isUnchangedMarker(value)).toBe(false); } } - // Byte-identical repetition — the pre-B1 behaviour full mode preserves. - expect(responseText(await getActivity(idx))).toBe(responseText(await getActivity(idx))); + // The payload repeats byte for byte. The batch standing that leads the response is live state — + // what this context has taken and been delivered — so it moves between calls by design. + expect(payloadOf(responseText(await getActivity(idx)))) + .toBe(payloadOf(responseText(await getActivity(idx)))); }); }); @@ -1306,19 +1324,31 @@ describe('reference-not-repeat delivery (B1)', () => { expect((await getResource(idx, { agent_id: 'w-b', bundle: 'reference' }))._meta?.['delivery']).toBeUndefined(); }); - it('leaves get_technique and get_resource in full delivery without the opt-in, and honours full: true over it', async () => { + it('answers a named context asking twice with a marker, and honours full: true over it', async () => { const session = await startSession({ workflow_id: 'work-package', agent_id: 'orchestrator' }); const idx = session['session_index'] as string; - // No bundle, default (fresh) session: a byte-identical refetch still arrives in full. - await getResource(idx, { agent_id: 'w-1' }); + // A context that names itself and asks again already holds the bytes, so the second answer is a + // marker whatever mode the call declares (#404 W9). expect((await getResource(idx, { agent_id: 'w-1' }))._meta?.['delivery']).toBeUndefined(); + expect((await getResource(idx, { agent_id: 'w-1' }))._meta?.['delivery']).toBe('unchanged'); - // Opt in, then force past it: `full: true` overrides `bundle: "reference"`. + // `full: true` is the escape hatch for a context that summarized the content away. + expect((await getResource(idx, { agent_id: 'w-1', full: true }))._meta?.['delivery']).toBeUndefined(); expect((await getResource(idx, { agent_id: 'w-1', bundle: 'reference' }))._meta?.['delivery']).toBe('unchanged'); expect((await getResource(idx, { agent_id: 'w-1', bundle: 'reference', full: true }))._meta?.['delivery']).toBeUndefined(); }); + it('never collapses for a caller that leaves its context unnamed', async () => { + const session = await startSession({ workflow_id: 'work-package', agent_id: 'orchestrator' }); + const idx = session['session_index'] as string; + + // With `agent_id` omitted the scope falls back to the session's own identity, which sibling + // workers share — so a marker could reach a context that never received the bytes. + expect((await getResource(idx, {}))._meta?.['delivery']).toBeUndefined(); + expect((await getResource(idx, {}))._meta?.['delivery']).toBeUndefined(); + }); + it('keys the on-disk ledger under the passed agent_id, leaving the session agent untouched', async () => { const slug = '2026-07-30-worker-scoped-ledger'; const session = await startSession({ diff --git a/tests/send-once.test.ts b/tests/send-once.test.ts new file mode 100644 index 000000000..3112bde95 --- /dev/null +++ b/tests/send-once.test.ts @@ -0,0 +1,233 @@ +/** + * What a delivery sends once, and what it says about where the context stands (#404 W7, W8, W9). + * + * Three defects with one theme: a response that carries the same block twice, two calls each answered + * in full, and an answer the receiving context could not read. All three are about a payload the + * server has already handed over once. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { join } from 'node:path'; +import { createHarness, rawText, isError, parseToolResponse, type Harness } from './e2e/harness.js'; + +/** An activity of the main workflow that binds several techniques, so its response repeats blocks. */ +const WORKFLOW_ID = 'work-package'; +const ACTIVITY_ID = 'implementation-analysis'; + +/** Every unchanged-marker in a payload, however deeply nested. */ +function countMarkers(text: string): number { + return (text.match(/delivery: unchanged/g) ?? []).length; +} + +describe('a delivery sends its shared blocks once (W7)', () => { + let h: Harness; + let sessionIndex: string; + + beforeAll(async () => { + h = await createHarness(); + const started = await h.client.callTool({ + name: 'start_session', + arguments: { + workflow_id: WORKFLOW_ID, agent_id: 'orchestrator', + planning_folder: join(h.workspaceDir, '.engineering/artifacts/planning', 'send-once-w7'), + }, + }); + if (isError(started)) throw new Error(rawText(started)); + sessionIndex = parseToolResponse(started).session_index as string; + const entered = await h.client.callTool({ + name: 'next_activity', arguments: { session_index: sessionIndex, activity_id: ACTIVITY_ID }, + }); + if (isError(entered)) throw new Error(rawText(entered)); + }); + + afterAll(async () => { await h?.close(); }); + + it('collapses a block a sibling of the same response already carried, on a full delivery', async () => { + const taken = await h.client.callTool({ + name: 'get_activity', + arguments: { session_index: sessionIndex, context_tokens: 200000, agent_id: 'fresh-worker-1' }, + }); + expect(isError(taken)).toBe(false); + const text = rawText(taken); + + // A fresh worker takes full delivery, so every marker here stands for a block an EARLIER entry of + // this same response carries in full — the only kind of reference a context holding nothing can + // resolve. + expect(countMarkers(text)).toBeGreaterThan(0); + // The note tells the reader where to look, and says so is about this response rather than context. + expect(text).toContain('EARLIER entry of THIS response'); + // The whole-bundle reference note, which is about content already in the reader's context, does + // not appear on a full delivery. + expect(text).not.toContain('already in your context'); + }); + + it('sends the first copy of every collapsed block in full', async () => { + const taken = await h.client.callTool({ + name: 'get_activity', + arguments: { session_index: sessionIndex, context_tokens: 200000, agent_id: 'fresh-worker-2' }, + }); + const text = rawText(taken); + // Each block that collapses anywhere is present in full somewhere: the marker is never the only + // copy in the payload. + for (const block of ['inherited_inputs', 'inherited_outputs', 'rules', 'provenance_note']) { + if (!text.includes(`${block}:`)) continue; + const firstAt = text.indexOf(`${block}:`); + const firstLines = text.slice(firstAt, firstAt + 200); + expect(firstLines, `the first ${block} in the payload is a marker with nothing to point at`) + .not.toMatch(/^[^\n]*\n\s+delivery: unchanged/); + } + }); +}); + +describe('a repeat fetch arrives as a marker (W9)', () => { + let h: Harness; + let sessionIndex: string; + + beforeAll(async () => { + h = await createHarness(); + const started = await h.client.callTool({ + name: 'start_session', + arguments: { + workflow_id: WORKFLOW_ID, agent_id: 'orchestrator', + planning_folder: join(h.workspaceDir, '.engineering/artifacts/planning', 'send-once-w9'), + }, + }); + if (isError(started)) throw new Error(rawText(started)); + sessionIndex = parseToolResponse(started).session_index as string; + const entered = await h.client.callTool({ + name: 'next_activity', arguments: { session_index: sessionIndex, activity_id: ACTIVITY_ID }, + }); + if (isError(entered)) throw new Error(rawText(entered)); + }); + + afterAll(async () => { await h?.close(); }); + + const fetchResource = (agentId: string | undefined, full?: boolean) => + h.client.callTool({ + name: 'get_resource', + arguments: { + session_index: sessionIndex, resource_id: 'meta/planning-readme', + ...(agentId ? { agent_id: agentId } : {}), ...(full === undefined ? {} : { full }), + }, + }); + + it('answers the second ask for a resource with a marker, and measures what came back', async () => { + const first = await fetchResource('repeat-worker'); + expect(isError(first)).toBe(false); + const firstChars = rawText(first).length; + + const second = await fetchResource('repeat-worker'); + expect(isError(second)).toBe(false); + const secondChars = rawText(second).length; + + expect(rawText(second)).toContain('delivery: unchanged'); + // The marker is a fraction of the body it stands for — the saving this defect was costing. + expect(secondChars).toBeLessThan(firstChars / 4); + }); + + it('serves the whole body again when the caller says it lost the content', async () => { + const first = await fetchResource('lost-content-worker'); + const firstChars = rawText(first).length; + const forced = await fetchResource('lost-content-worker', true); + expect(rawText(forced)).not.toContain('delivery: unchanged'); + expect(rawText(forced).length).toBe(firstChars); + }); + + it('never collapses for a caller that did not name its context', async () => { + // With `agent_id` omitted the scope is the session's own identity, which sibling workers share, so + // a marker could reach a context that never received the bytes. + const first = await fetchResource(undefined); + const second = await fetchResource(undefined); + expect(rawText(second)).not.toContain('delivery: unchanged'); + expect(rawText(second).length).toBe(rawText(first).length); + }); + + it('never collapses for a caller naming the session identity, which siblings share by construction', async () => { + // `dispatch_child` defaults `agent_id` to "worker", so two sibling workers can each pass the + // session's own identity without either having received what the other did. Naming it is not + // evidence of one context, so that scope collapses only on a declared reference opt-in. + const first = await fetchResource('orchestrator'); + const second = await fetchResource('orchestrator'); + expect(rawText(second)).not.toContain('delivery: unchanged'); + expect(rawText(second).length).toBe(rawText(first).length); + // The declared opt-in is still honoured for that identity — it is a claim about one context. + const optedIn = await h.client.callTool({ + name: 'get_resource', + arguments: { + session_index: sessionIndex, resource_id: 'meta/planning-readme', + agent_id: 'orchestrator', bundle: 'reference', + }, + }); + expect(rawText(optedIn)).toContain('delivery: unchanged'); + }); + + it('answers the second ask for a technique with a marker', async () => { + const args = { session_index: sessionIndex, agent_id: 'repeat-tech-worker', step_id: 'survey-codebase' }; + const first = await h.client.callTool({ name: 'get_technique', arguments: args }); + if (isError(first)) return; // step absent from this activity — nothing to measure + const second = await h.client.callTool({ name: 'get_technique', arguments: args }); + expect(rawText(second)).toContain('delivery: unchanged'); + expect(rawText(second).length).toBeLessThan(rawText(first).length / 4); + }); +}); + +describe('every delivery says where the context stands (W8)', () => { + let h: Harness; + let sessionIndex: string; + + beforeAll(async () => { + h = await createHarness(); + const started = await h.client.callTool({ + name: 'start_session', + arguments: { + workflow_id: WORKFLOW_ID, agent_id: 'orchestrator', + planning_folder: join(h.workspaceDir, '.engineering/artifacts/planning', 'send-once-w8'), + }, + }); + if (isError(started)) throw new Error(rawText(started)); + sessionIndex = parseToolResponse(started).session_index as string; + }); + + afterAll(async () => { await h?.close(); }); + + const take = async (activityId: string, agentId: string) => { + const entered = await h.client.callTool({ + name: 'next_activity', arguments: { session_index: sessionIndex, activity_id: activityId }, + }); + if (isError(entered)) throw new Error(rawText(entered)); + const taken = await h.client.callTool({ + name: 'get_activity', + arguments: { session_index: sessionIndex, context_tokens: 200000, agent_id: agentId }, + }); + if (isError(taken)) throw new Error(rawText(taken)); + return taken; + }; + + it('carries the batch standing in the response text, not only in _meta', async () => { + const taken = await take(ACTIVITY_ID, 'standing-worker'); + const text = rawText(taken); + expect(text).toContain('batch:'); + expect(text).toContain('may_continue:'); + expect(text).toContain('max_activities:'); + expect(text).toContain('budget_chars:'); + // The standing block leads the response, so it is read before the payload it describes. + expect(text.indexOf('batch:')).toBeLessThan(text.indexOf('session_index:')); + }); + + it('counts the activity being delivered as one this context has taken', async () => { + const taken = await take(ACTIVITY_ID, 'counting-worker'); + const meta = (taken as { _meta?: { batch?: { activities?: number; may_continue?: boolean } } })._meta; + expect(meta?.batch?.activities).toBe(1); + // A scope taking its first activity is admitted whatever it has read, so it may continue. + expect(meta?.batch?.may_continue).toBe(true); + expect(rawText(taken)).toContain('may_continue: true'); + }); + + it('reports a delivered-character count the response text and _meta agree on', async () => { + const taken = await take(ACTIVITY_ID, 'agreeing-worker'); + const meta = (taken as { _meta?: { batch?: { delivered_chars?: number } } })._meta; + const text = rawText(taken); + const stated = /delivered_chars: (\d+)/.exec(text); + expect(stated).not.toBeNull(); + expect(Number(stated![1])).toBe(meta?.batch?.delivered_chars); + }); +});