Fix workspace context review findings
This commit is contained in:
@@ -35,7 +35,7 @@ The hooks **themselves** run in the agent's session workspace: for the agent-sco
|
||||
| CC hook | Harness seam | Mapping |
|
||||
|---|---|---|
|
||||
| `SessionStart` | `agent/session-start` (emit) | additionalContext → `agent.inject()` into the new session (cannot block) |
|
||||
| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `deny` → `PromptDecision.block`; additionalContext-only → delegate via `next()` then fold context onto the downstream decision (a later listener can still block/rewrite) |
|
||||
| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `deny` → `PromptDecision.block`; additionalContext-only → delegate via `next()` then prepend a separately sourced context to downstream `additionalContexts` (a later listener can still block/rewrite) |
|
||||
| `PreToolUse` | `tools/pre-execute` (waterfall) | `deny` → `PreToolDecision.deny`; `ask` → `PreToolDecision.ask` |
|
||||
| `PostToolUse` | `tools/post-execute` (waterfall) | `deny` → `block` with feedback; additionalContext-only → delegate via `next()` then prepend a separately sourced context to the downstream decision; Code Mode defers sub-call contexts until the outer `run_code` result |
|
||||
| `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue`, feeding its reason as next-step steering |
|
||||
|
||||
@@ -221,22 +221,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
return { content, source: PLUGIN_SOURCE }
|
||||
}
|
||||
|
||||
/**
|
||||
* Concatenate this bridge's prompt {@link HookContext} with a downstream
|
||||
* prompt listener's optional one, so folding additionalContext drops neither.
|
||||
* The merged block
|
||||
* carries a single `source` — this bridge's — because a `HookContext` holds one
|
||||
* `MessageSource` and the seam cannot represent mixed provenance; the rendered
|
||||
* `context/message` only distinguishes by `source.kind` ('plugin'), so a
|
||||
* downstream plugin's text is still correctly framed as plugin context, not a
|
||||
* user prompt.
|
||||
*/
|
||||
function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext {
|
||||
if (!theirs) return ours
|
||||
return { content: [...ours.content, ...theirs.content], source: ours.source }
|
||||
}
|
||||
|
||||
/** Prepend one post-tool context without flattening downstream provenance. */
|
||||
/** Prepend one context without flattening downstream provenance or metadata. */
|
||||
function prependContext(ours: HookContext, theirs: HookContext[] | undefined): HookContext[] {
|
||||
return [ours, ...theirs ?? []]
|
||||
}
|
||||
@@ -279,7 +264,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
return {
|
||||
kind: 'allow',
|
||||
...downstream.content !== undefined ? { content: downstream.content } : {},
|
||||
additionalContext: concatContext(ours, downstream.additionalContext),
|
||||
additionalContexts: prependContext(ours, downstream.additionalContexts),
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -479,9 +479,9 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () =>
|
||||
expect(turnEnd?.type === 'turn/end' && turnEnd.data.reason).toMatchObject({ kind: 'rejected', reason: 'policy veto' })
|
||||
})
|
||||
|
||||
it('folds the bridge additionalContext WITH a downstream listener that also adds context', async () => {
|
||||
it('preserves separate bridge and downstream prompt contexts with framing and metadata', async () => {
|
||||
// Both the bridge hook and a later prompt-submit listener attach context; the
|
||||
// request must see BOTH (concatContext keeps the downstream one too).
|
||||
// request must see both as separately sourced durable events.
|
||||
const d = dir()
|
||||
const s = sh(d, 'ctx.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"from-bridge"}}\'\n')
|
||||
const path = hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: s }] }] })
|
||||
@@ -490,7 +490,12 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () =>
|
||||
ctx.on('agent/prompt-submit', async () => ({
|
||||
kind: 'allow' as const,
|
||||
content: [{ type: 'text' as const, text: 'rewritten-prompt' }],
|
||||
additionalContext: { content: [{ type: 'text' as const, text: 'from-downstream' }], source: { kind: 'plugin' as const, plugin: 'policy' } },
|
||||
additionalContexts: [{
|
||||
content: [{ type: 'text' as const, text: 'from-downstream' }],
|
||||
source: { kind: 'plugin' as const, plugin: 'policy' },
|
||||
envelope: 'raw' as const,
|
||||
meta: { owner: 'policy' },
|
||||
}],
|
||||
}))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
agent.send([{ type: 'text', text: 'go' }])
|
||||
@@ -502,6 +507,13 @@ describe('hooks-claude coverage — continue:false, context arm, no-cwd', () =>
|
||||
// the original prompt was replaced by the downstream rewrite
|
||||
const userMsg = events(agent).find(e => e.type === 'user/message')
|
||||
expect(userMsg?.type === 'user/message' && userMsg.data.content.some(b => b.type === 'text' && b.text === 'rewritten-prompt')).toBe(true)
|
||||
const contexts = events(agent).filter(event => event.type === 'context/message')
|
||||
expect(contexts.map(event => event.type === 'context/message' && event.data.source)).toEqual([
|
||||
{ kind: 'plugin', plugin: 'hooks-claude' },
|
||||
{ kind: 'plugin', plugin: 'policy' },
|
||||
])
|
||||
expect(contexts[1]?.type === 'context/message' && contexts[1].data.envelope).toBe('raw')
|
||||
expect(contexts[1]?.type === 'context/message' && contexts[1].data.meta).toEqual({ owner: 'policy' })
|
||||
})
|
||||
|
||||
it('folds the bridge PostToolUse context onto a downstream ACCEPT that replaces content', async () => {
|
||||
|
||||
@@ -41,7 +41,7 @@ The hooks themselves run in the agent's session workspace: for the agent-scoped
|
||||
| Codex hook | Harness seam | Mapping |
|
||||
|---|---|---|
|
||||
| `SessionStart` | `agent/session-start` (emit) | a plain-stdout hook's output → additionalContext → `agent.inject()` |
|
||||
| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `block` (exit 2) → `PromptDecision.block`; additionalContext-only → delegate via `next()` then fold context onto the downstream decision |
|
||||
| `UserPromptSubmit` | `agent/prompt-submit` (waterfall) | `block` (exit 2) → `PromptDecision.block`; additionalContext-only → delegate via `next()` then prepend a separately sourced context to downstream `additionalContexts` |
|
||||
| `PreToolUse` | `tools/pre-execute` (waterfall) | `block` → `PreToolDecision.deny` (no `allow`/`ask`) |
|
||||
| `PostToolUse` | `tools/post-execute` (waterfall) | `block` → `block` with feedback; additionalContext-only → delegate via `next()` then prepend a separately sourced context to the downstream decision; Code Mode defers sub-call contexts until the outer `run_code` result |
|
||||
| `Stop` | `agent/turn-continuation` (waterfall) | a blocking Stop hook forces `continue` with the reason as next-step steering |
|
||||
|
||||
@@ -176,21 +176,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
return { content, source: PLUGIN_SOURCE }
|
||||
}
|
||||
|
||||
/**
|
||||
* Concatenate this bridge's prompt {@link HookContext} with a downstream
|
||||
* prompt listener's optional one, so folding additionalContext drops neither.
|
||||
* The merged block
|
||||
* carries a single `source` — this bridge's — because a `HookContext` holds one
|
||||
* `MessageSource` and the seam cannot represent mixed provenance; the rendered
|
||||
* `context/message` only distinguishes by `source.kind` ('plugin'), so a
|
||||
* downstream plugin's text is still correctly framed as plugin context.
|
||||
*/
|
||||
function concatContext(ours: HookContext, theirs: HookContext | undefined): HookContext {
|
||||
if (!theirs) return ours
|
||||
return { content: [...ours.content, ...theirs.content], source: ours.source }
|
||||
}
|
||||
|
||||
/** Prepend one post-tool context without flattening downstream provenance. */
|
||||
/** Prepend one context without flattening downstream provenance or metadata. */
|
||||
function prependContext(ours: HookContext, theirs: HookContext[] | undefined): HookContext[] {
|
||||
return [ours, ...theirs ?? []]
|
||||
}
|
||||
@@ -222,7 +208,7 @@ export function apply(ctx: Context, config: Config): void {
|
||||
return {
|
||||
kind: 'allow',
|
||||
...downstream.content !== undefined ? { content: downstream.content } : {},
|
||||
additionalContext: concatContext(ours, downstream.additionalContext),
|
||||
additionalContexts: prependContext(ours, downstream.additionalContexts),
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -86,7 +86,7 @@ describe('hooks-codex coverage — decision mapping paths', () => {
|
||||
expect(te?.type === 'turn/end' && te.data.reason).toMatchObject({ kind: 'rejected', reason: 'policy veto' })
|
||||
})
|
||||
|
||||
it('folds the bridge additionalContext WITH a downstream listener that also adds context', async () => {
|
||||
it('preserves separate bridge and downstream prompt contexts with framing and metadata', async () => {
|
||||
const d = dir()
|
||||
hooks(d, { UserPromptSubmit: [{ hooks: [{ type: 'command', command: sh(d, 'c.sh', '#!/usr/bin/env bash\necho \'{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"from-bridge"}}\'\n') }] }] })
|
||||
const adapter = new MockAdapter([textResponse('ok')])
|
||||
@@ -94,7 +94,12 @@ describe('hooks-codex coverage — decision mapping paths', () => {
|
||||
ctx.on('agent/prompt-submit', async () => ({
|
||||
kind: 'allow' as const,
|
||||
content: [{ type: 'text' as const, text: 'rewritten-prompt' }],
|
||||
additionalContext: { content: [{ type: 'text' as const, text: 'from-downstream' }], source: { kind: 'plugin' as const, plugin: 'policy' } },
|
||||
additionalContexts: [{
|
||||
content: [{ type: 'text' as const, text: 'from-downstream' }],
|
||||
source: { kind: 'plugin' as const, plugin: 'policy' },
|
||||
envelope: 'raw' as const,
|
||||
meta: { owner: 'policy' },
|
||||
}],
|
||||
}))
|
||||
const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' })
|
||||
agent.send([{ type: 'text', text: 'go' }]); await waitForIdle(ctx, agent)
|
||||
@@ -102,6 +107,13 @@ describe('hooks-codex coverage — decision mapping paths', () => {
|
||||
expect(req).toContain('from-bridge')
|
||||
expect(req).toContain('from-downstream')
|
||||
expect(req).toContain('rewritten-prompt')
|
||||
const contexts = events(agent).filter(event => event.type === 'context/message')
|
||||
expect(contexts.map(event => event.type === 'context/message' && event.data.source)).toEqual([
|
||||
{ kind: 'plugin', plugin: 'hooks-codex' },
|
||||
{ kind: 'plugin', plugin: 'policy' },
|
||||
])
|
||||
expect(contexts[1]?.type === 'context/message' && contexts[1].data.envelope).toBe('raw')
|
||||
expect(contexts[1]?.type === 'context/message' && contexts[1].data.meta).toEqual({ owner: 'policy' })
|
||||
})
|
||||
|
||||
it('folds the bridge PostToolUse context onto a downstream ACCEPT that replaces content', async () => {
|
||||
|
||||
Reference in New Issue
Block a user