fix: address PR #504 review warnings
- workspace-context: a transiently unavailable but still-effective candidate keeps its cached trimmed digest in the directory's dedup slot, so an identical later sibling is not emitted as a duplicate set until the next successful reconciliation - app-boot: --resume rejects a following token that is itself resume syntax instead of accepting it as a session id - tui: the queued-steering badge tracks per-entry sources and a drain removes one matching entry, so loop-authored steering (no agent/queued) cannot consume a pending user message's slot
This commit is contained in:
@@ -448,7 +448,17 @@ export async function reconcileInstructionContext(
|
|||||||
const { directory } = decodeScopeKey(scope)
|
const { directory } = decodeScopeKey(scope)
|
||||||
const previous = effective.get(scope)
|
const previous = effective.get(scope)
|
||||||
const probe = await probeScopeInstruction(scope, projectRoot, resolved, fileSystem, options.signal)
|
const probe = await probeScopeInstruction(scope, projectRoot, resolved, fileSystem, options.signal)
|
||||||
if (probe.kind === 'unavailable') continue
|
if (probe.kind === 'unavailable') {
|
||||||
|
// Last-good-state: the candidate stays effective, so its cached trimmed
|
||||||
|
// digest must keep occupying the directory's dedup slot — otherwise an
|
||||||
|
// identical later sibling would be emitted as a duplicate `set` until the
|
||||||
|
// next successful reconciliation removed it again.
|
||||||
|
const cached = versions.get(scope)
|
||||||
|
if (cached !== undefined && previous !== undefined && previous.action !== 'remove') {
|
||||||
|
registerKeptTrimmed(directory, cached.trimmedDigest)
|
||||||
|
}
|
||||||
|
continue
|
||||||
|
}
|
||||||
if (probe.kind === 'absent') {
|
if (probe.kind === 'absent') {
|
||||||
if (previous === undefined || previous.action === 'remove') versions.delete(scope)
|
if (previous === undefined || previous.action === 'remove') versions.delete(scope)
|
||||||
else pushRemoval(scope, previous.path)
|
else pushRemoval(scope, previous.path)
|
||||||
|
|||||||
@@ -2201,6 +2201,47 @@ describe('dynamic nested workspace context injection', () => {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('keeps deduplicating against a loaded candidate whose probe transiently fails', async () => {
|
||||||
|
const root = await tempRepo()
|
||||||
|
const home = await tempRepo()
|
||||||
|
const ctx = new Context()
|
||||||
|
try {
|
||||||
|
await ctx.plugin(SystemPrompt)
|
||||||
|
await ctx.plugin(ToolRegistry)
|
||||||
|
await ctx.plugin(RecordingFileSystem)
|
||||||
|
const fs = ctx.fs as RecordingFileSystem
|
||||||
|
fs.entries.set(join(root, '.git'), { type: 'directory' })
|
||||||
|
fs.entries.set(join(root, 'pkg/AGENTS.md'), { type: 'file', content: 'nested rule' })
|
||||||
|
fs.entries.set(join(root, 'pkg/file.txt'), { type: 'file', content: 'hello' })
|
||||||
|
await ctx.plugin(ToolFs)
|
||||||
|
await ctx.plugin(workspaceContext, { dshHome: home, maxBytes: 65536 })
|
||||||
|
const agent = stubAgent(root)
|
||||||
|
|
||||||
|
const first = await ctx.tools.execute({
|
||||||
|
signal: testToolSignal,
|
||||||
|
callId: CallId('read-before-transient-probe-failure'), name: 'read', arguments: { file_path: join('pkg', 'file.txt') }, agent,
|
||||||
|
})
|
||||||
|
appendAdditionalContexts(agent, first)
|
||||||
|
expect(first.additionalContexts).toBeDefined()
|
||||||
|
|
||||||
|
// The loaded candidate's probe fails while an identical sibling appears:
|
||||||
|
// the cached candidate stays effective (last good state), so the sibling
|
||||||
|
// must still deduplicate against it rather than land as a duplicate set.
|
||||||
|
fs.throwOnStat.add(join(root, 'pkg/AGENTS.md'))
|
||||||
|
fs.entries.set(join(root, 'pkg/CLAUDE.md'), { type: 'file', content: 'nested rule' })
|
||||||
|
const duringFailure = await ctx.tools.execute({
|
||||||
|
signal: testToolSignal,
|
||||||
|
callId: CallId('read-during-transient-probe-failure'), name: 'read', arguments: { file_path: join('pkg', 'file.txt') }, agent,
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(duringFailure.additionalContexts).toBeUndefined()
|
||||||
|
} finally {
|
||||||
|
await ctx.fiber.dispose()
|
||||||
|
await rm(root, { recursive: true, force: true })
|
||||||
|
await rm(home, { recursive: true, force: true })
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
it('removes a previously rendered sibling once its content becomes a duplicate of an earlier candidate', async () => {
|
it('removes a previously rendered sibling once its content becomes a duplicate of an earlier candidate', async () => {
|
||||||
const root = await tempRepo()
|
const root = await tempRepo()
|
||||||
const home = await tempRepo()
|
const home = await tempRepo()
|
||||||
|
|||||||
@@ -66,7 +66,9 @@ export function parseResumeArg(
|
|||||||
if (arg === RESUME_FLAG || inlineValue) {
|
if (arg === RESUME_FLAG || inlineValue) {
|
||||||
if (resumeSessionId !== undefined) throw new Error(`${RESUME_FLAG} may be given only once`)
|
if (resumeSessionId !== undefined) throw new Error(`${RESUME_FLAG} may be given only once`)
|
||||||
const value = inlineValue ? arg.slice(RESUME_FLAG.length + 1) : argv[i + 1]
|
const value = inlineValue ? arg.slice(RESUME_FLAG.length + 1) : argv[i + 1]
|
||||||
if (value === undefined || value === '') {
|
// A following token that is itself resume syntax (`--resume --resume x`)
|
||||||
|
// is a missing id, not a session literally named `--resume…`.
|
||||||
|
if (value === undefined || value === '' || value === RESUME_FLAG || value.startsWith(`${RESUME_FLAG}=`)) {
|
||||||
throw new Error(`${RESUME_FLAG} requires a session id (e.g. ${RESUME_FLAG} <session-id>)`)
|
throw new Error(`${RESUME_FLAG} requires a session id (e.g. ${RESUME_FLAG} <session-id>)`)
|
||||||
}
|
}
|
||||||
resumeSessionId = value
|
resumeSessionId = value
|
||||||
|
|||||||
@@ -48,6 +48,11 @@ describe('parseResumeArg', () => {
|
|||||||
expect(() => parseResumeArg(['--resume='])).toThrow('--resume requires a session id')
|
expect(() => parseResumeArg(['--resume='])).toThrow('--resume requires a session id')
|
||||||
expect(() => parseResumeArg(['--resume', 'a', '--resume', 'b'])).toThrow('--resume may be given only once')
|
expect(() => parseResumeArg(['--resume', 'a', '--resume', 'b'])).toThrow('--resume may be given only once')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('rejects resume syntax used as the flag value instead of resuming a session named like the flag', () => {
|
||||||
|
expect(() => parseResumeArg(['--resume', '--resume', 'sess'])).toThrow('--resume requires a session id')
|
||||||
|
expect(() => parseResumeArg(['--resume', '--resume=sess'])).toThrow('--resume requires a session id')
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('loadEnv', () => {
|
describe('loadEnv', () => {
|
||||||
|
|||||||
@@ -1307,12 +1307,15 @@ export function createTuiChat(
|
|||||||
let toolsExpanded = false
|
let toolsExpanded = false
|
||||||
let streaming: StreamingAssistantComponent | undefined
|
let streaming: StreamingAssistantComponent | undefined
|
||||||
let runningStatus: RunningStatus | undefined
|
let runningStatus: RunningStatus | undefined
|
||||||
// Steering messages the user queued during the running turn that the loop has
|
// Steering messages queued during the running turn (`agent/queued`) that the
|
||||||
// not yet drained, shown as a badge on the status line. Reset on the
|
// loop has not yet drained, shown as a badge on the status line. Each entry is
|
||||||
// running→idle status transition, which also absorbs a cancellation that
|
// the queued message's serialized source: a drain (`steering/message`) removes
|
||||||
// clears the queue without logging drains; the status line exists only while
|
// one MATCHING entry, so loop-authored steering — continuation reasons enter
|
||||||
// running, so idle carries no badge to keep current.
|
// the inbox without an `agent/queued` event — cannot consume a pending user
|
||||||
let pendingSteering = 0
|
// message's slot. Cleared on leaving `running`, which also absorbs a
|
||||||
|
// cancellation that discards the queue without logging drains; the status
|
||||||
|
// line exists only while running, so idle carries no badge to keep current.
|
||||||
|
const pendingSteering: string[] = []
|
||||||
let disposed = false
|
let disposed = false
|
||||||
let shuttingDown: Promise<void> | undefined
|
let shuttingDown: Promise<void> | undefined
|
||||||
// Optional: skills mount conditionally, so read the global service store
|
// Optional: skills mount conditionally, so read the global service store
|
||||||
@@ -1509,7 +1512,9 @@ export function createTuiChat(
|
|||||||
// controller's phase and the current steering count.
|
// controller's phase and the current steering count.
|
||||||
const renderStatus = (running: RunningStatus): void => {
|
const renderStatus = (running: RunningStatus): void => {
|
||||||
const at = now()
|
const at = now()
|
||||||
running.loader.setMessage(formatTurnStatus(running.phase, at - running.phaseStartedAt, at - running.stepStartedAt, pendingSteering))
|
running.loader.setMessage(
|
||||||
|
formatTurnStatus(running.phase, at - running.phaseStartedAt, at - running.stepStartedAt, pendingSteering.length),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Move to a derived phase, resetting the phase timer on a genuine change and
|
// Move to a derived phase, resetting the phase timer on a genuine change and
|
||||||
@@ -1536,7 +1541,7 @@ export function createTuiChat(
|
|||||||
const phase = prior?.phase ?? 'waiting'
|
const phase = prior?.phase ?? 'waiting'
|
||||||
const phaseStartedAt = prior?.phaseStartedAt ?? at
|
const phaseStartedAt = prior?.phaseStartedAt ?? at
|
||||||
const stepStartedAt = prior?.stepStartedAt ?? at
|
const stepStartedAt = prior?.stepStartedAt ?? at
|
||||||
const message = formatTurnStatus(phase, at - phaseStartedAt, at - stepStartedAt, pendingSteering)
|
const message = formatTurnStatus(phase, at - phaseStartedAt, at - stepStartedAt, pendingSteering.length)
|
||||||
const loader = new Loader(ui, text => palette.accent(text), text => palette.muted(text), message)
|
const loader = new Loader(ui, text => palette.accent(text), text => palette.muted(text), message)
|
||||||
statusContainer.addChild(loader)
|
statusContainer.addChild(loader)
|
||||||
const running: RunningStatus = {
|
const running: RunningStatus = {
|
||||||
@@ -2240,12 +2245,16 @@ export function createTuiChat(
|
|||||||
if (session !== agent.session) return
|
if (session !== agent.session) return
|
||||||
recordEventUsage(tokens, event)
|
recordEventUsage(tokens, event)
|
||||||
advanceTurnPhase(event)
|
advanceTurnPhase(event)
|
||||||
if (event.type === 'steering/message' && pendingSteering > 0) {
|
if (event.type === 'steering/message') {
|
||||||
// A queued steering message reached the model as it drained; drop it from
|
// A queued steering message reached the model as it drained; drop its
|
||||||
// the badge. Clamped because loop-authored steering (e.g. continuation
|
// entry from the badge. Matching by source keeps loop-authored steering
|
||||||
// reasons) also logs here without a matching user-queued increment.
|
// (e.g. continuation reasons), which logs here without a matching
|
||||||
pendingSteering -= 1
|
// `agent/queued` increment, from consuming a pending user slot.
|
||||||
refreshStatus()
|
const drained = pendingSteering.indexOf(JSON.stringify(event.data.source))
|
||||||
|
if (drained >= 0) {
|
||||||
|
pendingSteering.splice(drained, 1)
|
||||||
|
refreshStatus()
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if ('surfaceOp' in event && typeof event.surfaceOp === 'object') {
|
if ('surfaceOp' in event && typeof event.surfaceOp === 'object') {
|
||||||
rebuildTranscript(false)
|
rebuildTranscript(false)
|
||||||
@@ -2256,7 +2265,7 @@ export function createTuiChat(
|
|||||||
})
|
})
|
||||||
const disposeQueued = ctx.on('agent/queued', (subject, _content, info) => {
|
const disposeQueued = ctx.on('agent/queued', (subject, _content, info) => {
|
||||||
if (subject !== agent || !info.steering) return
|
if (subject !== agent || !info.steering) return
|
||||||
pendingSteering += 1
|
pendingSteering.push(JSON.stringify(info.source))
|
||||||
refreshStatus()
|
refreshStatus()
|
||||||
})
|
})
|
||||||
const disposeStatus = ctx.on('agent/status', (subject, status) => {
|
const disposeStatus = ctx.on('agent/status', (subject, status) => {
|
||||||
@@ -2264,7 +2273,7 @@ export function createTuiChat(
|
|||||||
// Leaving 'running' ends the turn's status line; clear any badge so the
|
// Leaving 'running' ends the turn's status line; clear any badge so the
|
||||||
// next running turn starts from zero (and a cancellation, which discards
|
// next running turn starts from zero (and a cancellation, which discards
|
||||||
// the queue without logging drains, cannot strand a stale count).
|
// the queue without logging drains, cannot strand a stale count).
|
||||||
if (status !== 'running') pendingSteering = 0
|
if (status !== 'running') pendingSteering.length = 0
|
||||||
setStatus(status)
|
setStatus(status)
|
||||||
})
|
})
|
||||||
const disposeError = ctx.on('agent/error', (subject, turn, step, error) => {
|
const disposeError = ctx.on('agent/error', (subject, turn, step, error) => {
|
||||||
|
|||||||
@@ -584,13 +584,28 @@ describe('pi-tui chat lifecycle and transcript', () => {
|
|||||||
expect(result.terminal.output).toContain('— Enter sends steering, Esc cancels')
|
expect(result.terminal.output).toContain('— Enter sends steering, Esc cancels')
|
||||||
expect(result.terminal.output).not.toContain('queued')
|
expect(result.terminal.output).not.toContain('queued')
|
||||||
|
|
||||||
// A loop-authored steering drain past zero clamps rather than underflowing.
|
// A drain with no matching queued entry is ignored rather than underflowing.
|
||||||
result.terminal.output = ''
|
result.terminal.output = ''
|
||||||
drainSteering('continuation')
|
drainSteering('continuation')
|
||||||
queueSteering('after')
|
queueSteering('after')
|
||||||
await tick()
|
await tick()
|
||||||
expect(result.terminal.output).toContain('1 queued')
|
expect(result.terminal.output).toContain('1 queued')
|
||||||
|
|
||||||
|
// A loop-authored steering event (plugin source, no matching agent/queued)
|
||||||
|
// cannot consume a pending user slot, even when it drains first.
|
||||||
|
result.terminal.output = ''
|
||||||
|
result.session.append('steering/message', {
|
||||||
|
turn: 1,
|
||||||
|
content: [{ type: 'text', text: 'continue: goal not reached' }],
|
||||||
|
source: { kind: 'plugin', plugin: 'hooks' },
|
||||||
|
}, { surfaceOp: 'append' })
|
||||||
|
await tick()
|
||||||
|
expect(result.terminal.output).toContain('1 queued')
|
||||||
|
result.terminal.output = ''
|
||||||
|
drainSteering('after')
|
||||||
|
await tick()
|
||||||
|
expect(result.terminal.output).not.toContain('queued')
|
||||||
|
|
||||||
// The turn ending resets the badge, so the next running turn starts clean.
|
// The turn ending resets the badge, so the next running turn starts clean.
|
||||||
result.agent.status = 'idle'
|
result.agent.status = 'idle'
|
||||||
result.ctx.emit('agent/status', result.agent, 'idle')
|
result.ctx.emit('agent/status', result.agent, 'idle')
|
||||||
|
|||||||
Reference in New Issue
Block a user