fix review findings for session queries
This commit is contained in:
@@ -658,13 +658,10 @@ async function collectPages<T>(
|
|||||||
signal.throwIfAborted()
|
signal.throwIfAborted()
|
||||||
for (const item of page.items) {
|
for (const item of page.items) {
|
||||||
if (!accept(item)) continue
|
if (!accept(item)) continue
|
||||||
items.push(item)
|
|
||||||
if (items.length === maxResults) {
|
if (items.length === maxResults) {
|
||||||
return {
|
return { items, capped: true }
|
||||||
items,
|
|
||||||
capped: page.nextCursor !== undefined || item !== page.items.at(-1),
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
items.push(item)
|
||||||
}
|
}
|
||||||
if (page.nextCursor === undefined) return { items, capped: false }
|
if (page.nextCursor === undefined) return { items, capped: false }
|
||||||
if (seen.has(page.nextCursor)) {
|
if (seen.has(page.nextCursor)) {
|
||||||
|
|||||||
@@ -506,8 +506,10 @@ describe('search paging, prior-history bounds, titles, and cancellation', () =>
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
return Promise.resolve({
|
return Promise.resolve({
|
||||||
items: [sessionHit('b', '/work', 'second')],
|
items: [
|
||||||
nextCursor: SessionSearchCursor('more'),
|
sessionHit('b', '/work', 'second'),
|
||||||
|
sessionHit('additional-authorized', '/work', 'third'),
|
||||||
|
],
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -523,6 +525,27 @@ describe('search paging, prior-history bounds, titles, and cancellation', () =>
|
|||||||
expect(output).toContain('Result cap reached')
|
expect(output).toContain('Result cap reached')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('does not report a cap when only rejected hits remain after the authorized limit', async () => {
|
||||||
|
const mounted = await mount({ maxSearchResults: 1 })
|
||||||
|
const cursor = SessionSearchCursor('rejected-tail')
|
||||||
|
FakeQuery.sessionSearch = request => request.cursor === undefined
|
||||||
|
? Promise.resolve({
|
||||||
|
items: [sessionHit('authorized', '/work')],
|
||||||
|
nextCursor: cursor,
|
||||||
|
})
|
||||||
|
: Promise.resolve({
|
||||||
|
items: [
|
||||||
|
sessionHit(mounted.caller.id, '/work'),
|
||||||
|
sessionHit('outside', '/outside'),
|
||||||
|
],
|
||||||
|
})
|
||||||
|
|
||||||
|
const output = text(await mounted.call('session_search', { query: 'needle' }))
|
||||||
|
expect(FakeQuery.sessionRequests.map(request => request.cursor)).toEqual([undefined, cursor])
|
||||||
|
expect(output).toContain('Session authorized')
|
||||||
|
expect(output).not.toContain('Result cap reached')
|
||||||
|
})
|
||||||
|
|
||||||
it('preserves stale-cursor diagnostics without transparently restarting', async () => {
|
it('preserves stale-cursor diagnostics without transparently restarting', async () => {
|
||||||
const mounted = await mount({ maxSearchResults: 2 })
|
const mounted = await mount({ maxSearchResults: 2 })
|
||||||
const cursor = SessionSearchCursor('stale-next')
|
const cursor = SessionSearchCursor('stale-next')
|
||||||
|
|||||||
@@ -19,6 +19,8 @@ const CWD_ROOTED_PATH_RE = /\{\{cwd\}\}(?:[\\/][^\s<>"'`]+)+/g
|
|||||||
const PATH_TAG_RE = /(<path>)([^<]*)(<\/path>)/g
|
const PATH_TAG_RE = /(<path>)([^<]*)(<\/path>)/g
|
||||||
const ADDITIONAL_INSTRUCTIONS_PATH_RE = /(Additional instructions from: )([^\r\n]+)/g
|
const ADDITIONAL_INSTRUCTIONS_PATH_RE = /(Additional instructions from: )([^\r\n]+)/g
|
||||||
const EMBEDDED_EVENT_TIME_RE = /("time": )\d+(?=,\r?\n)/g
|
const EMBEDDED_EVENT_TIME_RE = /("time": )\d+(?=,\r?\n)/g
|
||||||
|
const EVENT_READ_RESULT_RE
|
||||||
|
= /^Session [^\r\n]+ — [^\r\n]+\r?\nTarget event seq \d+:\r?\n```json\r?\n\{\r?\n/
|
||||||
|
|
||||||
/** A UUID v4 string, the shape `randomUUID()` produces for session ids. */
|
/** A UUID v4 string, the shape `randomUUID()` produces for session ids. */
|
||||||
const UUID_RE = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/gi
|
const UUID_RE = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/gi
|
||||||
@@ -74,9 +76,12 @@ function scrubString(value: string, ctx: NormalizeContext, cwdPathMode: CwdPathM
|
|||||||
}
|
}
|
||||||
out = out.replace(LOCAL_SPILL_PATH_RE, (_match, name: string) => `{{spillLocator:${name}}}`)
|
out = out.replace(LOCAL_SPILL_PATH_RE, (_match, name: string) => `{{spillLocator:${name}}}`)
|
||||||
out = out.replace(SNAPSHOT_SPILL_PATH_RE, (_match, name: string) => `{{spillLocator:${name}}}`)
|
out = out.replace(SNAPSHOT_SPILL_PATH_RE, (_match, name: string) => `{{spillLocator:${name}}}`)
|
||||||
// Exact event-read tools render pretty JSON inside a text block. The event's
|
// Exact event-read results render pretty JSON inside a distinctive text
|
||||||
// wall-clock time is volatile even though its seq and payload are deterministic.
|
// envelope. Restrict time scrubbing to that envelope so JSON printed by
|
||||||
out = out.replace(EMBEDDED_EVENT_TIME_RE, `$1${EVENT_TIME}`)
|
// models, bash, or unrelated tools remains regression-visible.
|
||||||
|
if (EVENT_READ_RESULT_RE.test(out)) {
|
||||||
|
out = out.replace(EMBEDDED_EVENT_TIME_RE, `$1${EVENT_TIME}`)
|
||||||
|
}
|
||||||
for (const id of ctx.sessionIds) out = out.split(id).join(SESSION_ID)
|
for (const id of ctx.sessionIds) out = out.split(id).join(SESSION_ID)
|
||||||
out = out.replace(UUID_RE, SESSION_ID)
|
out = out.replace(UUID_RE, SESSION_ID)
|
||||||
return out
|
return out
|
||||||
|
|||||||
@@ -134,7 +134,7 @@ Additional instructions from: nested\AGENTS.md`,
|
|||||||
type: 'content',
|
type: 'content',
|
||||||
content: {
|
content: {
|
||||||
type: 'text',
|
type: 'text',
|
||||||
text: 'Target event:\n```json\n{\n "seq": 4,\n "time": 1784876275593,\n "data": {}\n}\n```',
|
text: 'Session prior — title\nTarget event seq 4:\n```json\n{\n "seq": 4,\n "time": 1784876275593,\n "data": {}\n}\n```',
|
||||||
},
|
},
|
||||||
}],
|
}],
|
||||||
},
|
},
|
||||||
@@ -145,6 +145,28 @@ Additional instructions from: nested\AGENTS.md`,
|
|||||||
expect(out).not.toContain('1784876275593')
|
expect(out).not.toContain('1784876275593')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('preserves event-like timestamps in unrelated output text', () => {
|
||||||
|
const raw = JSON.stringify({
|
||||||
|
jsonrpc: '2.0',
|
||||||
|
method: 'session/update',
|
||||||
|
params: {
|
||||||
|
update: {
|
||||||
|
sessionUpdate: 'tool_call_update',
|
||||||
|
content: [{
|
||||||
|
type: 'content',
|
||||||
|
content: {
|
||||||
|
type: 'text',
|
||||||
|
text: 'bash output:\n```json\n{\n "time": 1784876275593,\n "data": {}\n}\n```',
|
||||||
|
},
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
const out = normalizeStdout(raw, ctx)
|
||||||
|
expect(out).toContain('1784876275593')
|
||||||
|
expect(out).not.toContain('{{eventTime}}')
|
||||||
|
})
|
||||||
|
|
||||||
it('throws on a non-JSON stdout line (the purity check)', () => {
|
it('throws on a non-JSON stdout line (the purity check)', () => {
|
||||||
const raw = `${JSON.stringify({ jsonrpc: '2.0', id: 1 })}\noops a log leaked\n`
|
const raw = `${JSON.stringify({ jsonrpc: '2.0', id: 1 })}\noops a log leaked\n`
|
||||||
expect(() => normalizeStdout(raw, ctx)).toThrow()
|
expect(() => normalizeStdout(raw, ctx)).toThrow()
|
||||||
|
|||||||
Reference in New Issue
Block a user