luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #4040 from code-yeongyu/cleanup/typescript-ai-slop-20260515

Browse Source 
Refactor TypeScript cleanup patterns
This commit is contained in:
YeonGyu-Kim
2026-05-15 16:42:10 +09:00
committed by GitHub
parent d8f52aae7f b6a0be562d
commit 462b55efc7
17 changed files with 81 additions and 186 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/cli/doctor/checks/dependencies.ts
+6 -2
View File
@@ -6,7 +6,11 @@ import type { DependencyInfo } from "../types"
import { spawnWithTimeout } from "../spawn-with-timeout"
import { getCachedBinaryPath } from "../../../hooks/comment-checker/downloader"
async function checkBinaryExists(binary: string): Promise<{ exists: boolean; path: string | null }> {
type BinaryCheck =
| { exists: true; path: string }
| { exists: false; path: null }
async function checkBinaryExists(binary: string): Promise<BinaryCheck> {
try {
const path = Bun.which(binary)
if (path) {
@@ -44,7 +48,7 @@ export async function checkAstGrepCli(): Promise<DependencyInfo> {
}
}
const version = await getBinaryVersion(binary.path!)
const version = await getBinaryVersion(binary.path)
return {
name: "AST-Grep CLI",
src/cli/run/poll-for-completion.ts
+16 -12
View File
@@ -2,7 +2,7 @@ import pc from "picocolors"
import type { RunContext } from "./types"
import type { EventState } from "./events"
import { checkCompletionConditions } from "./completion"
import { normalizeSDKResponse } from "../../shared"
import { isRecord, normalizeSDKResponse } from "../../shared"
const DEFAULT_POLL_INTERVAL_MS = 500
const DEFAULT_REQUIRED_CONSECUTIVE = 1
@@ -11,6 +11,17 @@ const MIN_STABILIZATION_MS = 1_000
const DEFAULT_EVENT_WATCHDOG_MS = 30_000 // 30 seconds
const DEFAULT_SECONDARY_MEANINGFUL_WORK_TIMEOUT_MS = 60_000 // 60 seconds
type SessionStatusMap = Record<string, { type?: string }>
function isIncompleteTodo(value: unknown): boolean {
if (!isRecord(value)) {
return true
}
const status = value.status
return status !== "completed" && status !== "cancelled"
}
export interface PollOptions {
pollIntervalMs?: number
requiredConsecutive?: number
@@ -123,22 +134,18 @@ export async function pollForCompletion(
path: { id: ctx.sessionID },
query: { directory: ctx.directory },
})
const children = normalizeSDKResponse(childrenRes, [] as unknown[])
const children = normalizeSDKResponse<unknown[]>(childrenRes, [])
const todosRes = await ctx.client.session.todo({
path: { id: ctx.sessionID },
query: { directory: ctx.directory },
})
const todos = normalizeSDKResponse(todosRes, [] as unknown[])
const todos = normalizeSDKResponse<unknown[]>(todosRes, [])
const hasActiveChildren =
Array.isArray(children) && children.length > 0
const hasActiveTodos =
Array.isArray(todos) &&
todos.some(
(t: unknown) =>
(t as { status?: string })?.status !== "completed" &&
(t as { status?: string })?.status !== "cancelled"
)
todos.some(isIncompleteTodo)
const hasActiveWork = hasActiveChildren || hasActiveTodos
if (hasActiveWork) {
@@ -189,10 +196,7 @@ async function getMainSessionStatus(
const statusesRes = await ctx.client.session.status({
query: { directory: ctx.directory },
})
const statuses = normalizeSDKResponse(
statusesRes,
{} as Record<string, { type?: string }>
)
const statuses = normalizeSDKResponse<SessionStatusMap>(statusesRes, {})
if (!(ctx.sessionID in statuses)) {
return "idle"
}
src/features/team-mode/team-runtime/delete-team.ts
+5 -2
View File
@@ -103,8 +103,11 @@ export async function deleteTeam(
const removedLayout = config.tmux_visualization && tmuxMgr !== undefined && deps.canVisualize()
if (removedLayout) {
const memberPaneIds = runtimeState.members
.filter((member) => member.agentType !== "leader" && member.tmuxPaneId)
.map((member) => member.tmuxPaneId!)
.flatMap((member) => (
member.agentType !== "leader" && member.tmuxPaneId
? [member.tmuxPaneId]
: []
))
const cleanupTarget = runtimeState.tmuxLayout
? {
src/hooks/interactive-bash-session/hook.ts
+4 -7
View File
@@ -2,7 +2,7 @@ import type { PluginInput } from "@opencode-ai/plugin";
import { saveInteractiveBashSessionState, clearInteractiveBashSessionState } from "./storage";
import { buildSessionReminderMessage } from "./constants";
import type { InteractiveBashSessionState } from "./types";
import { tokenizeCommand, findSubcommand, extractSessionNameFromTokens } from "./parser";
import { parseTmuxCommand } from "./tmux-command-parser";
import { getOrCreateState, isOmoSession, killAllTrackedSessions } from "./state-manager";
import { subagentSessions } from "../../features/claude-code-session-state";
import { resolveSessionEventID } from "../../shared/event-session-id";
@@ -60,8 +60,7 @@ export function createInteractiveBashSessionHook(ctx: PluginInput) {
}
const tmuxCommand = args.tmux_command;
const tokens = tokenizeCommand(tmuxCommand);
const subCommand = findSubcommand(tokens);
const { subCommand, sessionName } = parseTmuxCommand(tmuxCommand);
const state = getOrCreateStateLocal(sessionID);
let stateChanged = false;
@@ -74,13 +73,11 @@ export function createInteractiveBashSessionHook(ctx: PluginInput) {
const isKillSession = subCommand === "kill-session";
const isKillServer = subCommand === "kill-server";
const sessionName = extractSessionNameFromTokens(tokens, subCommand);
if (isNewSession && isOmoSession(sessionName)) {
state.tmuxSessions.add(sessionName!);
state.tmuxSessions.add(sessionName);
stateChanged = true;
} else if (isKillSession && isOmoSession(sessionName)) {
state.tmuxSessions.delete(sessionName!);
state.tmuxSessions.delete(sessionName);
stateChanged = true;
} else if (isKillServer) {
state.tmuxSessions.clear();
src/hooks/interactive-bash-session/parser.ts
-118
View File
@@ -1,118 +0,0 @@
/**
* Quote-aware command tokenizer with escape handling
* Handles single/double quotes and backslash escapes
*/
export function tokenizeCommand(cmd: string): string[] {
const tokens: string[] = []
let current = ""
let inQuote = false
let quoteChar = ""
let escaped = false
for (let i = 0; i < cmd.length; i++) {
const char = cmd[i]
if (escaped) {
current += char
escaped = false
continue
}
if (char === "\\") {
escaped = true
continue
}
if ((char === "'" || char === '"') && !inQuote) {
inQuote = true
quoteChar = char
} else if (char === quoteChar && inQuote) {
inQuote = false
quoteChar = ""
} else if (char === " " && !inQuote) {
if (current) {
tokens.push(current)
current = ""
}
} else {
current += char
}
}
if (current) tokens.push(current)
return tokens
}
/**
* Normalize session name by stripping :window and .pane suffixes
* e.g., "omo-x:1" -> "omo-x", "omo-x:1.2" -> "omo-x"
*/
export function normalizeSessionName(name: string): string {
return name.split(":")[0].split(".")[0]
}
export function findFlagValue(tokens: string[], flag: string): string | null {
for (let i = 0; i < tokens.length - 1; i++) {
if (tokens[i] === flag) return tokens[i + 1]
}
return null
}
/**
* Extract session name from tokens, considering the subCommand
* For new-session: prioritize -s over -t
* For other commands: use -t
*/
export function extractSessionNameFromTokens(tokens: string[], subCommand: string): string | null {
if (subCommand === "new-session") {
const sFlag = findFlagValue(tokens, "-s")
if (sFlag) return normalizeSessionName(sFlag)
const tFlag = findFlagValue(tokens, "-t")
if (tFlag) return normalizeSessionName(tFlag)
} else {
const tFlag = findFlagValue(tokens, "-t")
if (tFlag) return normalizeSessionName(tFlag)
}
return null
}
/**
* Find the tmux subcommand from tokens, skipping global options.
* tmux allows global options before the subcommand:
* e.g., `tmux -L socket-name new-session -s omo-x`
* Global options with args: -L, -S, -f, -c, -T
* Standalone flags: -C, -v, -V, etc.
* Special: -- (end of options marker)
*/
export function findSubcommand(tokens: string[]): string {
// Options that require an argument: -L, -S, -f, -c, -T
const globalOptionsWithArgs = new Set(["-L", "-S", "-f", "-c", "-T"])
let i = 0
while (i < tokens.length) {
const token = tokens[i]
// Handle end of options marker
if (token === "--") {
// Next token is the subcommand
return tokens[i + 1] ?? ""
}
if (globalOptionsWithArgs.has(token)) {
// Skip the option and its argument
i += 2
continue
}
if (token.startsWith("-")) {
// Skip standalone flags like -C, -v, -V
i++
continue
}
// Found the subcommand
return token
}
return ""
}
src/hooks/interactive-bash-session/state-manager.ts
+20 -11
View File
@@ -2,21 +2,25 @@ import type { InteractiveBashSessionState } from "./types";
import { loadInteractiveBashSessionState } from "./storage";
import { OMO_SESSION_PREFIX } from "./constants";
import { spawnWithWindowsHide } from "../../shared/spawn-with-windows-hide";
import { log } from "../../shared/logger";
export function getOrCreateState(sessionID: string, sessionStates: Map<string, InteractiveBashSessionState>): InteractiveBashSessionState {
if (!sessionStates.has(sessionID)) {
const persisted = loadInteractiveBashSessionState(sessionID);
const state: InteractiveBashSessionState = persisted ?? {
sessionID,
tmuxSessions: new Set<string>(),
updatedAt: Date.now(),
};
sessionStates.set(sessionID, state);
const existing = sessionStates.get(sessionID);
if (existing) {
return existing;
}
return sessionStates.get(sessionID)!;
const persisted = loadInteractiveBashSessionState(sessionID);
const state: InteractiveBashSessionState = persisted ?? {
sessionID,
tmuxSessions: new Set<string>(),
updatedAt: Date.now(),
};
sessionStates.set(sessionID, state);
return state;
}
export function isOmoSession(sessionName: string | null): boolean {
export function isOmoSession(sessionName: string | null): sessionName is string {
return sessionName !== null && sessionName.startsWith(OMO_SESSION_PREFIX);
}
@@ -30,6 +34,11 @@ export async function killAllTrackedSessions(
stderr: "ignore",
});
await proc.exited;
} catch {}
} catch (error) {
log("[interactive-bash-session] failed to kill tracked tmux session", {
error: error instanceof Error ? error.message : String(error),
sessionName,
});
}
}
}
src/hooks/read-image-resizer/hook.ts
+2 -2
View File
@@ -189,8 +189,8 @@ export function createReadImageResizerHook(_ctx: PluginInput) {
}
}
if (attachmentsToRemove.length > 0) {
const rawAttachments = outputRecord.attachments as unknown[]
if (attachmentsToRemove.length > 0 && Array.isArray(outputRecord.attachments)) {
const rawAttachments = outputRecord.attachments
for (const toRemove of attachmentsToRemove) {
const removeIndex = rawAttachments.indexOf(toRemove)
if (removeIndex !== -1) {
src/hooks/session-recovery/storage/messages-reader.ts
+1 -1
View File
@@ -62,7 +62,7 @@ export async function readMessagesFromSDK(
): Promise<StoredMessageMeta[]> {
try {
const response = await client.session.messages({ path: { id: sessionID } })
const data = normalizeSDKResponse(response, [] as unknown[], {
const data = normalizeSDKResponse<unknown[]>(response, [], {
preferResponseOnMissingData: true,
})
if (!Array.isArray(data)) return []
src/hooks/todo-continuation-enforcer/todo-continuation-enforcer.test.ts
-11
View File
@@ -1190,16 +1190,9 @@ describe("todo-continuation-enforcer", () => {
// then - continuation injected (non-abort errors don't block)
expect(promptCalls.length).toBe(1)
}, { timeout: 15000 })
// ============================================================
// API-BASED ABORT DETECTION TESTS
// These tests verify that abort is detected by checking
// the last assistant message's error field via session.messages API
// ============================================================
test("should skip injection when last assistant message has MessageAbortedError", async () => {
// given - session where last assistant message was aborted
@@ -1673,11 +1666,9 @@ describe("todo-continuation-enforcer", () => {
expect(promptCalls[0].model).toEqual({ providerID: "openai", modelID: "gpt-5.4" })
})
// ============================================================
// COMPACTION AGENT FILTERING TESTS
// These tests verify that compaction agent messages are filtered
// when resolving agent info, preventing infinite continuation loops
// ============================================================
test("should skip injection while the latest message is from the compaction agent", async () => {
// given - session where the latest activity is still the compaction assistant turn
@@ -2102,11 +2093,9 @@ describe("todo-continuation-enforcer", () => {
expect(promptCalls).toHaveLength(1)
}, { timeout: 20000 })
// ============================================================
// TOKEN-LIMIT ERROR DETECTION TESTS (#2462)
// These tests verify that the enforcer does NOT retry continuation
// when the model returns a token-limit / context-length error.
// ============================================================
test("should stop continuation when session.error carries a ContextLengthError", async () => {
// given - session with incomplete todos
src/plugin-handlers/command-config-handler.ts
+2 -2
View File
@@ -43,8 +43,8 @@ export async function applyCommandConfig(params: {
const includeClaudeSkills = params.pluginConfig.claude_code?.skills ?? true;
const externalSkillPlugin = detectExternalSkillPlugin(params.ctx.directory);
if (includeClaudeSkills && externalSkillPlugin.detected) {
log(getSkillPluginConflictWarning(externalSkillPlugin.pluginName!));
if (includeClaudeSkills && externalSkillPlugin.detected && externalSkillPlugin.pluginName) {
log(getSkillPluginConflictWarning(externalSkillPlugin.pluginName));
}
const [
src/plugin/hooks/create-session-hooks.ts
+2 -2
View File
@@ -101,8 +101,8 @@ export function createSessionHooks(args: {
if (isHookEnabled("session-notification")) {
const forceEnable = pluginConfig.notification?.force_enable ?? false
const externalNotifier = detectExternalNotificationPlugin(ctx.directory)
if (externalNotifier.detected && !forceEnable) {
log(getNotificationConflictWarning(externalNotifier.pluginName!))
if (externalNotifier.detected && externalNotifier.pluginName && !forceEnable) {
log(getNotificationConflictWarning(externalNotifier.pluginName))
} else {
sessionNotification = safeHook("session-notification", () => createSessionNotification(ctx))
}
src/shared/log-legacy-plugin-startup-warning.ts
+2 -2
View File
@@ -16,7 +16,7 @@ export function logLegacyPluginStartupWarning(deps: LogLegacyPluginStartupWarnin
const migrateLegacyPluginEntryFn = deps.migrateLegacyPluginEntry ?? migrateLegacyPluginEntry
const result = checkForLegacyPluginEntryFn()
if (!result.hasLegacyEntry) {
if (!result.hasLegacyEntry || !result.configPath) {
return
}
@@ -34,7 +34,7 @@ export function logLegacyPluginStartupWarning(deps: LogLegacyPluginStartupWarnin
+ ` Attempting auto-migration...`,
)
const migrated = migrateLegacyPluginEntryFn(result.configPath!)
const migrated = migrateLegacyPluginEntryFn(result.configPath)
if (migrated) {
console.warn(`[oh-my-openagent] Auto-migrated opencode.json: ${result.legacyEntries.join(", ")} -> ${suggestedEntries.join(", ")}`)
} else {
src/tools/background-task/create-background-cancel.ts
+7 -6
View File
@@ -15,10 +15,6 @@ export function createBackgroundCancel(manager: BackgroundManager, _client: Back
try {
const cancelAll = args.all === true
if (!cancelAll && !args.taskId) {
return `[ERROR] Invalid arguments: Either provide a taskId or set all=true to cancel all running tasks.`
}
if (cancelAll) {
const tasks = manager.getAllDescendantTasks(toolContext.sessionID)
const cancellableTasks = tasks.filter((t: { status: string }) => t.status === "running" || t.status === "pending")
@@ -74,9 +70,14 @@ ${tableRows}
${resumeSection}`
}
const task = manager.getTask(args.taskId!)
const taskId = args.taskId
if (!taskId) {
return `[ERROR] Invalid arguments: Either provide a taskId or set all=true to cancel all running tasks.`
}
const task = manager.getTask(taskId)
if (!task) {
return `[ERROR] Task not found: ${args.taskId}`
return `[ERROR] Task not found: ${taskId}`
}
if (task.status !== "running" && task.status !== "pending") {
src/tools/delegate-task/tools.test.ts
-1
View File
@@ -3518,7 +3518,6 @@ describe("sisyphus-task", () => {
expect(actualModel).not.toBe(inheritedModel)
})
// ===== TESTS FOR resolveModel() INTEGRATION (TDD GREEN) =====
// These tests verify the NEW behavior where categories do NOT have default models
test("FIXED: category built-in model takes precedence over inheritedModel", () => {
src/tools/look-at/look-at-arguments.ts
+3 -2
View File
@@ -13,10 +13,11 @@ export function normalizeArgs(args: LookAtArgsWithAlias): LookAtArgs {
}
export function validateArgs(args: LookAtArgs): string | null {
const hasFilePath = Boolean(args.file_path && args.file_path.length > 0)
const filePath = args.file_path
const hasFilePath = Boolean(filePath && filePath.length > 0)
const hasImageData = Boolean(args.image_data && args.image_data.length > 0)
if (hasFilePath && /^https?:\/\//i.test(args.file_path!)) {
if (filePath && /^https?:\/\//i.test(filePath)) {
return "Error: Remote URLs are not supported for file_path. Download the file first or use a local path."
}
if (!hasFilePath && !hasImageData) {
src/tools/lsp/lsp-client-transport.ts
+8 -3
View File
@@ -136,22 +136,27 @@ export class LSPClientTransport {
throw new Error(`LSP server already exited (code: ${this.proc?.exitCode})` + (stderr ? `\nstderr: ${stderr}` : ""))
}
let timeoutId: ReturnType<typeof setTimeout>
let timeoutId: ReturnType<typeof setTimeout> | undefined
const timeoutPromise = new Promise<never>((_, reject) => {
timeoutId = setTimeout(() => {
const stderr = this.stderrBuffer.slice(-5).join("\n")
reject(new Error(`LSP request timeout (method: ${method})` + (stderr ? `\nrecent stderr: ${stderr}` : "")))
}, this.REQUEST_TIMEOUT)
})
const clearRequestTimeout = (): void => {
if (timeoutId !== undefined) {
clearTimeout(timeoutId)
}
}
const requestPromise = this.connection.sendRequest(method, ...args) as Promise<T>
try {
const result = await Promise.race([requestPromise, timeoutPromise])
clearTimeout(timeoutId!)
clearRequestTimeout()
return result
} catch (error) {
clearTimeout(timeoutId!)
clearRequestTimeout()
throw error
}
}
src/tools/lsp/symbols-tool.ts
+3 -2
View File
@@ -22,12 +22,13 @@ export const lsp_symbols: ToolDefinition = tool({
const scope = args.scope ?? "document"
if (scope === "workspace") {
if (!args.query) {
const query = args.query
if (!query) {
return "Error: 'query' is required for workspace scope"
}
const result = await withLspClient(args.filePath, async (client) => {
return (await client.workspaceSymbols(args.query!)) as SymbolInfo[] | null
return (await client.workspaceSymbols(query)) as SymbolInfo[] | null
})
if (!result || result.length === 0) {