refactor(hooks): fix empty catches and remove AI slop from code comments

2026-04-03 21:37:16 +09:00
parent 6b431c5522
commit 5c7299830d
17 changed files with 900 additions and 565 deletions
@@ -4,8 +4,10 @@ import { existsSync, realpathSync } from "fs"
 import { basename, dirname, isAbsolute, join, normalize, relative, resolve } from "path"

 import { log } from "../../shared"
+import { handleWriteExistingFileGuardToolExecuteBefore } from "./tool-execute-before-handler"
+import { evictLeastRecentlyUsedSession, touchSession, trimSessionReadSet } from "./session-read-permissions"

-type GuardArgs = {
+export type GuardArgs = {
  filePath?: string
  path?: string
  file_path?: string
@@ -16,7 +18,7 @@ const MAX_TRACKED_SESSIONS = 256
 export const MAX_TRACKED_PATHS_PER_SESSION = 1024
 const BLOCK_MESSAGE = "File already exists. Use edit tool instead."

-function asRecord(value: unknown): Record<string, unknown> | undefined {
+export function asRecord(value: unknown): Record<string, unknown> | undefined {
  if (!value || typeof value !== "object" || Array.isArray(value)) {
    return undefined
  }
@@ -24,22 +26,22 @@ function asRecord(value: unknown): Record<string, unknown> | undefined {
  return value as Record<string, unknown>
 }

-function getPathFromArgs(args: GuardArgs | undefined): string | undefined {
+export function getPathFromArgs(args: GuardArgs | undefined): string | undefined {
  return args?.filePath ?? args?.path ?? args?.file_path
 }

-function resolveInputPath(ctx: PluginInput, inputPath: string): string {
+export function resolveInputPath(ctx: PluginInput, inputPath: string): string {
  return normalize(isAbsolute(inputPath) ? inputPath : resolve(ctx.directory, inputPath))
 }

-function isPathInsideDirectory(pathToCheck: string, directory: string): boolean {
+export function isPathInsideDirectory(pathToCheck: string, directory: string): boolean {
  const relativePath = relative(directory, pathToCheck)
  return relativePath === "" || (!relativePath.startsWith("..") && !isAbsolute(relativePath))
 }



-function toCanonicalPath(absolutePath: string): string {
+export function toCanonicalPath(absolutePath: string): string {
  let canonicalPath = absolutePath

  if (existsSync(absolutePath)) {
@@ -59,7 +61,7 @@ function toCanonicalPath(absolutePath: string): string {
  return normalize(canonicalPath)
 }

-function isOverwriteEnabled(value: boolean | string | undefined): boolean {
+export function isOverwriteEnabled(value: boolean | string | undefined): boolean {
  if (value === true) {
    return true
  }
@@ -76,165 +78,17 @@ export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
  const sessionLastAccess = new Map<string, number>()
  const canonicalSessionRoot = toCanonicalPath(resolveInputPath(ctx, ctx.directory))

-  const touchSession = (sessionID: string): void => {
-    sessionLastAccess.set(sessionID, Date.now())
-  }
-
-  const evictLeastRecentlyUsedSession = (): void => {
-    let oldestSessionID: string | undefined
-    let oldestSeen = Number.POSITIVE_INFINITY
-
-    for (const [sessionID, lastSeen] of sessionLastAccess.entries()) {
-      if (lastSeen < oldestSeen) {
-        oldestSeen = lastSeen
-        oldestSessionID = sessionID
-      }
-    }
-
-    if (!oldestSessionID) {
-      return
-    }
-
-    readPermissionsBySession.delete(oldestSessionID)
-    sessionLastAccess.delete(oldestSessionID)
-  }
-
-  const ensureSessionReadSet = (sessionID: string): Set<string> => {
-    let readSet = readPermissionsBySession.get(sessionID)
-    if (!readSet) {
-      if (readPermissionsBySession.size >= MAX_TRACKED_SESSIONS) {
-        evictLeastRecentlyUsedSession()
-      }
-
-      readSet = new Set<string>()
-      readPermissionsBySession.set(sessionID, readSet)
-    }
-
-    touchSession(sessionID)
-    return readSet
-  }
-
-  const trimSessionReadSet = (readSet: Set<string>): void => {
-    while (readSet.size > MAX_TRACKED_PATHS_PER_SESSION) {
-      const oldestPath = readSet.values().next().value
-      if (!oldestPath) {
-        return
-      }
-
-      readSet.delete(oldestPath)
-    }
-  }
-
-  const registerReadPermission = (sessionID: string, canonicalPath: string): void => {
-    const readSet = ensureSessionReadSet(sessionID)
-    if (readSet.has(canonicalPath)) {
-      readSet.delete(canonicalPath)
-    }
-
-    readSet.add(canonicalPath)
-    trimSessionReadSet(readSet)
-  }
-
-  const consumeReadPermission = (sessionID: string, canonicalPath: string): boolean => {
-    const readSet = readPermissionsBySession.get(sessionID)
-    if (!readSet || !readSet.has(canonicalPath)) {
-      return false
-    }
-
-    readSet.delete(canonicalPath)
-    touchSession(sessionID)
-    return true
-  }
-
-  const invalidateOtherSessions = (canonicalPath: string, writingSessionID?: string): void => {
-    for (const [sessionID, readSet] of readPermissionsBySession.entries()) {
-      if (writingSessionID && sessionID === writingSessionID) {
-        continue
-      }
-
-      readSet.delete(canonicalPath)
-    }
-  }
-
  return {
    "tool.execute.before": async (input, output) => {
-      const toolName = input.tool?.toLowerCase()
-      if (toolName !== "write" && toolName !== "read") {
-        return
-      }
-
-      const argsRecord = asRecord(output.args)
-      const args = argsRecord as GuardArgs | undefined
-      const filePath = getPathFromArgs(args)
-      if (!filePath) {
-        return
-      }
-
-      const resolvedPath = resolveInputPath(ctx, filePath)
-      const canonicalPath = toCanonicalPath(resolvedPath)
-      const isInsideSessionDirectory = isPathInsideDirectory(canonicalPath, canonicalSessionRoot)
-
-      if (!isInsideSessionDirectory) {
-        return
-      }
-
-      if (toolName === "read") {
-        if (!existsSync(resolvedPath) || !input.sessionID) {
-          return
-        }
-
-        registerReadPermission(input.sessionID, canonicalPath)
-        return
-      }
-
-      const overwriteEnabled = isOverwriteEnabled(args?.overwrite)
-
-      if (argsRecord && "overwrite" in argsRecord) {
-        // Intentionally mutate output args so overwrite bypass remains hook-only.
-        delete argsRecord.overwrite
-      }
-
-      if (!existsSync(resolvedPath)) {
-        return
-      }
-
-      const isSisyphusPath = canonicalPath.includes("/.sisyphus/")
-      if (isSisyphusPath) {
-        log("[write-existing-file-guard] Allowing .sisyphus/** overwrite", {
-          sessionID: input.sessionID,
-          filePath,
-        })
-        invalidateOtherSessions(canonicalPath, input.sessionID)
-        return
-      }
-
-      if (overwriteEnabled) {
-        log("[write-existing-file-guard] Allowing overwrite flag bypass", {
-          sessionID: input.sessionID,
-          filePath,
-          resolvedPath,
-        })
-        invalidateOtherSessions(canonicalPath, input.sessionID)
-        return
-      }
-
-      if (input.sessionID && consumeReadPermission(input.sessionID, canonicalPath)) {
-        log("[write-existing-file-guard] Allowing overwrite after read", {
-          sessionID: input.sessionID,
-          filePath,
-          resolvedPath,
-        })
-        invalidateOtherSessions(canonicalPath, input.sessionID)
-        return
-      }
-
-      log("[write-existing-file-guard] Blocking write to existing file", {
-        sessionID: input.sessionID,
-        filePath,
-        resolvedPath,
+      await handleWriteExistingFileGuardToolExecuteBefore({
+        ctx,
+        input,
+        output,
+        readPermissionsBySession,
+        sessionLastAccess,
+        canonicalSessionRoot,
+        maxTrackedSessions: MAX_TRACKED_SESSIONS,
      })
-
-      throw new Error("File already exists. Use edit tool instead.")
    },
    event: async ({ event }: { event: { type: string; properties?: unknown } }) => {
      if (event.type !== "session.deleted") {
@@ -0,0 +1,36 @@
+export function touchSession(sessionLastAccess: Map<string, number>, sessionID: string): void {
+  sessionLastAccess.set(sessionID, Date.now())
+}
+
+export function evictLeastRecentlyUsedSession(
+  readPermissionsBySession: Map<string, Set<string>>,
+  sessionLastAccess: Map<string, number>,
+): void {
+  let oldestSessionID: string | undefined
+  let oldestSeen = Number.POSITIVE_INFINITY
+
+  for (const [sessionID, lastSeen] of sessionLastAccess.entries()) {
+    if (lastSeen < oldestSeen) {
+      oldestSeen = lastSeen
+      oldestSessionID = sessionID
+    }
+  }
+
+  if (!oldestSessionID) {
+    return
+  }
+
+  readPermissionsBySession.delete(oldestSessionID)
+  sessionLastAccess.delete(oldestSessionID)
+}
+
+export function trimSessionReadSet(readSet: Set<string>, maxTrackedPathsPerSession: number): void {
+  while (readSet.size > maxTrackedPathsPerSession) {
+    const oldestPath = readSet.values().next().value
+    if (!oldestPath) {
+      return
+    }
+
+    readSet.delete(oldestPath)
+  }
+}
@@ -0,0 +1,176 @@
+import type { PluginInput } from "@opencode-ai/plugin"
+import { existsSync } from "fs"
+import { log } from "../../shared"
+import { MAX_TRACKED_PATHS_PER_SESSION } from "./hook"
+import {
+  asRecord,
+  getPathFromArgs,
+  isOverwriteEnabled,
+  isPathInsideDirectory,
+  resolveInputPath,
+  toCanonicalPath,
+  type GuardArgs,
+} from "./hook"
+import {
+  evictLeastRecentlyUsedSession,
+  touchSession,
+  trimSessionReadSet,
+} from "./session-read-permissions"
+
+function ensureSessionReadSet(params: {
+  sessionID: string
+  readPermissionsBySession: Map<string, Set<string>>
+  sessionLastAccess: Map<string, number>
+  maxTrackedSessions: number
+}): Set<string> {
+  const { sessionID, readPermissionsBySession, sessionLastAccess, maxTrackedSessions } = params
+  let readSet = readPermissionsBySession.get(sessionID)
+  if (!readSet) {
+    if (readPermissionsBySession.size >= maxTrackedSessions) {
+      evictLeastRecentlyUsedSession(readPermissionsBySession, sessionLastAccess)
+    }
+
+    readSet = new Set<string>()
+    readPermissionsBySession.set(sessionID, readSet)
+  }
+
+  touchSession(sessionLastAccess, sessionID)
+  return readSet
+}
+
+function registerReadPermission(params: {
+  sessionID: string
+  canonicalPath: string
+  readPermissionsBySession: Map<string, Set<string>>
+  sessionLastAccess: Map<string, number>
+  maxTrackedSessions: number
+}): void {
+  const readSet = ensureSessionReadSet(params)
+  if (readSet.has(params.canonicalPath)) {
+    readSet.delete(params.canonicalPath)
+  }
+
+  readSet.add(params.canonicalPath)
+  trimSessionReadSet(readSet, MAX_TRACKED_PATHS_PER_SESSION)
+}
+
+function consumeReadPermission(params: {
+  sessionID: string
+  canonicalPath: string
+  readPermissionsBySession: Map<string, Set<string>>
+  sessionLastAccess: Map<string, number>
+}): boolean {
+  const readSet = params.readPermissionsBySession.get(params.sessionID)
+  if (!readSet || !readSet.has(params.canonicalPath)) {
+    return false
+  }
+
+  readSet.delete(params.canonicalPath)
+  touchSession(params.sessionLastAccess, params.sessionID)
+  return true
+}
+
+function invalidateOtherSessions(
+  readPermissionsBySession: Map<string, Set<string>>,
+  canonicalPath: string,
+  writingSessionID?: string,
+): void {
+  for (const [sessionID, readSet] of readPermissionsBySession.entries()) {
+    if (writingSessionID && sessionID === writingSessionID) {
+      continue
+    }
+
+    readSet.delete(canonicalPath)
+  }
+}
+
+export async function handleWriteExistingFileGuardToolExecuteBefore(params: {
+  ctx: PluginInput
+  input: { tool?: string; sessionID?: string }
+  output: { args?: unknown }
+  readPermissionsBySession: Map<string, Set<string>>
+  sessionLastAccess: Map<string, number>
+  canonicalSessionRoot: string
+  maxTrackedSessions: number
+}): Promise<void> {
+  const { ctx, input, output, readPermissionsBySession, sessionLastAccess, canonicalSessionRoot, maxTrackedSessions } = params
+  const toolName = input.tool?.toLowerCase()
+  if (toolName !== "write" && toolName !== "read") {
+    return
+  }
+
+  const argsRecord = asRecord(output.args)
+  const args = argsRecord as GuardArgs | undefined
+  const filePath = getPathFromArgs(args)
+  if (!filePath) {
+    return
+  }
+
+  const resolvedPath = resolveInputPath(ctx, filePath)
+  const canonicalPath = toCanonicalPath(resolvedPath)
+  if (!isPathInsideDirectory(canonicalPath, canonicalSessionRoot)) {
+    return
+  }
+
+  if (toolName === "read") {
+    if (!existsSync(resolvedPath) || !input.sessionID) {
+      return
+    }
+
+    registerReadPermission({
+      sessionID: input.sessionID,
+      canonicalPath,
+      readPermissionsBySession,
+      sessionLastAccess,
+      maxTrackedSessions,
+    })
+    return
+  }
+
+  const overwriteEnabled = isOverwriteEnabled(args?.overwrite)
+  if (argsRecord && "overwrite" in argsRecord) {
+    delete argsRecord.overwrite
+  }
+
+  if (!existsSync(resolvedPath)) {
+    return
+  }
+
+  const isSisyphusPath = canonicalPath.includes("/.sisyphus/")
+  if (isSisyphusPath) {
+    log("[write-existing-file-guard] Allowing .sisyphus/** overwrite", {
+      sessionID: input.sessionID,
+      filePath,
+    })
+    invalidateOtherSessions(readPermissionsBySession, canonicalPath, input.sessionID)
+    return
+  }
+
+  if (overwriteEnabled) {
+    log("[write-existing-file-guard] Allowing overwrite flag bypass", {
+      sessionID: input.sessionID,
+      filePath,
+      resolvedPath,
+    })
+    invalidateOtherSessions(readPermissionsBySession, canonicalPath, input.sessionID)
+    return
+  }
+
+  if (input.sessionID && consumeReadPermission({ sessionID: input.sessionID, canonicalPath, readPermissionsBySession, sessionLastAccess })) {
+    log("[write-existing-file-guard] Allowing overwrite after read", {
+      sessionID: input.sessionID,
+      filePath,
+      resolvedPath,
+    })
+    invalidateOtherSessions(readPermissionsBySession, canonicalPath, input.sessionID)
+    return
+  }
+
+  log("[write-existing-file-guard] Blocking write to existing file", {
+    sessionID: input.sessionID,
+    filePath,
+    resolvedPath,
+  })
+
+  throw new Error("File already exists. Use edit tool instead.")
+}