fix(review): resolve 3 review-work blocking issues
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
/// <reference path="../../../bun-test.d.ts" />
|
/// <reference path="../../../bun-test.d.ts" />
|
||||||
|
|
||||||
|
import { createOpencodeClient } from "@opencode-ai/sdk"
|
||||||
import { describe, expect, it as test } from "bun:test"
|
import { describe, expect, it as test } from "bun:test"
|
||||||
|
|
||||||
import { createGptPermissionContinuationHook } from "."
|
import { createGptPermissionContinuationHook } from "."
|
||||||
@@ -17,25 +18,79 @@ type SessionMessage = {
|
|||||||
parts?: Array<{ type: string; text?: string }>
|
parts?: Array<{ type: string; text?: string }>
|
||||||
}
|
}
|
||||||
|
|
||||||
function createMockPluginInput(messages: SessionMessage[]) {
|
type GptPermissionContext = Parameters<typeof createGptPermissionContinuationHook>[0]
|
||||||
const promptCalls: string[] = []
|
|
||||||
|
|
||||||
const ctx = {
|
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||||
directory: "/tmp/test",
|
return typeof value === "object" && value !== null
|
||||||
client: {
|
}
|
||||||
session: {
|
|
||||||
messages: async () => ({ data: messages }),
|
function extractPromptText(input: unknown): string {
|
||||||
prompt: async (input: { body: { parts: Array<{ text: string }> } }) => {
|
if (!isRecord(input)) return ""
|
||||||
promptCalls.push(input.body.parts[0]?.text ?? "")
|
|
||||||
return {}
|
const body = input.body
|
||||||
},
|
if (!isRecord(body)) return ""
|
||||||
promptAsync: async (input: { body: { parts: Array<{ text: string }> } }) => {
|
|
||||||
promptCalls.push(input.body.parts[0]?.text ?? "")
|
const parts = body.parts
|
||||||
return {}
|
if (!Array.isArray(parts)) return ""
|
||||||
},
|
|
||||||
|
const firstPart = parts[0]
|
||||||
|
if (!isRecord(firstPart)) return ""
|
||||||
|
|
||||||
|
return typeof firstPart.text === "string" ? firstPart.text : ""
|
||||||
|
}
|
||||||
|
|
||||||
|
function createMockPluginInput(messages: SessionMessage[]): {
|
||||||
|
ctx: GptPermissionContext
|
||||||
|
promptCalls: string[]
|
||||||
|
} {
|
||||||
|
const promptCalls: string[] = []
|
||||||
|
const client = createOpencodeClient({ directory: "/tmp/test" })
|
||||||
|
const shell = Object.assign(
|
||||||
|
() => {
|
||||||
|
throw new Error("$ is not used in this test")
|
||||||
|
},
|
||||||
|
{
|
||||||
|
braces: () => [],
|
||||||
|
escape: (input: string) => input,
|
||||||
|
env() {
|
||||||
|
return shell
|
||||||
|
},
|
||||||
|
cwd() {
|
||||||
|
return shell
|
||||||
|
},
|
||||||
|
nothrow() {
|
||||||
|
return shell
|
||||||
|
},
|
||||||
|
throws() {
|
||||||
|
return shell
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
} as any
|
)
|
||||||
|
const request = new Request("http://localhost")
|
||||||
|
const response = new Response()
|
||||||
|
|
||||||
|
Reflect.set(client.session, "messages", async () => ({ data: messages, error: undefined, request, response }))
|
||||||
|
Reflect.set(client.session, "prompt", async (input: unknown) => {
|
||||||
|
promptCalls.push(extractPromptText(input))
|
||||||
|
return { data: undefined, error: undefined, request, response }
|
||||||
|
})
|
||||||
|
Reflect.set(client.session, "promptAsync", async (input: unknown) => {
|
||||||
|
promptCalls.push(extractPromptText(input))
|
||||||
|
return { data: undefined, error: undefined, request, response }
|
||||||
|
})
|
||||||
|
|
||||||
|
const ctx: GptPermissionContext = {
|
||||||
|
client,
|
||||||
|
project: {
|
||||||
|
id: "test-project",
|
||||||
|
worktree: "/tmp/test",
|
||||||
|
time: { created: Date.now() },
|
||||||
|
},
|
||||||
|
directory: "/tmp/test",
|
||||||
|
worktree: "/tmp/test",
|
||||||
|
serverUrl: new URL("http://localhost"),
|
||||||
|
$: shell,
|
||||||
|
}
|
||||||
|
|
||||||
return { ctx, promptCalls }
|
return { ctx, promptCalls }
|
||||||
}
|
}
|
||||||
@@ -245,5 +300,35 @@ describe("gpt-permission-continuation", () => {
|
|||||||
expect(promptCalls).toEqual(["continue"])
|
expect(promptCalls).toEqual(["continue"])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#when a user manually types continue after the cap is reached", () => {
|
||||||
|
test("resets the cap and allows another auto-continue", async () => {
|
||||||
|
// given
|
||||||
|
const messages: SessionMessage[] = [
|
||||||
|
createUserMessage("msg-0", "Please continue the fix."),
|
||||||
|
createAssistantMessage("msg-1", "If you want, I can apply the patch next."),
|
||||||
|
]
|
||||||
|
const { ctx, promptCalls } = createMockPluginInput(messages)
|
||||||
|
const hook = createGptPermissionContinuationHook(ctx)
|
||||||
|
|
||||||
|
// when
|
||||||
|
await hook.handler({ event: { type: "session.idle", properties: { sessionID: "ses-1" } } })
|
||||||
|
messages.push(createUserMessage("msg-2", "continue"))
|
||||||
|
messages.push(createAssistantMessage("msg-3", "Would you like me to continue with the tests?"))
|
||||||
|
await hook.handler({ event: { type: "session.idle", properties: { sessionID: "ses-1" } } })
|
||||||
|
messages.push(createUserMessage("msg-4", "continue"))
|
||||||
|
messages.push(createAssistantMessage("msg-5", "Do you want me to wire the remaining cleanup?"))
|
||||||
|
await hook.handler({ event: { type: "session.idle", properties: { sessionID: "ses-1" } } })
|
||||||
|
messages.push(createUserMessage("msg-6", "continue"))
|
||||||
|
messages.push(createAssistantMessage("msg-7", "Shall I finish the remaining updates?"))
|
||||||
|
await hook.handler({ event: { type: "session.idle", properties: { sessionID: "ses-1" } } })
|
||||||
|
messages.push(createUserMessage("msg-8", "continue"))
|
||||||
|
messages.push(createAssistantMessage("msg-9", "If you want, I can apply the final polish."))
|
||||||
|
await hook.handler({ event: { type: "session.idle", properties: { sessionID: "ses-1" } } })
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptCalls).toEqual(["continue", "continue", "continue", "continue"])
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -67,6 +67,7 @@ function extractPermissionPhrase(text: string): string | null {
|
|||||||
|
|
||||||
function resetAutoContinuationState(state: SessionState): void {
|
function resetAutoContinuationState(state: SessionState): void {
|
||||||
state.consecutiveAutoContinueCount = 0
|
state.consecutiveAutoContinueCount = 0
|
||||||
|
state.awaitingAutoContinuationResponse = false
|
||||||
state.lastAutoContinuePermissionPhrase = undefined
|
state.lastAutoContinuePermissionPhrase = undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -117,8 +118,17 @@ export function createGptPermissionContinuationHandler(args: {
|
|||||||
|
|
||||||
const lastAssistantIndex = messages.lastIndexOf(lastAssistantMessage)
|
const lastAssistantIndex = messages.lastIndexOf(lastAssistantMessage)
|
||||||
const previousUserMessage = getLastUserMessageBefore(messages, lastAssistantIndex)
|
const previousUserMessage = getLastUserMessageBefore(messages, lastAssistantIndex)
|
||||||
if (previousUserMessage && !isAutoContinuationUserMessage(previousUserMessage)) {
|
const previousUserMessageWasAutoContinuation =
|
||||||
|
previousUserMessage !== null
|
||||||
|
&& state.awaitingAutoContinuationResponse
|
||||||
|
&& isAutoContinuationUserMessage(previousUserMessage)
|
||||||
|
|
||||||
|
if (previousUserMessageWasAutoContinuation) {
|
||||||
|
state.awaitingAutoContinuationResponse = false
|
||||||
|
} else if (previousUserMessage) {
|
||||||
resetAutoContinuationState(state)
|
resetAutoContinuationState(state)
|
||||||
|
} else {
|
||||||
|
state.awaitingAutoContinuationResponse = false
|
||||||
}
|
}
|
||||||
|
|
||||||
const messageID = lastAssistantMessage.info?.id
|
const messageID = lastAssistantMessage.info?.id
|
||||||
@@ -174,6 +184,7 @@ export function createGptPermissionContinuationHandler(args: {
|
|||||||
await promptContinuation(ctx, sessionID)
|
await promptContinuation(ctx, sessionID)
|
||||||
state.lastHandledMessageID = messageID
|
state.lastHandledMessageID = messageID
|
||||||
state.consecutiveAutoContinueCount += 1
|
state.consecutiveAutoContinueCount += 1
|
||||||
|
state.awaitingAutoContinuationResponse = true
|
||||||
state.lastAutoContinuePermissionPhrase = permissionPhrase
|
state.lastAutoContinuePermissionPhrase = permissionPhrase
|
||||||
state.lastInjectedAt = Date.now()
|
state.lastInjectedAt = Date.now()
|
||||||
log(`[${HOOK_NAME}] Injected continuation prompt`, { sessionID, messageID })
|
log(`[${HOOK_NAME}] Injected continuation prompt`, { sessionID, messageID })
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
type SessionState = {
|
type SessionState = {
|
||||||
inFlight: boolean
|
inFlight: boolean
|
||||||
consecutiveAutoContinueCount: number
|
consecutiveAutoContinueCount: number
|
||||||
|
awaitingAutoContinuationResponse: boolean
|
||||||
lastHandledMessageID?: string
|
lastHandledMessageID?: string
|
||||||
lastAutoContinuePermissionPhrase?: string
|
lastAutoContinuePermissionPhrase?: string
|
||||||
lastInjectedAt?: number
|
lastInjectedAt?: number
|
||||||
@@ -18,6 +19,7 @@ export function createSessionStateStore() {
|
|||||||
const created: SessionState = {
|
const created: SessionState = {
|
||||||
inFlight: false,
|
inFlight: false,
|
||||||
consecutiveAutoContinueCount: 0,
|
consecutiveAutoContinueCount: 0,
|
||||||
|
awaitingAutoContinuationResponse: false,
|
||||||
}
|
}
|
||||||
states.set(sessionID, created)
|
states.set(sessionID, created)
|
||||||
return created
|
return created
|
||||||
|
|||||||
@@ -13,6 +13,8 @@ import { applyAgentConfig } from "./agent-config-handler"
|
|||||||
import type { PluginComponents } from "./plugin-components-loader"
|
import type { PluginComponents } from "./plugin-components-loader"
|
||||||
|
|
||||||
const BUILTIN_SISYPHUS_DISPLAY_NAME = getAgentDisplayName("sisyphus")
|
const BUILTIN_SISYPHUS_DISPLAY_NAME = getAgentDisplayName("sisyphus")
|
||||||
|
const BUILTIN_SISYPHUS_JUNIOR_DISPLAY_NAME = getAgentDisplayName("sisyphus-junior")
|
||||||
|
const BUILTIN_MULTIMODAL_LOOKER_DISPLAY_NAME = getAgentDisplayName("multimodal-looker")
|
||||||
|
|
||||||
function createPluginComponents(): PluginComponents {
|
function createPluginComponents(): PluginComponents {
|
||||||
return {
|
return {
|
||||||
@@ -66,6 +68,12 @@ describe("applyAgentConfig builtin override protection", () => {
|
|||||||
mode: "subagent",
|
mode: "subagent",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const builtinMultimodalLookerConfig: AgentConfig = {
|
||||||
|
name: "multimodal-looker",
|
||||||
|
prompt: "multimodal prompt",
|
||||||
|
mode: "subagent",
|
||||||
|
}
|
||||||
|
|
||||||
const sisyphusJuniorConfig: AgentConfig = {
|
const sisyphusJuniorConfig: AgentConfig = {
|
||||||
name: "Sisyphus-Junior",
|
name: "Sisyphus-Junior",
|
||||||
prompt: "junior prompt",
|
prompt: "junior prompt",
|
||||||
@@ -76,6 +84,7 @@ describe("applyAgentConfig builtin override protection", () => {
|
|||||||
createBuiltinAgentsSpy = spyOn(agents, "createBuiltinAgents").mockResolvedValue({
|
createBuiltinAgentsSpy = spyOn(agents, "createBuiltinAgents").mockResolvedValue({
|
||||||
sisyphus: builtinSisyphusConfig,
|
sisyphus: builtinSisyphusConfig,
|
||||||
oracle: builtinOracleConfig,
|
oracle: builtinOracleConfig,
|
||||||
|
"multimodal-looker": builtinMultimodalLookerConfig,
|
||||||
})
|
})
|
||||||
|
|
||||||
createSisyphusJuniorAgentSpy = spyOn(
|
createSisyphusJuniorAgentSpy = spyOn(
|
||||||
@@ -194,4 +203,56 @@ describe("applyAgentConfig builtin override protection", () => {
|
|||||||
// then
|
// then
|
||||||
expect(result[BUILTIN_SISYPHUS_DISPLAY_NAME]).toEqual(builtinSisyphusConfig)
|
expect(result[BUILTIN_SISYPHUS_DISPLAY_NAME]).toEqual(builtinSisyphusConfig)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given protected builtin agents use hyphenated names", () => {
|
||||||
|
describe("#when a user agent uses the underscored multimodal looker alias", () => {
|
||||||
|
test("filters the override", async () => {
|
||||||
|
// given
|
||||||
|
loadUserAgentsSpy.mockReturnValue({
|
||||||
|
multimodal_looker: {
|
||||||
|
name: "multimodal_looker",
|
||||||
|
prompt: "user multimodal alias prompt",
|
||||||
|
mode: "subagent",
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = await applyAgentConfig({
|
||||||
|
config: createBaseConfig(),
|
||||||
|
pluginConfig: createPluginConfig(),
|
||||||
|
ctx: { directory: "/tmp" },
|
||||||
|
pluginComponents: createPluginComponents(),
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result[BUILTIN_MULTIMODAL_LOOKER_DISPLAY_NAME]).toEqual(builtinMultimodalLookerConfig)
|
||||||
|
expect(result.multimodal_looker).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("#when a user agent uses the underscored sisyphus junior alias", () => {
|
||||||
|
test("filters the override", async () => {
|
||||||
|
// given
|
||||||
|
loadUserAgentsSpy.mockReturnValue({
|
||||||
|
sisyphus_junior: {
|
||||||
|
name: "sisyphus_junior",
|
||||||
|
prompt: "user junior alias prompt",
|
||||||
|
mode: "subagent",
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
// when
|
||||||
|
const result = await applyAgentConfig({
|
||||||
|
config: createBaseConfig(),
|
||||||
|
pluginConfig: createPluginConfig(),
|
||||||
|
ctx: { directory: "/tmp" },
|
||||||
|
pluginComponents: createPluginComponents(),
|
||||||
|
})
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(result[BUILTIN_SISYPHUS_JUNIOR_DISPLAY_NAME]).toEqual(sisyphusJuniorConfig)
|
||||||
|
expect(result.sisyphus_junior).toBeUndefined()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1,7 +1,12 @@
|
|||||||
const PARENTHETICAL_SUFFIX_PATTERN = /\s*(\([^)]*\)\s*)+$/u
|
const PARENTHETICAL_SUFFIX_PATTERN = /\s*(\([^)]*\)\s*)+$/u
|
||||||
|
|
||||||
export function normalizeProtectedAgentName(agentName: string): string {
|
export function normalizeProtectedAgentName(agentName: string): string {
|
||||||
return agentName.trim().toLowerCase().replace(PARENTHETICAL_SUFFIX_PATTERN, "").trim()
|
return agentName
|
||||||
|
.trim()
|
||||||
|
.toLowerCase()
|
||||||
|
.replace(PARENTHETICAL_SUFFIX_PATTERN, "")
|
||||||
|
.replace(/[-_]/g, "")
|
||||||
|
.trim()
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createProtectedAgentNameSet(agentNames: Iterable<string>): Set<string> {
|
export function createProtectedAgentNameSet(agentNames: Iterable<string>): Set<string> {
|
||||||
|
|||||||
Reference in New Issue
Block a user