From 54f984d58c4983238c40de1618c13128f5b2cfac Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 15:25:48 +0000 Subject: [PATCH 01/31] fix(tests): eliminate flaky concurrent-writes test by awaiting write promises The "serializes concurrent writes" test was flaky in CI because it relied on a 100ms timeout to drain the write queue. On slow CI runners, 3 sequential atomic writes (mkdir + writeFile + rename) could exceed 100ms, causing the test to read intermediate state. Changes: - writeActiveRulesState now returns Promise so tests can await the exact completion of chained writes instead of polling + sleeping - _resetWriteQueues helper added and called in afterEach to keep the module-level Map clean between tests - All fire-and-forget patterns in active-rules-state.test.ts replaced with deterministic awaits; waitForFile helper removed --- src/active-rules-state.test.ts | 58 ++++++++-------------------------- src/active-rules-state.ts | 13 ++++++-- 2 files changed, 23 insertions(+), 48 deletions(-) diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index 86de4fe..46336ba 100644 --- a/src/active-rules-state.test.ts +++ b/src/active-rules-state.test.ts @@ -8,6 +8,7 @@ import { writeActiveRulesState, readActiveRulesState, _setStateDirForTesting, + _resetWriteQueues, } from './active-rules-state.js'; describe('active-rules-state', () => { @@ -27,6 +28,7 @@ describe('active-rules-state', () => { afterEach(async () => { // Reset the override _setStateDirForTesting(null); + _resetWriteQueues(); // Clean up test directory if (testStateDir) { @@ -79,10 +81,7 @@ describe('active-rules-state', () => { const sessionId = 'ses_roundtrip'; const matchedPaths = ['/path/to/rule1.md', '/path/to/rule2.md']; - writeActiveRulesState(sessionId, matchedPaths); - - // Wait for the fire-and-forget write to complete - await waitForFile(getStateFilePath(sessionId)); + await writeActiveRulesState(sessionId, matchedPaths); const state = await readActiveRulesState(sessionId); @@ -155,11 +154,8 @@ describe('active-rules-state', () => { }); it('silently ignores write with invalid sessionId', async () => { - writeActiveRulesState('../escape', ['/rule.md']); - writeActiveRulesState('foo/bar', ['/rule.md']); - - // Give time for any writes to occur - await new Promise(resolve => setTimeout(resolve, 50)); + await writeActiveRulesState('../escape', ['/rule.md']); + await writeActiveRulesState('foo/bar', ['/rule.md']); // Verify no files were created try { @@ -180,10 +176,7 @@ describe('active-rules-state', () => { const sessionId = 'ses_no_temp'; const matchedPaths = ['/rule.md']; - writeActiveRulesState(sessionId, matchedPaths); - - // Wait for write to complete - await waitForFile(getStateFilePath(sessionId)); + await writeActiveRulesState(sessionId, matchedPaths); // Check that no temp files remain const files = await fs.readdir(testStateDir); @@ -196,15 +189,11 @@ describe('active-rules-state', () => { const sessionId = 'ses_concurrent'; // Fire multiple writes concurrently - writeActiveRulesState(sessionId, ['path1']); - writeActiveRulesState(sessionId, ['path2']); - writeActiveRulesState(sessionId, ['path3']); - - // Wait for all writes to complete - await waitForFile(getStateFilePath(sessionId)); + const first = writeActiveRulesState(sessionId, ['path1']); + const second = writeActiveRulesState(sessionId, ['path2']); + const third = writeActiveRulesState(sessionId, ['path3']); - // Give a bit more time for all queued writes to finish - await new Promise(resolve => setTimeout(resolve, 100)); + await Promise.all([first, second, third]); // The final state should reflect the last write const state = await readActiveRulesState(sessionId); @@ -219,23 +208,16 @@ describe('active-rules-state', () => { // Verify directory doesn't exist yet await expect(fs.access(testStateDir)).rejects.toThrow(); - writeActiveRulesState(sessionId, matchedPaths); - - // Wait for write to complete - await waitForFile(getStateFilePath(sessionId)); + await writeActiveRulesState(sessionId, matchedPaths); // Verify directory now exists await expect(fs.access(testStateDir)).resolves.toBeUndefined(); }); it('handles writes to different sessions independently', async () => { - writeActiveRulesState('ses_a', ['ruleA']); - writeActiveRulesState('ses_b', ['ruleB']); - - // Wait for both writes await Promise.all([ - waitForFile(getStateFilePath('ses_a')), - waitForFile(getStateFilePath('ses_b')), + writeActiveRulesState('ses_a', ['ruleA']), + writeActiveRulesState('ses_b', ['ruleB']), ]); const stateA = await readActiveRulesState('ses_a'); @@ -246,17 +228,3 @@ describe('active-rules-state', () => { }); }); }); - -// Helper to wait for a file to exist -async function waitForFile(filePath: string, timeoutMs = 1000): Promise { - const start = Date.now(); - while (Date.now() - start < timeoutMs) { - try { - await fs.access(filePath); - return; - } catch { - await new Promise(resolve => setTimeout(resolve, 10)); - } - } - throw new Error(`Timed out waiting for file: ${filePath}`); -} diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index aaba3ec..a5c3b1a 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -30,6 +30,11 @@ export function _setStateDirForTesting(dir: string | null): void { stateDirOverride = dir; } +/** @internal Test-only: clear queued writes between tests */ +export function _resetWriteQueues(): void { + writeQueues.clear(); +} + export function resolveStateDir(): string { if (stateDirOverride !== null) { return stateDirOverride; @@ -47,10 +52,10 @@ export function getStateFilePath(sessionId: string): string { export function writeActiveRulesState( sessionId: string, matchedPaths: string[] -): void { +): Promise { if (!isValidSessionId(sessionId)) { debugLog(`Invalid sessionId rejected: ${sessionId}`); - return; + return Promise.resolve(); } const state: ActiveRulesState = { @@ -68,10 +73,12 @@ export function writeActiveRulesState( writeQueues.set(sessionId, currentWrite); - // Fire-and-forget: catch errors to prevent unhandled rejection + // Prevent unhandled rejection for callers that ignore the return value. currentWrite.catch(() => { // Errors already logged in doAtomicWrite }); + + return currentWrite; } async function doAtomicWrite( From 93d06e34435640c0b1dd97335ca6d7588f7d7b3b Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:37:59 +0000 Subject: [PATCH 02/31] refactor: remove pass-through wrappers createSessionStore and extractUserPromptFromParts --- src/index.runtime.test.ts | 25 ++----------------------- src/index.ts | 4 ++-- src/runtime-chat.ts | 15 +-------------- src/runtime.tool-ids.test.ts | 7 ------- src/session-store.test.ts | 4 ++-- src/session-store.ts | 4 ---- 6 files changed, 7 insertions(+), 52 deletions(-) diff --git a/src/index.runtime.test.ts b/src/index.runtime.test.ts index d4e245c..3c86fb8 100644 --- a/src/index.runtime.test.ts +++ b/src/index.runtime.test.ts @@ -128,13 +128,6 @@ describe('module boundary tests', () => { expect(typeof runtimeChatModule.handleChatMessage).toBe('function'); }); - it('should export extractUserPromptFromParts from runtime-chat module', () => { - expect(runtimeChatModule.extractUserPromptFromParts).toBeDefined(); - expect(typeof runtimeChatModule.extractUserPromptFromParts).toBe( - 'function' - ); - }); - it('should detect CI environment correctly via runtime-context module', () => { const originalCI = process.env.CI; @@ -151,20 +144,6 @@ describe('module boundary tests', () => { } }); - it('should extract user prompt from parts via runtime-chat module', () => { - const parts = [ - { type: 'text', text: 'Hello ' }, - { type: 'text', text: 'world' }, - ]; - const result = runtimeChatModule.extractUserPromptFromParts(parts); - expect(result).toBe('Hello world'); - }); - - it('should return empty string for undefined parts in runtime-chat module', () => { - const result = runtimeChatModule.extractUserPromptFromParts(undefined); - expect(result).toBe(''); - }); - it('should re-export evaluateHooks and serializeToolArgs from rule-hooks module', () => { expect(ruleHooksModule.evaluateHooks).toBeDefined(); expect(ruleHooksModule.serializeToolArgs).toBeDefined(); @@ -1126,9 +1105,9 @@ describe('utils runtime exports', () => { }); describe('session-store runtime exports', () => { - it('exports only SessionStore and createSessionStore at runtime', () => { + it('exports only SessionStore at runtime', () => { const exportedKeys = Object.keys(sessionStoreModule).sort(); - expect(exportedKeys).toEqual(['SessionStore', 'createSessionStore']); + expect(exportedKeys).toEqual(['SessionStore']); }); }); diff --git a/src/index.ts b/src/index.ts index 7d82167..cdab047 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,9 +7,9 @@ import type { Plugin, PluginInput } from '@opencode-ai/plugin'; import { discoverRuleFiles } from './utils.js'; import { OpenCodeRulesRuntime } from './runtime.js'; -import { createSessionStore, type SessionState } from './session-store.js'; +import { SessionStore, type SessionState } from './session-store.js'; -const sessionStore = createSessionStore(); +const sessionStore = new SessionStore(); import { createDebugLog } from './debug.js'; const debugLog = createDebugLog(); diff --git a/src/runtime-chat.ts b/src/runtime-chat.ts index 372e6f7..189ffcb 100644 --- a/src/runtime-chat.ts +++ b/src/runtime-chat.ts @@ -13,19 +13,6 @@ export interface ChatMessageOutput { parts?: Array<{ type?: string; text?: string; synthetic?: boolean }>; } -/** - * Extract user prompt text from chat message parts. - * Returns empty string if no text parts found. - */ -export function extractUserPromptFromParts( - parts: - | Array<{ type?: string; text?: string; synthetic?: boolean }> - | undefined -): string { - if (!parts) return ''; - return extractTextFromParts(parts); -} - /** * Handle incoming chat messages to update session state. * Captures user prompts, model IDs, and agent types. @@ -46,7 +33,7 @@ export function handleChatMessage( return; } - const userPrompt = extractUserPromptFromParts(output.parts); + const userPrompt = output.parts ? extractTextFromParts(output.parts) : ''; sessionStore.upsert(sessionID, state => { if (userPrompt) { diff --git a/src/runtime.tool-ids.test.ts b/src/runtime.tool-ids.test.ts index 1459d18..7f73614 100644 --- a/src/runtime.tool-ids.test.ts +++ b/src/runtime.tool-ids.test.ts @@ -27,13 +27,6 @@ describe('runtime module boundaries', () => { expect(runtimeChatModule.handleChatMessage).toBeDefined(); expect(typeof runtimeChatModule.handleChatMessage).toBe('function'); }); - - it('exports extractUserPromptFromParts from runtime-chat module', () => { - expect(runtimeChatModule.extractUserPromptFromParts).toBeDefined(); - expect(typeof runtimeChatModule.extractUserPromptFromParts).toBe( - 'function' - ); - }); }); describe('OpenCodeRulesRuntime.queryAvailableToolIDs', () => { diff --git a/src/session-store.test.ts b/src/session-store.test.ts index 20a09e0..a8f7dfa 100644 --- a/src/session-store.test.ts +++ b/src/session-store.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { createSessionStore, SessionStore } from './session-store.js'; +import { SessionStore } from './session-store.js'; describe('SessionStore', () => { it('prunes oldest sessions when over max', () => { @@ -84,7 +84,7 @@ describe('SessionStore', () => { describe('pending hook injections', () => { it('stores and retrieves pending hook injections', () => { - const store = createSessionStore(); + const store = new SessionStore(); store.upsert('ses_hooks', state => { state.pendingHookInjections = ['Injection A', 'Injection B']; }); diff --git a/src/session-store.ts b/src/session-store.ts index ff1c68c..01ba611 100644 --- a/src/session-store.ts +++ b/src/session-store.ts @@ -128,7 +128,3 @@ export class SessionStore { }; } } - -export function createSessionStore(opts?: SessionStoreOptions): SessionStore { - return new SessionStore(opts ?? { max: 100 }); -} From 9e3be25d36b8804fe0a3a9fe75b17dbbb7dc0c01 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:38:04 +0000 Subject: [PATCH 03/31] refactor: extract shared logWarning helper and remove redundant guard --- src/debug.ts | 10 ++++++++++ src/rule-discovery.ts | 17 +++-------------- src/rule-filter.ts | 3 --- src/rule-metadata.ts | 13 ++----------- src/runtime.ts | 7 ++----- 5 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/debug.ts b/src/debug.ts index 4663153..fbad87b 100644 --- a/src/debug.ts +++ b/src/debug.ts @@ -7,3 +7,13 @@ export function createDebugLog(prefix = '[opencode-rules]'): DebugLog { } }; } + +/** Format an unknown error value into a human-readable message. */ +export function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** Log a warning with the standard opencode-rules prefix. */ +export function logWarning(context: string, error: unknown): void { + console.warn(`[opencode-rules] Warning: ${context}: ${formatError(error)}`); +} diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index d726cb3..43b6729 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -5,7 +5,7 @@ import { stat, readFile, readdir } from 'fs/promises'; import path from 'path'; import os from 'os'; -import { createDebugLog } from './debug'; +import { createDebugLog, logWarning } from './debug'; import { parseRuleMetadata, stripFrontmatter, @@ -54,14 +54,12 @@ export async function getCachedRule( const stats = await stat(filePath); const mtime = stats.mtimeMs; - // Check if we have a valid cached entry const cached = ruleCache.get(filePath); if (cached && cached.mtime === mtime) { debugLog(`Cache hit: ${filePath}`); return cached; } - // Read and cache the file debugLog(`Cache miss: ${filePath}`); const content = await readFile(filePath, 'utf-8'); const metadata = parseRuleMetadata(content); @@ -79,10 +77,7 @@ export async function getCachedRule( } catch (error) { // Remove stale cache entry if file no longer exists ruleCache.delete(filePath); - const message = error instanceof Error ? error.message : String(error); - console.warn( - `[opencode-rules] Warning: Failed to read rule file ${filePath}: ${message}` - ); + logWarning(`Failed to read rule file ${filePath}`, error); return undefined; } } @@ -121,7 +116,6 @@ async function scanDirectoryRecursively( try { const entries = await readdir(dir, { withFileTypes: true }); for (const entry of entries) { - // Skip hidden files and directories if (entry.name.startsWith('.')) { continue; } @@ -129,10 +123,8 @@ async function scanDirectoryRecursively( const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { - // Recurse into subdirectory results.push(...(await scanDirectoryRecursively(fullPath, baseDir))); } else if (entry.name.endsWith('.md') || entry.name.endsWith('.mdc')) { - // Add markdown file const relativePath = path.relative(baseDir, fullPath); results.push({ filePath: fullPath, relativePath }); } @@ -143,10 +135,7 @@ async function scanDirectoryRecursively( return results; } // Log non-ENOENT directory read errors - const message = error instanceof Error ? error.message : String(error); - console.warn( - `[opencode-rules] Warning: Failed to read directory ${dir}: ${message}` - ); + logWarning(`Failed to read directory ${dir}`, error); } return results; diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 706a70f..8cbae1b 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -51,9 +51,6 @@ export function toolsMatchAvailable( availableToolIDs: string[], requiredTools: string[] ): boolean { - if (requiredTools.length === 0) { - return false; - } // Create a Set for O(1) lookups const availableSet = new Set(availableToolIDs); return requiredTools.some(tool => availableSet.has(tool)); diff --git a/src/rule-metadata.ts b/src/rule-metadata.ts index 13e308b..6e8d062 100644 --- a/src/rule-metadata.ts +++ b/src/rule-metadata.ts @@ -3,6 +3,7 @@ */ const { parse: parseYaml } = await import('yaml'); +import { logWarning } from './debug.js'; /** * Metadata extracted from .mdc file frontmatter @@ -83,25 +84,21 @@ function extractStringArray(value: unknown): string[] | undefined { * Extracts frontmatter (---) and returns metadata object. */ export function parseRuleMetadata(content: string): RuleMetadata | undefined { - // Check if content starts with frontmatter if (!content.startsWith('---')) { return undefined; } - // Find the closing --- marker const endIndex = content.indexOf('---', 3); if (endIndex === -1) { return undefined; } - // Extract the YAML frontmatter const frontmatter = content.substring(3, endIndex).trim(); if (!frontmatter) { return undefined; } try { - // Parse YAML using the yaml package const parsed = parseYaml(frontmatter) as ParsedFrontmatter | null; if (!parsed || typeof parsed !== 'object') { return undefined; @@ -109,7 +106,6 @@ export function parseRuleMetadata(content: string): RuleMetadata | undefined { const metadata: RuleMetadata = {}; - // Array fields to extract using shared helper const arrayFields: StringArrayField[] = [ 'globs', 'keywords', @@ -129,12 +125,10 @@ export function parseRuleMetadata(content: string): RuleMetadata | undefined { } } - // Extract ci boolean (only if strictly boolean) if (typeof parsed.ci === 'boolean') { metadata.ci = parsed.ci; } - // Extract match (normalize to 'any' | 'all' only) if (parsed.match === 'any' || parsed.match === 'all') { metadata.match = parsed.match; } @@ -171,10 +165,7 @@ export function parseRuleMetadata(content: string): RuleMetadata | undefined { return Object.keys(metadata).length > 0 ? metadata : undefined; } catch (error) { // Log warning for YAML parsing errors - const message = error instanceof Error ? error.message : String(error); - console.warn( - `[opencode-rules] Warning: Failed to parse YAML frontmatter: ${message}` - ); + logWarning('Failed to parse YAML frontmatter', error); return undefined; } } diff --git a/src/runtime.ts b/src/runtime.ts index 27dab6c..6fa602c 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -10,7 +10,7 @@ import { type MessageWithInfo, } from './message-context.js'; import { extractConnectedMcpCapabilityIDs } from './mcp-tools.js'; -import { createDebugLog, type DebugLog } from './debug.js'; +import { createDebugLog, logWarning, type DebugLog } from './debug.js'; import type { SessionStore } from './session-store.js'; import { buildFilterContext, @@ -449,10 +449,7 @@ export class OpenCodeRulesRuntime { ); await execAsync(command, { cwd: this.projectDirectory }); } catch (error) { - const message = error instanceof Error ? error.message : String(error); - console.warn( - `[opencode-rules] Warning: Hook side-effect failed: ${message}` - ); + logWarning('Hook side-effect failed', error); } } From f004d2f3bcd0b2f8fb5c44e3922f2f8b61117db8 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:38:15 +0000 Subject: [PATCH 04/31] style: remove restating comments in rule-discovery, rule-metadata, and active-rules-state --- src/active-rules-state.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index a5c3b1a..85adbc5 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -93,14 +93,10 @@ async function doAtomicWrite( ); try { - // Ensure directory exists + // Atomic write: temp file + rename ensures crash safety await fs.mkdir(stateDir, { recursive: true }); - - // Write to temp file const content = JSON.stringify(state); await fs.writeFile(tempPath, content, 'utf-8'); - - // Atomic rename await fs.rename(tempPath, finalPath); } catch (error) { debugLog( From 80fd6bfc25b7e30771f3a917bf082a37dec77e4e Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:38:42 +0000 Subject: [PATCH 05/31] chore: ignore .desloppify directory and scorecard.png --- .gitignore | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 7cb2afb..fbd2ee3 100644 --- a/.gitignore +++ b/.gitignore @@ -55,8 +55,5 @@ temp/ .opencode/plans # Desloppify runtime artifacts -.desloppify/external_review_sessions/ -.desloppify/review_packets/ -.desloppify/subagents/ -.desloppify/*.bak -.desloppify/review_packet_blind.json +.desloppify/ +scorecard.png From bfbc1459998b4b9454a33f30c302b07b1b52b2b7 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:41:30 +0000 Subject: [PATCH 06/31] refactor(runtime-context): move debugLog and projectDirectory into BuildFilterContextOptions --- src/runtime-context.ts | 17 ++++++++++++----- src/runtime.ts | 17 +++++------------ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/runtime-context.ts b/src/runtime-context.ts index 17bee5d..8de82a4 100644 --- a/src/runtime-context.ts +++ b/src/runtime-context.ts @@ -10,6 +10,8 @@ export interface BuildFilterContextOptions { availableToolIDs: string[]; modelID: string | undefined; agentType: string | undefined; + projectDirectory: string; + debugLog: DebugLog; } /** @@ -67,12 +69,17 @@ export function detectCiEnvironment(): boolean { * Assembles runtime information from various sources. */ export async function buildFilterContext( - opts: BuildFilterContextOptions, - projectDirectory: string, - debugLog: DebugLog + opts: BuildFilterContextOptions ): Promise { - const { contextFilePaths, userPrompt, availableToolIDs, modelID, agentType } = - opts; + const { + contextFilePaths, + userPrompt, + availableToolIDs, + modelID, + agentType, + projectDirectory, + debugLog, + } = opts; const command = extractSlashCommand(userPrompt); diff --git a/src/runtime.ts b/src/runtime.ts index 6fa602c..dd2c661 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -12,10 +12,7 @@ import { import { extractConnectedMcpCapabilityIDs } from './mcp-tools.js'; import { createDebugLog, logWarning, type DebugLog } from './debug.js'; import type { SessionStore } from './session-store.js'; -import { - buildFilterContext, - type BuildFilterContextOptions, -} from './runtime-context.js'; +import { buildFilterContext } from './runtime-context.js'; import { handleChatMessage, type ChatMessageInput, @@ -266,19 +263,15 @@ export class OpenCodeRulesRuntime { const availableToolIDs = await this.queryAvailableToolIDs(); - const filterContextOpts: BuildFilterContextOptions = { + const filterContext: RuleFilterContext = await buildFilterContext({ contextFilePaths: contextPaths, userPrompt, availableToolIDs, modelID: sessionState?.lastModelID, agentType: sessionState?.lastAgentType, - }; - - const filterContext: RuleFilterContext = await buildFilterContext( - filterContextOpts, - this.projectDirectory, - this.debugLog - ); + projectDirectory: this.projectDirectory, + debugLog: this.debugLog, + }); const result = await readAndFormatRules(this.ruleFiles, filterContext); formattedRules = result.formattedRules; From 6f1f6e30970059c5fde66b79b5d9bcef02342ce0 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:42:24 +0000 Subject: [PATCH 07/31] refactor(active-rules-state): remove fire-and-forget catch so callers can handle errors --- src/active-rules-state.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index 85adbc5..ec38a2d 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -73,11 +73,6 @@ export function writeActiveRulesState( writeQueues.set(sessionId, currentWrite); - // Prevent unhandled rejection for callers that ignore the return value. - currentWrite.catch(() => { - // Errors already logged in doAtomicWrite - }); - return currentWrite; } From bdcccfd374e3eb9f1fd3f1e8b7334595f91e98f9 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:49:00 +0000 Subject: [PATCH 08/31] refactor: standardize I/O boundary error returns to null instead of undefined --- src/git-branch.test.ts | 12 ++++++------ src/git-branch.ts | 12 +++++------- src/index.rules.test.ts | 6 +++--- src/rule-discovery.ts | 6 +++--- src/rule-metadata.ts | 14 +++++++------- src/runtime-context.ts | 6 +++--- tui/data/rules.ts | 2 +- 7 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/git-branch.test.ts b/src/git-branch.test.ts index 7558e29..2276406 100644 --- a/src/git-branch.test.ts +++ b/src/git-branch.test.ts @@ -65,7 +65,7 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/not-a-repo'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); it('returns undefined if command fails', async () => { @@ -76,7 +76,7 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/project'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); it('returns undefined for detached HEAD state', async () => { @@ -86,7 +86,7 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/project'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); it('trims stdout whitespace', async () => { @@ -116,7 +116,7 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/project'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); it('returns undefined when stdout is only whitespace', async () => { @@ -126,7 +126,7 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/project'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); it('never throws on unexpected errors', async () => { @@ -135,6 +135,6 @@ describe('getGitBranch', () => { }); const branch = await getGitBranch('/project'); - expect(branch).toBeUndefined(); + expect(branch).toBeNull(); }); }); diff --git a/src/git-branch.ts b/src/git-branch.ts index fdff579..4a82db5 100644 --- a/src/git-branch.ts +++ b/src/git-branch.ts @@ -2,11 +2,9 @@ import { execFile, type ExecFileOptions } from 'child_process'; const GIT_TIMEOUT_MS = 5000; -export async function getGitBranch( - projectDir: string -): Promise { +export async function getGitBranch(projectDir: string): Promise { try { - const branch = await new Promise(resolve => { + const branch = await new Promise(resolve => { const opts: ExecFileOptions = { cwd: projectDir, timeout: GIT_TIMEOUT_MS, @@ -18,12 +16,12 @@ export async function getGitBranch( opts, (error, stdout) => { if (error) { - resolve(undefined); + resolve(null); return; } const trimmed = String(stdout).trim(); if (!trimmed || trimmed === 'HEAD') { - resolve(undefined); + resolve(null); return; } resolve(trimmed); @@ -32,6 +30,6 @@ export async function getGitBranch( }); return branch; } catch { - return undefined; + return null; } } diff --git a/src/index.rules.test.ts b/src/index.rules.test.ts index 324ef1f..c261d01 100644 --- a/src/index.rules.test.ts +++ b/src/index.rules.test.ts @@ -622,7 +622,7 @@ This is a rule for TypeScript components.`; it('should return undefined for files without metadata', () => { const content = 'This rule should always apply.'; const metadata = parseRuleMetadata(content); - expect(metadata).toBeUndefined(); + expect(metadata).toBeNull(); }); it('should extract rule content without metadata', () => { @@ -1347,13 +1347,13 @@ describe('YAML Parsing Edge Cases', () => { it('should handle empty frontmatter', () => { const content = '---\n---\nRule content here'; const metadata = parseRuleMetadata(content); - expect(metadata).toBeUndefined(); + expect(metadata).toBeNull(); }); it('should handle frontmatter with only whitespace', () => { const content = '---\n \n---\nRule content here'; const metadata = parseRuleMetadata(content); - expect(metadata).toBeUndefined(); + expect(metadata).toBeNull(); }); it('should handle complex YAML structures', () => { diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index 43b6729..d690e67 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -21,7 +21,7 @@ interface CachedRule { /** Raw file content */ content: string; /** Parsed metadata from frontmatter */ - metadata: RuleMetadata | undefined; + metadata: RuleMetadata | null; /** Content with frontmatter stripped */ strippedContent: string; /** File modification time for cache invalidation */ @@ -49,7 +49,7 @@ export function clearRuleCache(): void { */ export async function getCachedRule( filePath: string -): Promise { +): Promise { try { const stats = await stat(filePath); const mtime = stats.mtimeMs; @@ -78,7 +78,7 @@ export async function getCachedRule( // Remove stale cache entry if file no longer exists ruleCache.delete(filePath); logWarning(`Failed to read rule file ${filePath}`, error); - return undefined; + return null; } } diff --git a/src/rule-metadata.ts b/src/rule-metadata.ts index 6e8d062..f3a7859 100644 --- a/src/rule-metadata.ts +++ b/src/rule-metadata.ts @@ -83,25 +83,25 @@ function extractStringArray(value: unknown): string[] | undefined { * Parse YAML metadata from rule file content using the yaml package. * Extracts frontmatter (---) and returns metadata object. */ -export function parseRuleMetadata(content: string): RuleMetadata | undefined { +export function parseRuleMetadata(content: string): RuleMetadata | null { if (!content.startsWith('---')) { - return undefined; + return null; } const endIndex = content.indexOf('---', 3); if (endIndex === -1) { - return undefined; + return null; } const frontmatter = content.substring(3, endIndex).trim(); if (!frontmatter) { - return undefined; + return null; } try { const parsed = parseYaml(frontmatter) as ParsedFrontmatter | null; if (!parsed || typeof parsed !== 'object') { - return undefined; + return null; } const metadata: RuleMetadata = {}; @@ -162,11 +162,11 @@ export function parseRuleMetadata(content: string): RuleMetadata | undefined { } // Return metadata only if it has content - return Object.keys(metadata).length > 0 ? metadata : undefined; + return Object.keys(metadata).length > 0 ? metadata : null; } catch (error) { // Log warning for YAML parsing errors logWarning('Failed to parse YAML frontmatter', error); - return undefined; + return null; } } diff --git a/src/runtime-context.ts b/src/runtime-context.ts index 8de82a4..fc3d551 100644 --- a/src/runtime-context.ts +++ b/src/runtime-context.ts @@ -93,11 +93,11 @@ export async function buildFilterContext( projectTags = undefined; } - let gitBranch: string | undefined; + let gitBranch: string | null = null; try { gitBranch = await getGitBranch(projectDirectory); } catch { - gitBranch = undefined; + gitBranch = null; } const os = process.platform; @@ -129,7 +129,7 @@ export async function buildFilterContext( if (projectTags !== undefined) { context.projectTags = projectTags; } - if (gitBranch !== undefined) { + if (gitBranch !== null) { context.gitBranch = gitBranch; } diff --git a/tui/data/rules.ts b/tui/data/rules.ts index 61cbd03..42cd823 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -130,7 +130,7 @@ export function ruleSource( /** * Check if metadata has any conditional fields set. */ -export function hasConditions(meta: RuleMetadata | undefined): boolean { +export function hasConditions(meta: RuleMetadata | null | undefined): boolean { if (!meta) return false; return !!( meta.globs || From 58ba71c9d47a2091eb2e9cf467a7e5a6f7a32ba8 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:56:56 +0000 Subject: [PATCH 09/31] refactor(runtime): replace as any cast with typed OpenCodeClient interface --- src/runtime.ts | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/runtime.ts b/src/runtime.ts index dd2c661..8434903 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -37,6 +37,19 @@ interface SystemTransformOutput { system?: string | string[]; } +interface OpenCodeClient { + tool?: { + ids?: (args: { + query: { directory: string }; + }) => Promise<{ data: string[] }>; + }; + mcp?: { + status?: (args: { + query: { directory: string }; + }) => Promise<{ connected?: Array<{ id: string }> }>; + }; +} + interface OpenCodeRulesRuntimeOptions { client: unknown; directory: string; @@ -48,7 +61,7 @@ interface OpenCodeRulesRuntimeOptions { } export class OpenCodeRulesRuntime { - private client: unknown; + private client: OpenCodeClient; private directory: string; private projectDirectory: string; private ruleFiles: DiscoveredRule[]; @@ -57,7 +70,7 @@ export class OpenCodeRulesRuntime { private now: () => number; constructor(opts: OpenCodeRulesRuntimeOptions) { - this.client = opts.client; + this.client = opts.client as OpenCodeClient; this.directory = opts.directory; this.projectDirectory = opts.projectDirectory; this.ruleFiles = opts.ruleFiles; @@ -336,14 +349,15 @@ export class OpenCodeRulesRuntime { private async queryAvailableToolIDs(): Promise { const ids = new Set(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const client = this.client as any; const query = { directory: this.directory }; + const toolPromise = this.client.tool?.ids?.({ query }); + const mcpPromise = this.client.mcp?.status?.({ query }); + const [toolResult, mcpResult] = await Promise.allSettled([ - client.tool?.ids?.({ query }), - client.mcp?.status?.({ query }), - ]); + toolPromise, + mcpPromise, + ] as const); if ( toolResult.status === 'fulfilled' && @@ -365,8 +379,14 @@ export class OpenCodeRulesRuntime { ); } - if (mcpResult.status === 'fulfilled' && mcpResult.value?.data) { - const mcpIds = extractConnectedMcpCapabilityIDs(mcpResult.value.data); + if ( + mcpResult.status === 'fulfilled' && + mcpResult.value && + 'data' in mcpResult.value + ) { + const mcpIds = extractConnectedMcpCapabilityIDs( + mcpResult.value.data as Record + ); for (const id of mcpIds) { ids.add(id); } From f3f011917065b72a902e48d6b443e405d3f83c37 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 18:59:16 +0000 Subject: [PATCH 10/31] refactor(runtime): add completion audit log for hook side-effects --- src/runtime.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/runtime.ts b/src/runtime.ts index 8434903..d897b31 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -461,6 +461,9 @@ export class OpenCodeRulesRuntime { `Executing hook side-effect for session ${sessionID}: ${command}` ); await execAsync(command, { cwd: this.projectDirectory }); + this.debugLog( + `Hook side-effect completed for session ${sessionID}: ${command}` + ); } catch (error) { logWarning('Hook side-effect failed', error); } From 728a1ad4b72aa4a5e75c1f097202bb0d7167f6fa Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:01:13 +0000 Subject: [PATCH 11/31] fix(message-context): strictly require role === 'user' in extractLatestUserPrompt --- src/message-context.test.ts | 2 ++ src/message-context.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/message-context.test.ts b/src/message-context.test.ts index 1b55be7..4048d43 100644 --- a/src/message-context.test.ts +++ b/src/message-context.test.ts @@ -23,9 +23,11 @@ describe('message-context', () => { it('extracts latest non-synthetic user prompt', () => { const prompt = extractLatestUserPrompt([ { + role: 'user', parts: [{ type: 'text', text: 'older', synthetic: true }], }, { + role: 'user', parts: [{ type: 'text', text: 'hello world' }], }, ]); diff --git a/src/message-context.ts b/src/message-context.ts index aec34b8..7762bbc 100644 --- a/src/message-context.ts +++ b/src/message-context.ts @@ -90,7 +90,7 @@ export function extractLatestUserPrompt( ): string | undefined { for (let i = messages.length - 1; i >= 0; i--) { const message = messages[i]; - if (message.role && message.role !== 'user') continue; + if (message.role !== 'user') continue; const parts = message.parts || []; const userPrompt = extractTextFromParts(parts); From 0af29bec8f4e1b51d379877a050baa2764b2f875 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:03:48 +0000 Subject: [PATCH 12/31] fix(active-rules-state): throw on invalid sessionId to align with getStateFilePath --- src/active-rules-state.test.ts | 19 +++++++------------ src/active-rules-state.ts | 3 +-- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index 46336ba..da987fc 100644 --- a/src/active-rules-state.test.ts +++ b/src/active-rules-state.test.ts @@ -153,18 +153,13 @@ describe('active-rules-state', () => { expect(state).toBeNull(); }); - it('silently ignores write with invalid sessionId', async () => { - await writeActiveRulesState('../escape', ['/rule.md']); - await writeActiveRulesState('foo/bar', ['/rule.md']); - - // Verify no files were created - try { - await fs.access(testStateDir); - const files = await fs.readdir(testStateDir); - expect(files).toHaveLength(0); - } catch { - // Directory doesn't exist, which is expected - } + it('throws on write with invalid sessionId', () => { + expect(() => writeActiveRulesState('../escape', ['/rule.md'])).toThrow( + 'Invalid sessionId' + ); + expect(() => writeActiveRulesState('foo/bar', ['/rule.md'])).toThrow( + 'Invalid sessionId' + ); }); it('returns null for read with invalid sessionId', async () => { diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index ec38a2d..53230b2 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -54,8 +54,7 @@ export function writeActiveRulesState( matchedPaths: string[] ): Promise { if (!isValidSessionId(sessionId)) { - debugLog(`Invalid sessionId rejected: ${sessionId}`); - return Promise.resolve(); + throw new Error(`Invalid sessionId: ${sessionId}`); } const state: ActiveRulesState = { From 76fbeef067d02ef0efc304a53f22a4bb94d6696c Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:04:56 +0000 Subject: [PATCH 13/31] style(test-fixtures): rename duplicate section headers to be unique --- src/test-fixtures.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test-fixtures.ts b/src/test-fixtures.ts index d1cfb65..a6860a0 100644 --- a/src/test-fixtures.ts +++ b/src/test-fixtures.ts @@ -44,7 +44,7 @@ export function getTestDirs(): TestDirs { } // ============================================================================ -// Environment Snapshot Helpers +// Rule Helpers // ============================================================================ /** @@ -101,7 +101,7 @@ export function restoreCiEnvVars(saved: CiEnvSnapshot): void { } // ============================================================================ -// Environment Snapshot Helpers +// Mock Plugin Input Helpers // ============================================================================ interface MockPluginInput { @@ -152,7 +152,7 @@ export function createMockPluginInput(opts: MockPluginInput): { } // ============================================================================ -// Environment Snapshot Helpers +// Generic Environment Snapshot Helpers // ============================================================================ /** From 8c8f7e0aa3157d90c7e23deb40e5318e4f696b06 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:06:23 +0000 Subject: [PATCH 14/31] style: add .js extension to local imports in active-rules-state and rule-discovery --- src/active-rules-state.ts | 2 +- src/rule-discovery.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index 53230b2..c30395d 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import { createDebugLog } from './debug'; +import { createDebugLog } from './debug.js'; const debugLog = createDebugLog(); diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index d690e67..18d90dc 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -5,12 +5,12 @@ import { stat, readFile, readdir } from 'fs/promises'; import path from 'path'; import os from 'os'; -import { createDebugLog, logWarning } from './debug'; +import { createDebugLog, logWarning } from './debug.js'; import { parseRuleMetadata, stripFrontmatter, type RuleMetadata, -} from './rule-metadata'; +} from './rule-metadata.js'; const debugLog = createDebugLog(); From fa6b160836ec89bf8d4da0253397ed464800d30d Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:08:27 +0000 Subject: [PATCH 15/31] style: add node: prefix to all Node built-in module imports --- src/git-branch.test.ts | 2 +- src/git-branch.ts | 2 +- src/index.integration.test.ts | 4 ++-- src/index.rules.test.ts | 4 ++-- src/index.runtime.test.ts | 4 ++-- src/index.test.ts | 4 ++-- src/message-context.ts | 2 +- src/project-fingerprint.test.ts | 2 +- src/project-fingerprint.ts | 4 ++-- src/rule-discovery.ts | 6 +++--- src/test-fixtures.ts | 6 +++--- 11 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/git-branch.test.ts b/src/git-branch.test.ts index 2276406..6f497ab 100644 --- a/src/git-branch.test.ts +++ b/src/git-branch.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import * as childProcess from 'child_process'; +import * as childProcess from 'node:child_process'; import { getGitBranch } from './git-branch.js'; diff --git a/src/git-branch.ts b/src/git-branch.ts index 4a82db5..3f70ac4 100644 --- a/src/git-branch.ts +++ b/src/git-branch.ts @@ -1,4 +1,4 @@ -import { execFile, type ExecFileOptions } from 'child_process'; +import { execFile, type ExecFileOptions } from 'node:child_process'; const GIT_TIMEOUT_MS = 5000; diff --git a/src/index.integration.test.ts b/src/index.integration.test.ts index 27acef4..a8d3735 100644 --- a/src/index.integration.test.ts +++ b/src/index.integration.test.ts @@ -5,8 +5,8 @@ * Split from index.test.ts for maintainability. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import path from 'path'; -import { writeFileSync, utimesSync } from 'fs'; +import path from 'node:path'; +import { writeFileSync, utimesSync } from 'node:fs'; import { readAndFormatRules, clearRuleCache } from './utils.js'; import { setupTestDirs, diff --git a/src/index.rules.test.ts b/src/index.rules.test.ts index c261d01..6cfe101 100644 --- a/src/index.rules.test.ts +++ b/src/index.rules.test.ts @@ -3,8 +3,8 @@ * Split from index.test.ts for maintainability. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import path from 'path'; -import { writeFileSync, mkdirSync, rmSync } from 'fs'; +import path from 'node:path'; +import { writeFileSync, mkdirSync, rmSync } from 'node:fs'; import { discoverRuleFiles, parseRuleMetadata, diff --git a/src/index.runtime.test.ts b/src/index.runtime.test.ts index 3c86fb8..2628b5f 100644 --- a/src/index.runtime.test.ts +++ b/src/index.runtime.test.ts @@ -3,8 +3,8 @@ * Split from index.test.ts for maintainability. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import path from 'path'; -import { writeFileSync, mkdirSync, readdirSync } from 'fs'; +import path from 'node:path'; +import { writeFileSync, mkdirSync, readdirSync } from 'node:fs'; import { setupTestDirs, teardownTestDirs, diff --git a/src/index.test.ts b/src/index.test.ts index ee1bc15..db0216d 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -13,8 +13,8 @@ * focused test file above. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import path from 'path'; -import { writeFileSync, mkdirSync } from 'fs'; +import path from 'node:path'; +import { writeFileSync, mkdirSync } from 'node:fs'; import { readAndFormatRules, clearRuleCache } from './utils.js'; import { __testOnly } from './index.js'; import { diff --git a/src/message-context.ts b/src/message-context.ts index 7762bbc..33c976b 100644 --- a/src/message-context.ts +++ b/src/message-context.ts @@ -1,4 +1,4 @@ -import path from 'path'; +import path from 'node:path'; import type { Message, MessagePart } from './message-paths.js'; export interface MessagePartWithSession { diff --git a/src/project-fingerprint.test.ts b/src/project-fingerprint.test.ts index da7e169..39b391b 100644 --- a/src/project-fingerprint.test.ts +++ b/src/project-fingerprint.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import * as fs from 'fs/promises'; +import * as fs from 'node:fs/promises'; import { detectProjectTags } from './project-fingerprint.js'; diff --git a/src/project-fingerprint.ts b/src/project-fingerprint.ts index e60074d..8890f0c 100644 --- a/src/project-fingerprint.ts +++ b/src/project-fingerprint.ts @@ -1,5 +1,5 @@ -import * as fs from 'fs/promises'; -import * as path from 'path'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; const SIMPLE_MARKERS: Array<[string, string]> = [ ['package.json', 'node'], diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index 18d90dc..50fca94 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -2,9 +2,9 @@ * Rule file discovery utilities */ -import { stat, readFile, readdir } from 'fs/promises'; -import path from 'path'; -import os from 'os'; +import { stat, readFile, readdir } from 'node:fs/promises'; +import path from 'node:path'; +import os from 'node:os'; import { createDebugLog, logWarning } from './debug.js'; import { parseRuleMetadata, diff --git a/src/test-fixtures.ts b/src/test-fixtures.ts index a6860a0..2a8e268 100644 --- a/src/test-fixtures.ts +++ b/src/test-fixtures.ts @@ -2,9 +2,9 @@ * Shared test fixtures, builders, and helpers for opencode-rules tests. * Extracted to reduce duplication and tighten typing across test files. */ -import path from 'path'; -import os from 'os'; -import { mkdirSync, mkdtempSync, rmSync } from 'fs'; +import path from 'node:path'; +import os from 'node:os'; +import { mkdirSync, mkdtempSync, rmSync } from 'node:fs'; import type { DiscoveredRule } from './utils.js'; // ============================================================================ From 34262df435f7e1b29a982d350b91b50717fcfbcd Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:09:48 +0000 Subject: [PATCH 16/31] style(active-rules-state): rename sessionId to sessionID to match convention --- src/active-rules-state.test.ts | 54 +++++++++++++++++----------------- src/active-rules-state.ts | 52 ++++++++++++++++---------------- 2 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index da987fc..54b6f16 100644 --- a/src/active-rules-state.test.ts +++ b/src/active-rules-state.test.ts @@ -63,30 +63,30 @@ describe('active-rules-state', () => { expect(filePath).toBe(path.join(testStateDir, 'ses_123.json')); }); - it('throws for sessionId with path traversal', () => { - expect(() => getStateFilePath('../escape')).toThrow('Invalid sessionId'); - expect(() => getStateFilePath('foo/bar')).toThrow('Invalid sessionId'); - expect(() => getStateFilePath('/absolute')).toThrow('Invalid sessionId'); + it('throws for sessionID with path traversal', () => { + expect(() => getStateFilePath('../escape')).toThrow('Invalid sessionID'); + expect(() => getStateFilePath('foo/bar')).toThrow('Invalid sessionID'); + expect(() => getStateFilePath('/absolute')).toThrow('Invalid sessionID'); }); - it('throws for sessionId with special characters', () => { - expect(() => getStateFilePath('ses.123')).toThrow('Invalid sessionId'); - expect(() => getStateFilePath('ses 123')).toThrow('Invalid sessionId'); - expect(() => getStateFilePath('')).toThrow('Invalid sessionId'); + it('throws for sessionID with special characters', () => { + expect(() => getStateFilePath('ses.123')).toThrow('Invalid sessionID'); + expect(() => getStateFilePath('ses 123')).toThrow('Invalid sessionID'); + expect(() => getStateFilePath('')).toThrow('Invalid sessionID'); }); }); describe('writeActiveRulesState and readActiveRulesState', () => { it('write/read round-trip preserves data', async () => { - const sessionId = 'ses_roundtrip'; + const sessionID = 'ses_roundtrip'; const matchedPaths = ['/path/to/rule1.md', '/path/to/rule2.md']; - await writeActiveRulesState(sessionId, matchedPaths); + await writeActiveRulesState(sessionID, matchedPaths); - const state = await readActiveRulesState(sessionId); + const state = await readActiveRulesState(sessionID); expect(state).not.toBeNull(); - expect(state!.sessionId).toBe(sessionId); + expect(state!.sessionID).toBe(sessionID); expect(state!.matchedRulePaths).toEqual(matchedPaths); expect(typeof state!.evaluatedAt).toBe('number'); expect(state!.evaluatedAt).toBeLessThanOrEqual(Date.now()); @@ -124,7 +124,7 @@ describe('active-rules-state', () => { await fs.writeFile( filePath, JSON.stringify({ - sessionId: 123, + sessionID: 123, matchedRulePaths: 'not-an-array', evaluatedAt: 'not-a-number', }), @@ -142,7 +142,7 @@ describe('active-rules-state', () => { await fs.writeFile( filePath, JSON.stringify({ - sessionId: 'ses_badarray', + sessionID: 'ses_badarray', matchedRulePaths: ['/valid.md', 123, null], evaluatedAt: Date.now(), }), @@ -153,25 +153,25 @@ describe('active-rules-state', () => { expect(state).toBeNull(); }); - it('throws on write with invalid sessionId', () => { + it('throws on write with invalid sessionID', () => { expect(() => writeActiveRulesState('../escape', ['/rule.md'])).toThrow( - 'Invalid sessionId' + 'Invalid sessionID' ); expect(() => writeActiveRulesState('foo/bar', ['/rule.md'])).toThrow( - 'Invalid sessionId' + 'Invalid sessionID' ); }); - it('returns null for read with invalid sessionId', async () => { + it('returns null for read with invalid sessionID', async () => { const state = await readActiveRulesState('../escape'); expect(state).toBeNull(); }); it('no temp file remains after write', async () => { - const sessionId = 'ses_no_temp'; + const sessionID = 'ses_no_temp'; const matchedPaths = ['/rule.md']; - await writeActiveRulesState(sessionId, matchedPaths); + await writeActiveRulesState(sessionID, matchedPaths); // Check that no temp files remain const files = await fs.readdir(testStateDir); @@ -181,29 +181,29 @@ describe('active-rules-state', () => { }); it('serializes concurrent writes for same session', async () => { - const sessionId = 'ses_concurrent'; + const sessionID = 'ses_concurrent'; // Fire multiple writes concurrently - const first = writeActiveRulesState(sessionId, ['path1']); - const second = writeActiveRulesState(sessionId, ['path2']); - const third = writeActiveRulesState(sessionId, ['path3']); + const first = writeActiveRulesState(sessionID, ['path1']); + const second = writeActiveRulesState(sessionID, ['path2']); + const third = writeActiveRulesState(sessionID, ['path3']); await Promise.all([first, second, third]); // The final state should reflect the last write - const state = await readActiveRulesState(sessionId); + const state = await readActiveRulesState(sessionID); expect(state).not.toBeNull(); expect(state!.matchedRulePaths).toEqual(['path3']); }); it('creates state directory when it does not exist', async () => { - const sessionId = 'ses_newdir'; + const sessionID = 'ses_newdir'; const matchedPaths = ['/rule.md']; // Verify directory doesn't exist yet await expect(fs.access(testStateDir)).rejects.toThrow(); - await writeActiveRulesState(sessionId, matchedPaths); + await writeActiveRulesState(sessionID, matchedPaths); // Verify directory now exists await expect(fs.access(testStateDir)).resolves.toBeUndefined(); diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index c30395d..0fa5a6a 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -7,7 +7,7 @@ import { createDebugLog } from './debug.js'; const debugLog = createDebugLog(); export interface ActiveRulesState { - sessionId: string; + sessionID: string; matchedRulePaths: string[]; evaluatedAt: number; } @@ -18,11 +18,11 @@ const writeQueues = new Map>(); // Allows tests to override the state directory let stateDirOverride: string | null = null; -// Strict pattern for safe sessionId: alphanumeric, underscore, hyphen only +// Strict pattern for safe sessionID: alphanumeric, underscore, hyphen only const SAFE_SESSION_ID_PATTERN = /^[A-Za-z0-9_-]+$/; -function isValidSessionId(sessionId: string): boolean { - return SAFE_SESSION_ID_PATTERN.test(sessionId); +function isValidSessionId(sessionID: string): boolean { + return SAFE_SESSION_ID_PATTERN.test(sessionID); } /** @internal Test-only: override the state directory */ @@ -42,48 +42,48 @@ export function resolveStateDir(): string { return path.join(os.homedir(), '.opencode', 'state', 'opencode-rules'); } -export function getStateFilePath(sessionId: string): string { - if (!isValidSessionId(sessionId)) { - throw new Error(`Invalid sessionId: ${sessionId}`); +export function getStateFilePath(sessionID: string): string { + if (!isValidSessionId(sessionID)) { + throw new Error(`Invalid sessionID: ${sessionID}`); } - return path.join(resolveStateDir(), `${sessionId}.json`); + return path.join(resolveStateDir(), `${sessionID}.json`); } export function writeActiveRulesState( - sessionId: string, + sessionID: string, matchedPaths: string[] ): Promise { - if (!isValidSessionId(sessionId)) { - throw new Error(`Invalid sessionId: ${sessionId}`); + if (!isValidSessionId(sessionID)) { + throw new Error(`Invalid sessionID: ${sessionID}`); } const state: ActiveRulesState = { - sessionId, + sessionID, matchedRulePaths: matchedPaths, evaluatedAt: Date.now(), }; // Chain onto existing queue for this session, or start fresh - const previousWrite = writeQueues.get(sessionId) ?? Promise.resolve(); + const previousWrite = writeQueues.get(sessionID) ?? Promise.resolve(); const currentWrite = previousWrite.then(async () => { - await doAtomicWrite(sessionId, state); + await doAtomicWrite(sessionID, state); }); - writeQueues.set(sessionId, currentWrite); + writeQueues.set(sessionID, currentWrite); return currentWrite; } async function doAtomicWrite( - sessionId: string, + sessionID: string, state: ActiveRulesState ): Promise { const stateDir = resolveStateDir(); - const finalPath = getStateFilePath(sessionId); + const finalPath = getStateFilePath(sessionID); const tempPath = path.join( stateDir, - `.${sessionId}-${crypto.randomBytes(8).toString('hex')}.tmp` + `.${sessionID}-${crypto.randomBytes(8).toString('hex')}.tmp` ); try { @@ -94,7 +94,7 @@ async function doAtomicWrite( await fs.rename(tempPath, finalPath); } catch (error) { debugLog( - `Failed to write active rules state for session ${sessionId}: ${error}` + `Failed to write active rules state for session ${sessionID}: ${error}` ); // Clean up temp file if it exists @@ -107,28 +107,28 @@ async function doAtomicWrite( } export async function readActiveRulesState( - sessionId: string + sessionID: string ): Promise { - if (!isValidSessionId(sessionId)) { - debugLog(`Invalid sessionId rejected: ${sessionId}`); + if (!isValidSessionId(sessionID)) { + debugLog(`Invalid sessionID rejected: ${sessionID}`); return null; } - const filePath = getStateFilePath(sessionId); + const filePath = getStateFilePath(sessionID); try { const content = await fs.readFile(filePath, 'utf-8'); const parsed: unknown = JSON.parse(content); if (!isValidActiveRulesState(parsed)) { - debugLog(`Invalid active rules state format for session ${sessionId}`); + debugLog(`Invalid active rules state format for session ${sessionID}`); return null; } return parsed; } catch (error) { debugLog( - `Failed to read active rules state for session ${sessionId}: ${error}` + `Failed to read active rules state for session ${sessionID}: ${error}` ); return null; } @@ -141,7 +141,7 @@ function isValidActiveRulesState(value: unknown): value is ActiveRulesState { const obj = value as Record; - if (typeof obj['sessionId'] !== 'string') { + if (typeof obj['sessionID'] !== 'string') { return false; } From c61448dc1e5c03a90ae9479bc52a51b1978698e6 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:10:55 +0000 Subject: [PATCH 17/31] style(tui): add .js extension to cross-package imports from src --- tui/data/rules.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tui/data/rules.ts b/tui/data/rules.ts index 42cd823..66e329c 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -1,7 +1,7 @@ // tui/data/rules.ts -import { discoverRuleFiles, getCachedRule } from '../../src/rule-discovery'; -import type { RuleMetadata } from '../../src/rule-metadata'; -import { readActiveRulesState } from '../../src/active-rules-state'; +import { discoverRuleFiles, getCachedRule } from '../../src/rule-discovery.js'; +import type { RuleMetadata } from '../../src/rule-metadata.js'; +import { readActiveRulesState } from '../../src/active-rules-state.js'; import path from 'path'; /** Represents a rule as displayed in the sidebar */ From c3f58c0c8a2cb466d2a8121daf0491f691695251 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:12:08 +0000 Subject: [PATCH 18/31] docs(utils): clarify barrel file as intentional public API facade --- src/utils.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 9bc2d46..dcb4306 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,12 +1,17 @@ /** - * Utility functions for OpenCode Rules Plugin + * Stable public API surface for OpenCode Rules Plugin. * - * This module serves as a compatibility facade that re-exports - * from focused modules: + * This barrel file intentionally re-exports the subset of modules + * that external consumers (plugins, TUI, tests) should depend on. + * It isolates consumers from internal module restructuring and + * provides a single import point for the plugin's public surface. + * + * Re-exported modules: * - rule-discovery.ts: File discovery and caching * - rule-metadata.ts: Frontmatter parsing * - rule-filter.ts: Rule filtering and formatting * - message-paths.ts: Message path extraction + * - rule-hooks.ts: Hook evaluation and serialization */ // Re-export from rule-discovery From ee0ffec4f772c3daa8636aefa942b9ed14f02f06 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:18:06 +0000 Subject: [PATCH 19/31] refactor: extract shared hasConditions to rule-metadata and deduplicate callers --- src/rule-filter.ts | 16 +++------------- src/rule-metadata.ts | 19 +++++++++++++++++++ src/utils.ts | 4 ++-- tui/data/rules.ts | 21 ++------------------- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 8cbae1b..7841910 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -5,6 +5,7 @@ import { minimatch } from 'minimatch'; import { createDebugLog } from './debug.js'; import { getCachedRule, type DiscoveredRule } from './rule-discovery.js'; +import { hasConditions } from './utils.js'; const debugLog = createDebugLog(); @@ -120,20 +121,9 @@ export async function readAndFormatRules( const { metadata, strippedContent } = cachedRule; // Check if rule has any conditional filters - const hasConditions = Boolean( - metadata?.globs || - metadata?.keywords || - metadata?.tools || - metadata?.model || - metadata?.agent || - metadata?.command || - metadata?.project || - metadata?.branch || - metadata?.os || - metadata?.ci !== undefined - ); + const ruleHasConditions = hasConditions(metadata); - if (hasConditions && metadata) { + if (ruleHasConditions && metadata) { // Compute per-dimension match booleans (only for declared conditions) const declaredChecks: boolean[] = []; diff --git a/src/rule-metadata.ts b/src/rule-metadata.ts index f3a7859..573f78c 100644 --- a/src/rule-metadata.ts +++ b/src/rule-metadata.ts @@ -188,3 +188,22 @@ export function stripFrontmatter(content: string): string { // Return content after the closing marker, trimming leading newline return content.substring(endIndex + 3).trimStart(); } + +/** + * Check if metadata has any conditional fields set. + */ +export function hasConditions(meta: RuleMetadata | null | undefined): boolean { + if (!meta) return false; + return !!( + meta.globs || + meta.keywords || + meta.tools || + meta.model || + meta.agent || + meta.command || + meta.project || + meta.branch || + meta.os || + meta.ci !== undefined + ); +} diff --git a/src/utils.ts b/src/utils.ts index dcb4306..f5b6dac 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -21,8 +21,8 @@ export { type DiscoveredRule, } from './rule-discovery.js'; -// Re-export from rule-metadata (RuleMetadata is internal, not re-exported) -export { parseRuleMetadata } from './rule-metadata.js'; +// Re-export from rule-metadata +export { parseRuleMetadata, hasConditions } from './rule-metadata.js'; // Re-export from rule-filter export { diff --git a/tui/data/rules.ts b/tui/data/rules.ts index 66e329c..a92b29c 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -2,6 +2,7 @@ import { discoverRuleFiles, getCachedRule } from '../../src/rule-discovery.js'; import type { RuleMetadata } from '../../src/rule-metadata.js'; import { readActiveRulesState } from '../../src/active-rules-state.js'; +import { hasConditions } from '../../src/utils.js'; import path from 'path'; /** Represents a rule as displayed in the sidebar */ @@ -128,25 +129,7 @@ export function ruleSource( } /** - * Check if metadata has any conditional fields set. - */ -export function hasConditions(meta: RuleMetadata | null | undefined): boolean { - if (!meta) return false; - return !!( - meta.globs || - meta.keywords || - meta.tools || - meta.model || - meta.agent || - meta.command || - meta.project || - meta.branch || - meta.os || - meta.ci !== undefined - ); -} - -/** + * Format a concise summary of which conditions a rule has. * Build a human-readable, comma-separated summary of active conditions. * E.g., "globs: src/*.ts, keywords: auth, security" */ From 88ecc753137e43a3705c5300427d4bec0be5ec7d Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:19:41 +0000 Subject: [PATCH 20/31] docs(utils): explicitly document internal modules not re-exported by facade --- src/utils.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index f5b6dac..2714914 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,12 +1,22 @@ /** * Stable public API surface for OpenCode Rules Plugin. * - * This barrel file intentionally re-exports the subset of modules + * This barrel file intentionally re-exports a focused subset of modules * that external consumers (plugins, TUI, tests) should depend on. - * It isolates consumers from internal module restructuring and - * provides a single import point for the plugin's public surface. + * It isolates consumers from internal module restructuring and provides + * a single import point for the plugin's public surface. * - * Re-exported modules: + * Modules intentionally NOT re-exported (internal implementation): + * - active-rules-state.ts: internal state persistence + * - debug.ts: internal logging utilities + * - message-context.ts: internal message helpers + * - mcp-tools.ts: internal MCP integration + * - runtime.ts: internal orchestration (entry point is index.ts) + * - runtime-chat.ts: internal chat hook handler + * - runtime-context.ts: internal filter context builder + * - session-store.ts: internal session state + * + * Re-exported public modules: * - rule-discovery.ts: File discovery and caching * - rule-metadata.ts: Frontmatter parsing * - rule-filter.ts: Rule filtering and formatting From 1be10506aae5c63545c9f19adebc4c2674091326 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:26:25 +0000 Subject: [PATCH 21/31] refactor: extend utils.ts facade for TUI needs and update cross-package imports --- src/api-surface.typecheck.ts | 5 ----- src/utils.ts | 11 +++++++++-- tui/data/rules.ts | 11 +++++++---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/api-surface.typecheck.ts b/src/api-surface.typecheck.ts index 35246a8..b3b20f9 100644 --- a/src/api-surface.typecheck.ts +++ b/src/api-surface.typecheck.ts @@ -20,12 +20,7 @@ import type { OpenCodeRulesRuntimeOptions } from './runtime.js'; // @ts-expect-error SessionStoreOptions is internal and should not be exported import type { SessionStoreOptions } from './session-store.js'; -// --- utils.ts: RuleMetadata should NOT be exported --- -// @ts-expect-error RuleMetadata is internal and should not be exported -import type { RuleMetadata } from './utils.js'; - // Suppress unused variable warnings for the type imports above void (0 as unknown as McpStatusMap); void (0 as unknown as OpenCodeRulesRuntimeOptions); void (0 as unknown as SessionStoreOptions); -void (0 as unknown as RuleMetadata); diff --git a/src/utils.ts b/src/utils.ts index 2714914..8044cb3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -7,7 +7,6 @@ * a single import point for the plugin's public surface. * * Modules intentionally NOT re-exported (internal implementation): - * - active-rules-state.ts: internal state persistence * - debug.ts: internal logging utilities * - message-context.ts: internal message helpers * - mcp-tools.ts: internal MCP integration @@ -27,12 +26,17 @@ // Re-export from rule-discovery export { discoverRuleFiles, + getCachedRule, clearRuleCache, type DiscoveredRule, } from './rule-discovery.js'; // Re-export from rule-metadata -export { parseRuleMetadata, hasConditions } from './rule-metadata.js'; +export { + parseRuleMetadata, + hasConditions, + type RuleMetadata, +} from './rule-metadata.js'; // Re-export from rule-filter export { @@ -50,6 +54,9 @@ export { type MessagePart, } from './message-paths.js'; +// Re-export from active-rules-state (needed by TUI and external consumers) +export { readActiveRulesState } from './active-rules-state.js'; + // Re-export from rule-hooks export { evaluateHooks, diff --git a/tui/data/rules.ts b/tui/data/rules.ts index a92b29c..f332504 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -1,8 +1,11 @@ // tui/data/rules.ts -import { discoverRuleFiles, getCachedRule } from '../../src/rule-discovery.js'; -import type { RuleMetadata } from '../../src/rule-metadata.js'; -import { readActiveRulesState } from '../../src/active-rules-state.js'; -import { hasConditions } from '../../src/utils.js'; +import { + discoverRuleFiles, + getCachedRule, + readActiveRulesState, + hasConditions, + type RuleMetadata, +} from '../../src/utils.js'; import path from 'path'; /** Represents a rule as displayed in the sidebar */ From f8c67c4368cc5fb2e1aeb7bc5e5d907eb693d510 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:27:49 +0000 Subject: [PATCH 22/31] fix(deps): remove duplicated @opentui/* and solid-js from devDependencies --- package.json | 3 --- 1 file changed, 3 deletions(-) diff --git a/package.json b/package.json index 8ccf0ba..9e264ec 100644 --- a/package.json +++ b/package.json @@ -72,15 +72,12 @@ "devDependencies": { "@opencode-ai/plugin": "^1.3.9", "@opencode-ai/sdk": "^1.3.9", - "@opentui/core": "^0.1.93", - "@opentui/solid": "^0.1.93", "@types/minimatch": "^5.1.2", "@types/node": "^20.19.30", "@typescript-eslint/eslint-plugin": "^6.21.0", "@typescript-eslint/parser": "^6.21.0", "eslint": "^8.57.1", "prettier": "^3.8.1", - "solid-js": "1.9.11", "typescript": "^5.9.3", "vitest": "^1.6.1" }, From b11642e4f2efc75da32faf0ca536231600c83dad Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:28:19 +0000 Subject: [PATCH 23/31] fix(deps): add @opentui/* as optional peerDependencies --- package.json | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 9e264ec..690c8a2 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,17 @@ }, "peerDependencies": { "@opencode-ai/plugin": "^1.3.7", - "@opencode-ai/sdk": "^1.3.7" + "@opencode-ai/sdk": "^1.3.7", + "@opentui/core": "^0.1.93", + "@opentui/solid": "^0.1.93" + }, + "peerDependenciesMeta": { + "@opentui/core": { + "optional": true + }, + "@opentui/solid": { + "optional": true + } }, "devDependencies": { "@opencode-ai/plugin": "^1.3.9", From 7f09415c2a7bf2fc65fbc92734459721466099b3 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:28:41 +0000 Subject: [PATCH 24/31] fix(deps): remove redundant @types/minimatch --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 690c8a2..f426821 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,6 @@ "devDependencies": { "@opencode-ai/plugin": "^1.3.9", "@opencode-ai/sdk": "^1.3.9", - "@types/minimatch": "^5.1.2", "@types/node": "^20.19.30", "@typescript-eslint/eslint-plugin": "^6.21.0", "@typescript-eslint/parser": "^6.21.0", From 797fb1e3196786d41074f48a78eba04b56170ce4 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:30:48 +0000 Subject: [PATCH 25/31] =?UTF-8?q?fix(deps):=20align=20@opentui=20versions?= =?UTF-8?q?=20in=20manifest=20with=20lock=20file=20(0.1.93=E2=86=920.1.97)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- package.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index f426821..5267ca9 100644 --- a/package.json +++ b/package.json @@ -68,8 +68,8 @@ "peerDependencies": { "@opencode-ai/plugin": "^1.3.7", "@opencode-ai/sdk": "^1.3.7", - "@opentui/core": "^0.1.93", - "@opentui/solid": "^0.1.93" + "@opentui/core": "^0.1.97", + "@opentui/solid": "^0.1.97" }, "peerDependenciesMeta": { "@opentui/core": { @@ -91,8 +91,8 @@ "vitest": "^1.6.1" }, "dependencies": { - "@opentui/core": "^0.1.93", - "@opentui/solid": "^0.1.93", + "@opentui/core": "^0.1.97", + "@opentui/solid": "^0.1.97", "minimatch": "^9.0.5", "solid-js": "1.9.11", "yaml": "^2.8.2" From ba14f11d2bff618f7708cf35cf6c4136a680bbdb Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:32:23 +0000 Subject: [PATCH 26/31] refactor(rule-filter): extract repeated condition checks into evaluateConditionChecks helper --- src/rule-filter.ts | 208 +++++++++++++++++++++++++-------------------- 1 file changed, 114 insertions(+), 94 deletions(-) diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 7841910..41b207b 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -57,6 +57,115 @@ export function toolsMatchAvailable( return requiredTools.some(tool => availableSet.has(tool)); } +/** + * Evaluate all declared condition checks for a rule against runtime context. + * Returns an array of boolean match results (one per declared condition). + */ +function evaluateConditionChecks( + metadata: { + globs?: string[]; + keywords?: string[]; + tools?: string[]; + model?: string[]; + agent?: string[]; + command?: string[]; + project?: string[]; + branch?: string[]; + os?: string[]; + ci?: boolean; + }, + context: RuleFilterContext, + availableToolSet?: Set +): boolean[] { + const checks: boolean[] = []; + + if (metadata.globs) { + checks.push( + Boolean( + context.contextFilePaths && + context.contextFilePaths.length > 0 && + context.contextFilePaths.some(contextPath => + fileMatchesGlobs(contextPath, metadata.globs!) + ) + ) + ); + } + + if (metadata.keywords) { + checks.push( + Boolean( + context.userPrompt && + promptMatchesKeywords(context.userPrompt, metadata.keywords) + ) + ); + } + + if (metadata.tools) { + checks.push( + Boolean( + availableToolSet && + metadata.tools.some(tool => availableToolSet.has(tool)) + ) + ); + } + + if (metadata.model) { + checks.push( + Boolean(context.modelID && metadata.model.includes(context.modelID)) + ); + } + + if (metadata.agent) { + checks.push( + Boolean(context.agentType && metadata.agent.includes(context.agentType)) + ); + } + + if (metadata.command) { + checks.push( + Boolean(context.command && metadata.command.includes(context.command)) + ); + } + + if (metadata.project) { + const projectTags = context.projectTags; + checks.push( + Boolean( + projectTags && + projectTags.length > 0 && + metadata.project.some(tag => projectTags.includes(tag)) + ) + ); + } + + if (metadata.branch) { + const gitBranch = context.gitBranch; + checks.push( + Boolean( + gitBranch && + metadata.branch.some(pattern => { + if (pattern === gitBranch) return true; + const hasGlobChars = /[*?\[{]/.test(pattern); + if (hasGlobChars) { + return minimatch(gitBranch, pattern); + } + return false; + }) + ) + ); + } + + if (metadata.os) { + checks.push(Boolean(context.os && metadata.os.includes(context.os))); + } + + if (metadata.ci !== undefined) { + checks.push(context.ci === metadata.ci); + } + + return checks; +} + /** * Result of reading and formatting rules */ @@ -124,101 +233,12 @@ export async function readAndFormatRules( const ruleHasConditions = hasConditions(metadata); if (ruleHasConditions && metadata) { - // Compute per-dimension match booleans (only for declared conditions) - const declaredChecks: boolean[] = []; - - // Legacy: globs - if (metadata.globs) { - const globs = metadata.globs; - const globsMatch = - context.contextFilePaths && - context.contextFilePaths.length > 0 && - context.contextFilePaths.some(contextPath => - fileMatchesGlobs(contextPath, globs) - ); - declaredChecks.push(Boolean(globsMatch)); - } - - // Legacy: keywords - if (metadata.keywords) { - const keywordsMatch = - context.userPrompt && - promptMatchesKeywords(context.userPrompt, metadata.keywords); - declaredChecks.push(Boolean(keywordsMatch)); - } - - // Legacy: tools - if (metadata.tools) { - const toolsMatch = - availableToolSet && - metadata.tools.some(tool => availableToolSet.has(tool)); - declaredChecks.push(Boolean(toolsMatch)); - } - - // New: model - if (metadata.model) { - const modelMatch = - context.modelID && metadata.model.includes(context.modelID); - declaredChecks.push(Boolean(modelMatch)); - } - - // New: agent - if (metadata.agent) { - const agentMatch = - context.agentType && metadata.agent.includes(context.agentType); - declaredChecks.push(Boolean(agentMatch)); - } - - // New: command - if (metadata.command) { - const commandMatch = - context.command && metadata.command.includes(context.command); - declaredChecks.push(Boolean(commandMatch)); - } - - // New: project - if (metadata.project) { - const projectTags = context.projectTags; - const projectMatch = - projectTags && - projectTags.length > 0 && - metadata.project.some(tag => projectTags.includes(tag)); - declaredChecks.push(Boolean(projectMatch)); - } - - // New: branch (supports glob patterns) - if (metadata.branch) { - const gitBranch = context.gitBranch; - const branchMatch = - gitBranch && - metadata.branch.some(pattern => { - // Exact match for non-glob patterns - if (pattern === gitBranch) { - return true; - } - // Only use glob matching if pattern contains glob characters - const hasGlobChars = /[*?\[{]/.test(pattern); - if (hasGlobChars) { - return minimatch(gitBranch, pattern); - } - return false; - }); - declaredChecks.push(Boolean(branchMatch)); - } - - // New: os - if (metadata.os) { - const osMatch = context.os && metadata.os.includes(context.os); - declaredChecks.push(Boolean(osMatch)); - } - - // New: ci (strict boolean equality) - if (metadata.ci !== undefined) { - const ciMatch = context.ci === metadata.ci; - declaredChecks.push(ciMatch); - } + const declaredChecks = evaluateConditionChecks( + metadata, + context, + availableToolSet + ); - // Apply combinator: default 'any', or 'all' if specified const mode = metadata.match ?? 'any'; const shouldInclude = mode === 'all' From 387f1f92ace5eeb8fa3564ec124965dc16afc4fb Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 19:34:35 +0000 Subject: [PATCH 27/31] refactor(tui): consolidate loadRules and deduplicate metadata display in SidebarContent --- tui/slots/sidebar-content.tsx | 122 ++++++++++++---------------------- 1 file changed, 44 insertions(+), 78 deletions(-) diff --git a/tui/slots/sidebar-content.tsx b/tui/slots/sidebar-content.tsx index cef1135..404f1c0 100644 --- a/tui/slots/sidebar-content.tsx +++ b/tui/slots/sidebar-content.tsx @@ -11,6 +11,22 @@ import { } from 'solid-js'; import type { TuiPluginApi, TuiTheme } from '@opencode-ai/plugin/tui'; import { loadSidebarRules, type SidebarRuleEntry } from '../data/rules'; +import type { RuleMetadata } from '../../src/rule-metadata.js'; + +const metadataFieldDescriptors: Array<{ + key: keyof RuleMetadata; + label: string; +}> = [ + { key: 'globs', label: 'Globs' }, + { key: 'keywords', label: 'Keywords' }, + { key: 'tools', label: 'Tools' }, + { key: 'model', label: 'Model' }, + { key: 'agent', label: 'Agent' }, + { key: 'command', label: 'Command' }, + { key: 'project', label: 'Project' }, + { key: 'branch', label: 'Branch' }, + { key: 'os', label: 'OS' }, +]; interface SidebarContentProps { sessionId: string; @@ -86,51 +102,19 @@ function RuleSection(props: RuleSectionProps): JSX.Element { {rule.path} - 0}> - - Globs: {rule.metadata.globs!.join(', ')} - - - 0}> - - Keywords: {rule.metadata.keywords!.join(', ')} - - - 0}> - - Tools: {rule.metadata.tools!.join(', ')} - - - 0}> - - Model: {rule.metadata.model!.join(', ')} - - - 0}> - - Agent: {rule.metadata.agent!.join(', ')} - - - 0}> - - Command: {rule.metadata.command!.join(', ')} - - - 0}> - - Project: {rule.metadata.project!.join(', ')} - - - 0}> - - Branch: {rule.metadata.branch!.join(', ')} - - - 0}> - - OS: {rule.metadata.os!.join(', ')} - - + + {({ key, label }) => { + const value = rule.metadata[key]; + if (Array.isArray(value) && value.length > 0) { + return ( + + {label}: {value.join(', ')} + + ); + } + return null; + }} + CI: {String(rule.metadata.ci)} @@ -182,40 +166,19 @@ export function SidebarContent(props: SidebarContentProps): JSX.Element { // Debounce timer for event-driven refresh let debounceTimer: ReturnType | null = null; - // Initial load: triggered by session/directory change, resets all UI state - const loadRulesInitial = async (): Promise => { + async function loadRules(options: { resetUi?: boolean } = {}): Promise { const thisRequest = ++requestId; const dir = resolveProjectDir(); const sessionId = props.sessionId; - setLastDir(dir); - setLastSessionId(sessionId); - setStatus('loading'); - - try { - const result = await loadSidebarRules(dir, sessionId); - // Discard if a newer request started - if (requestId !== thisRequest) return; - setRules(result.rules); - setSkippedCount(result.skippedCount); - setHasEvaluationState(result.hasEvaluationState); - setStatus('loaded'); - } catch (err) { - // Discard if a newer request started - if (requestId !== thisRequest) return; - console.error('[opencode-rules] Failed to load rules:', err); - setStatus('error'); + if (options.resetUi) { + setLastDir(dir); + setLastSessionId(sessionId); + setStatus('loading'); + } else if (status() === 'loading') { + // Skip refresh if initial load is still in flight + return; } - }; - - // Refresh load: triggered by events, only updates rule data (no UI state reset) - const loadRulesRefresh = async (): Promise => { - // Skip refresh if initial load is still in flight - if (status() === 'loading') return; - - const thisRequest = ++requestId; - const dir = resolveProjectDir(); - const sessionId = props.sessionId; try { const result = await loadSidebarRules(dir, sessionId); @@ -228,9 +191,12 @@ export function SidebarContent(props: SidebarContentProps): JSX.Element { } catch (err) { // Discard if a newer request started if (requestId !== thisRequest) return; - console.error('[opencode-rules] Failed to refresh rules:', err); + console.error('[opencode-rules] Failed to load rules:', err); + if (options.resetUi) { + setStatus('error'); + } } - }; + } // Effect 1: Initial load on session/directory change createEffect(() => { @@ -248,7 +214,7 @@ export function SidebarContent(props: SidebarContentProps): JSX.Element { setExpandedIndex(null); setProjectOpen(false); setGlobalOpen(false); - void loadRulesInitial(); + void loadRules({ resetUi: true }); } }); @@ -256,7 +222,7 @@ export function SidebarContent(props: SidebarContentProps): JSX.Element { createEffect(() => { const counter = refreshCounter(); if (counter > 0) { - void loadRulesRefresh(); + void loadRules(); } }); From 11b23f4c5942a55cdee2c586cf7567323dec5eef Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 20:12:49 +0000 Subject: [PATCH 28/31] fix: improve error consistency, fix cross-module imports, and update tests --- src/active-rules-state.test.ts | 7 ++++--- src/active-rules-state.ts | 10 +++++----- src/git-branch.ts | 5 ++++- src/index.runtime.test.ts | 7 +++++-- src/rule-filter.ts | 2 +- src/runtime-context.ts | 6 ++++-- tui/data/rules.ts | 1 + 7 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index 54b6f16..a1283e5 100644 --- a/src/active-rules-state.test.ts +++ b/src/active-rules-state.test.ts @@ -162,9 +162,10 @@ describe('active-rules-state', () => { ); }); - it('returns null for read with invalid sessionID', async () => { - const state = await readActiveRulesState('../escape'); - expect(state).toBeNull(); + it('throws for read with invalid sessionID', async () => { + await expect(readActiveRulesState('../escape')).rejects.toThrow( + 'Invalid sessionID' + ); }); it('no temp file remains after write', async () => { diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index 0fa5a6a..29b31ec 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -2,7 +2,7 @@ import * as fs from 'node:fs/promises'; import * as os from 'node:os'; import * as path from 'node:path'; import * as crypto from 'node:crypto'; -import { createDebugLog } from './debug.js'; +import { createDebugLog, logWarning } from './debug.js'; const debugLog = createDebugLog(); @@ -93,8 +93,9 @@ async function doAtomicWrite( await fs.writeFile(tempPath, content, 'utf-8'); await fs.rename(tempPath, finalPath); } catch (error) { - debugLog( - `Failed to write active rules state for session ${sessionID}: ${error}` + logWarning( + `Failed to write active rules state for session ${sessionID}`, + error ); // Clean up temp file if it exists @@ -110,8 +111,7 @@ export async function readActiveRulesState( sessionID: string ): Promise { if (!isValidSessionId(sessionID)) { - debugLog(`Invalid sessionID rejected: ${sessionID}`); - return null; + throw new Error(`Invalid sessionID: ${sessionID}`); } const filePath = getStateFilePath(sessionID); diff --git a/src/git-branch.ts b/src/git-branch.ts index 3f70ac4..66b4b8c 100644 --- a/src/git-branch.ts +++ b/src/git-branch.ts @@ -1,5 +1,7 @@ import { execFile, type ExecFileOptions } from 'node:child_process'; +import { createDebugLog } from './debug.js'; +const debugLog = createDebugLog(); const GIT_TIMEOUT_MS = 5000; export async function getGitBranch(projectDir: string): Promise { @@ -29,7 +31,8 @@ export async function getGitBranch(projectDir: string): Promise { ); }); return branch; - } catch { + } catch (err) { + debugLog(`Failed to get git branch: ${err}`); return null; } } diff --git a/src/index.runtime.test.ts b/src/index.runtime.test.ts index 2628b5f..b178530 100644 --- a/src/index.runtime.test.ts +++ b/src/index.runtime.test.ts @@ -1006,7 +1006,7 @@ describe('Active rules state persistence', () => { const state = await readActiveRulesState(sessionID); expect(state).not.toBeNull(); - expect(state?.sessionId).toBe(sessionID); + expect(state?.sessionID).toBe(sessionID); expect(state?.matchedRulePaths).toHaveLength(1); expect(state?.matchedRulePaths[0]).toBe(rulePath); }); @@ -1052,7 +1052,7 @@ Conditional rule for gpt-5 only.` const state = await readActiveRulesState(sessionID); expect(state).not.toBeNull(); - expect(state?.sessionId).toBe(sessionID); + expect(state?.sessionID).toBe(sessionID); expect(state?.matchedRulePaths).toHaveLength(0); }); @@ -1095,8 +1095,11 @@ describe('utils runtime exports', () => { 'discoverRuleFiles', 'evaluateHooks', 'extractFilePathsFromMessages', + 'getCachedRule', + 'hasConditions', 'parseRuleMetadata', 'promptMatchesKeywords', + 'readActiveRulesState', 'readAndFormatRules', 'serializeToolArgs', 'toolsMatchAvailable', diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 41b207b..852673c 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -5,7 +5,7 @@ import { minimatch } from 'minimatch'; import { createDebugLog } from './debug.js'; import { getCachedRule, type DiscoveredRule } from './rule-discovery.js'; -import { hasConditions } from './utils.js'; +import { hasConditions } from './rule-metadata.js'; const debugLog = createDebugLog(); diff --git a/src/runtime-context.ts b/src/runtime-context.ts index fc3d551..edbc00b 100644 --- a/src/runtime-context.ts +++ b/src/runtime-context.ts @@ -89,14 +89,16 @@ export async function buildFilterContext( if (projectTags.length === 0) { projectTags = undefined; } - } catch { + } catch (error) { + debugLog(`Failed to detect project tags: ${error}`); projectTags = undefined; } let gitBranch: string | null = null; try { gitBranch = await getGitBranch(projectDirectory); - } catch { + } catch (error) { + debugLog(`Failed to get git branch: ${error}`); gitBranch = null; } diff --git a/tui/data/rules.ts b/tui/data/rules.ts index f332504..1633108 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -6,6 +6,7 @@ import { hasConditions, type RuleMetadata, } from '../../src/utils.js'; +export { hasConditions }; import path from 'path'; /** Represents a rule as displayed in the sidebar */ From baa2d924895dd82020f5d8c3c0b3d5982dc8d998 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sat, 25 Apr 2026 20:21:55 +0000 Subject: [PATCH 29/31] refactor: fix convention drift, naming, and AI debt across 10 files --- src/active-rules-state.ts | 11 ++++++---- src/index.runtime.test.ts | 8 ++++--- src/mcp-tools.ts | 6 ++--- src/message-context.test.ts | 16 +++++++------- src/message-context.ts | 18 ++++++++------- src/rule-discovery.ts | 2 +- src/rule-filter.ts | 13 +---------- src/rule-metadata.ts | 5 ----- src/runtime-chat.ts | 4 ++-- src/runtime-context.ts | 36 +++++++++--------------------- src/runtime.tool-ids.test.ts | 8 ++++--- src/runtime.ts | 41 +++++++++++++++++++---------------- tui/data/rules.test.ts | 40 +++++++++++++++++++--------------- tui/data/rules.ts | 10 ++++----- tui/index.tsx | 2 +- tui/slots/sidebar-content.tsx | 4 ++-- 16 files changed, 105 insertions(+), 119 deletions(-) diff --git a/src/active-rules-state.ts b/src/active-rules-state.ts index 29b31ec..146747b 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -21,7 +21,7 @@ let stateDirOverride: string | null = null; // Strict pattern for safe sessionID: alphanumeric, underscore, hyphen only const SAFE_SESSION_ID_PATTERN = /^[A-Za-z0-9_-]+$/; -function isValidSessionId(sessionID: string): boolean { +function isValidSessionID(sessionID: string): boolean { return SAFE_SESSION_ID_PATTERN.test(sessionID); } @@ -42,18 +42,20 @@ export function resolveStateDir(): string { return path.join(os.homedir(), '.opencode', 'state', 'opencode-rules'); } +/** @throws {Error} If sessionID fails validation. */ export function getStateFilePath(sessionID: string): string { - if (!isValidSessionId(sessionID)) { + if (!isValidSessionID(sessionID)) { throw new Error(`Invalid sessionID: ${sessionID}`); } return path.join(resolveStateDir(), `${sessionID}.json`); } +/** Write matched rule paths to state. @throws {Error} If sessionID fails validation. */ export function writeActiveRulesState( sessionID: string, matchedPaths: string[] ): Promise { - if (!isValidSessionId(sessionID)) { + if (!isValidSessionID(sessionID)) { throw new Error(`Invalid sessionID: ${sessionID}`); } @@ -107,10 +109,11 @@ async function doAtomicWrite( } } +/** Read active rules state. @throws {Error} If sessionID fails validation. */ export async function readActiveRulesState( sessionID: string ): Promise { - if (!isValidSessionId(sessionID)) { + if (!isValidSessionID(sessionID)) { throw new Error(`Invalid sessionID: ${sessionID}`); } diff --git a/src/index.runtime.test.ts b/src/index.runtime.test.ts index b178530..25dd08f 100644 --- a/src/index.runtime.test.ts +++ b/src/index.runtime.test.ts @@ -123,9 +123,11 @@ describe('module boundary tests', () => { expect(typeof runtimeContextModule.detectCiEnvironment).toBe('function'); }); - it('should export handleChatMessage from runtime-chat module', () => { - expect(runtimeChatModule.handleChatMessage).toBeDefined(); - expect(typeof runtimeChatModule.handleChatMessage).toBe('function'); + it('should export updateSessionFromChatMessage from runtime-chat module', () => { + expect(runtimeChatModule.updateSessionFromChatMessage).toBeDefined(); + expect(typeof runtimeChatModule.updateSessionFromChatMessage).toBe( + 'function' + ); }); it('should detect CI environment correctly via runtime-context module', () => { diff --git a/src/mcp-tools.ts b/src/mcp-tools.ts index 327ea66..ee8e7c6 100644 --- a/src/mcp-tools.ts +++ b/src/mcp-tools.ts @@ -11,14 +11,14 @@ export function extractConnectedMcpCapabilityIDs( ): string[] { if (!status || typeof status !== 'object' || Array.isArray(status)) return []; - const out: string[] = []; + const capabilityIDs: string[] = []; for (const [clientName, clientStatus] of Object.entries(status)) { if (clientStatus?.status === 'connected') { const sanitized = sanitizeMcpClientName(clientName); if (sanitized) { - out.push(`mcp_${sanitized}`); + capabilityIDs.push(`mcp_${sanitized}`); } } } - return out; + return capabilityIDs; } diff --git a/src/message-context.test.ts b/src/message-context.test.ts index 4048d43..cdea005 100644 --- a/src/message-context.test.ts +++ b/src/message-context.test.ts @@ -4,7 +4,7 @@ import { sanitizePathForContext, extractLatestUserPrompt, extractSessionID, - toExtractableMessages, + filterValidMessages, extractSlashCommand, extractTextFromParts, MessageWithInfo, @@ -35,12 +35,12 @@ describe('message-context', () => { }); }); -describe('toExtractableMessages', () => { +describe('filterValidMessages', () => { it('passes through messages with role and parts', () => { const messages: MessageWithInfo[] = [ { role: 'user', parts: [{ type: 'text', text: 'hello' }] }, ]; - const result = toExtractableMessages(messages); + const result = filterValidMessages(messages); expect(result).toEqual([ { role: 'user', parts: [{ type: 'text', text: 'hello' }] }, ]); @@ -50,17 +50,17 @@ describe('toExtractableMessages', () => { const messages: MessageWithInfo[] = [ { parts: [{ type: 'text', text: 'hello' }] }, ]; - expect(toExtractableMessages(messages)).toEqual([]); + expect(filterValidMessages(messages)).toEqual([]); }); it('filters out messages with missing parts', () => { const messages: MessageWithInfo[] = [{ role: 'user' }]; - expect(toExtractableMessages(messages)).toEqual([]); + expect(filterValidMessages(messages)).toEqual([]); }); it('filters out messages with empty parts array', () => { const messages: MessageWithInfo[] = [{ role: 'user', parts: [] }]; - expect(toExtractableMessages(messages)).toEqual([]); + expect(filterValidMessages(messages)).toEqual([]); }); it('handles mixed valid and invalid messages', () => { @@ -70,14 +70,14 @@ describe('toExtractableMessages', () => { { parts: [{ type: 'text', text: 'x' }] }, { role: 'user', parts: [{ type: 'text', text: 'bye' }] }, ]; - const result = toExtractableMessages(messages); + const result = filterValidMessages(messages); expect(result).toHaveLength(2); expect(result[0].role).toBe('user'); expect(result[1].role).toBe('user'); }); it('returns empty array for empty input', () => { - expect(toExtractableMessages([])).toEqual([]); + expect(filterValidMessages([])).toEqual([]); }); }); diff --git a/src/message-context.ts b/src/message-context.ts index 33c976b..e9d1b5b 100644 --- a/src/message-context.ts +++ b/src/message-context.ts @@ -47,18 +47,20 @@ export function extractTextFromParts( * If path is absolute and under baseDir, convert to relative POSIX path. * Otherwise return path as-is. */ -export function normalizeContextPath(p: string, baseDir: string): string { - if (!path.isAbsolute(p)) return p; - const rel = path.relative(baseDir, p); +export function normalizeContextPath( + filePath: string, + baseDir: string +): string { + if (!path.isAbsolute(filePath)) return filePath; + const rel = path.relative(baseDir, filePath); return rel.split(path.sep).join('/'); } /** - * Sanitize a file path for safe inclusion in context strings. - * Prevents prompt injection by removing control characters and limiting length. + * Strip control characters and limit length for safe inclusion in context strings. */ -export function sanitizePathForContext(p: string): string { - return p.replace(/[\r\n\t]/g, ' ').slice(0, 300); +export function sanitizePathForContext(filePath: string): string { + return filePath.replace(/[\r\n\t]/g, ' ').slice(0, 300); } /** @@ -106,7 +108,7 @@ export function extractLatestUserPrompt( * Convert MessageWithInfo[] to Message[] by filtering out messages * that lack required fields (role, non-empty parts array). */ -export function toExtractableMessages(messages: MessageWithInfo[]): Message[] { +export function filterValidMessages(messages: MessageWithInfo[]): Message[] { const result: Message[] = []; for (const msg of messages) { if ( diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index 50fca94..b766bec 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -45,7 +45,7 @@ export function clearRuleCache(): void { * Uses mtime-based invalidation to detect file changes. * * @param filePath - Absolute path to the rule file - * @returns Cached rule data or undefined if file cannot be read + * @returns Cached rule data or null if file cannot be read */ export async function getCachedRule( filePath: string diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 852673c..5ac0012 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -40,19 +40,11 @@ export function promptMatchesKeywords( }); } -/** - * Check if any of the required tools are available. - * Uses exact string matching (OR logic: any match returns true). - * - * @param availableToolIDs - Array of tool IDs currently available - * @param requiredTools - Array of tool IDs from rule metadata - * @returns true if any required tool is available - */ +/** Check if any required tool is in the available set. */ export function toolsMatchAvailable( availableToolIDs: string[], requiredTools: string[] ): boolean { - // Create a Set for O(1) lookups const availableSet = new Set(availableToolIDs); return requiredTools.some(tool => availableSet.has(tool)); } @@ -229,7 +221,6 @@ export async function readAndFormatRules( const { metadata, strippedContent } = cachedRule; - // Check if rule has any conditional filters const ruleHasConditions = hasConditions(metadata); if (ruleHasConditions && metadata) { @@ -257,8 +248,6 @@ export async function readAndFormatRules( ); } - // Use cached stripped content for output - // Use relativePath for unique headings instead of just filename ruleContents.push(`## ${relativePath}\n\n${strippedContent}`); matchedPaths.push(filePath); } diff --git a/src/rule-metadata.ts b/src/rule-metadata.ts index 573f78c..009eea8 100644 --- a/src/rule-metadata.ts +++ b/src/rule-metadata.ts @@ -161,10 +161,8 @@ export function parseRuleMetadata(content: string): RuleMetadata | null { } } - // Return metadata only if it has content return Object.keys(metadata).length > 0 ? metadata : null; } catch (error) { - // Log warning for YAML parsing errors logWarning('Failed to parse YAML frontmatter', error); return null; } @@ -174,18 +172,15 @@ export function parseRuleMetadata(content: string): RuleMetadata | null { * Strip YAML frontmatter from rule content */ export function stripFrontmatter(content: string): string { - // Check if content starts with frontmatter if (!content.startsWith('---')) { return content; } - // Find the closing --- marker const endIndex = content.indexOf('---', 3); if (endIndex === -1) { return content; } - // Return content after the closing marker, trimming leading newline return content.substring(endIndex + 3).trimStart(); } diff --git a/src/runtime-chat.ts b/src/runtime-chat.ts index 189ffcb..298403b 100644 --- a/src/runtime-chat.ts +++ b/src/runtime-chat.ts @@ -14,10 +14,10 @@ export interface ChatMessageOutput { } /** - * Handle incoming chat messages to update session state. + * Update session state from incoming chat message data. * Captures user prompts, model IDs, and agent types. */ -export function handleChatMessage( +export function updateSessionFromChatMessage( input: ChatMessageInput, output: ChatMessageOutput, sessionStore: SessionStore, diff --git a/src/runtime-context.ts b/src/runtime-context.ts index edbc00b..8cd563e 100644 --- a/src/runtime-context.ts +++ b/src/runtime-context.ts @@ -26,23 +26,7 @@ function parseEnvBoolean(value: string | undefined): boolean | undefined { return true; } -/** - * Check if a string value represents a truthy CI environment variable. - * Treats 'false', '0', and empty strings as falsy; other non-empty values as truthy. - */ -function isTruthyEnvValue(value: string | undefined): boolean { - return parseEnvBoolean(value) === true; -} - -/** - * Detect if running in a CI environment by checking common CI environment variables. - * - * If process.env.CI is explicitly set, it is treated as authoritative: - * - CI='false' or CI='0' or CI='' => return false (no provider var fallback) - * - CI='true' or CI='1' or any truthy value => return true - * - * If process.env.CI is not set (undefined), fall back to provider-specific detection. - */ +/** Detect if running in a CI environment by checking common CI environment variables. */ export function detectCiEnvironment(): boolean { const env = process.env; @@ -52,15 +36,15 @@ export function detectCiEnvironment(): boolean { } return ( - isTruthyEnvValue(env.CONTINUOUS_INTEGRATION) || - isTruthyEnvValue(env.BUILD_NUMBER) || - isTruthyEnvValue(env.GITHUB_ACTIONS) || - isTruthyEnvValue(env.GITLAB_CI) || - isTruthyEnvValue(env.CIRCLECI) || - isTruthyEnvValue(env.TRAVIS) || - isTruthyEnvValue(env.JENKINS_URL) || - isTruthyEnvValue(env.BUILDKITE) || - isTruthyEnvValue(env.TEAMCITY_VERSION) + parseEnvBoolean(env.CONTINUOUS_INTEGRATION) === true || + parseEnvBoolean(env.BUILD_NUMBER) === true || + parseEnvBoolean(env.GITHUB_ACTIONS) === true || + parseEnvBoolean(env.GITLAB_CI) === true || + parseEnvBoolean(env.CIRCLECI) === true || + parseEnvBoolean(env.TRAVIS) === true || + parseEnvBoolean(env.JENKINS_URL) === true || + parseEnvBoolean(env.BUILDKITE) === true || + parseEnvBoolean(env.TEAMCITY_VERSION) === true ); } diff --git a/src/runtime.tool-ids.test.ts b/src/runtime.tool-ids.test.ts index 7f73614..3f08f55 100644 --- a/src/runtime.tool-ids.test.ts +++ b/src/runtime.tool-ids.test.ts @@ -23,9 +23,11 @@ describe('runtime module boundaries', () => { expect(typeof runtimeContextModule.detectCiEnvironment).toBe('function'); }); - it('exports handleChatMessage from runtime-chat module', () => { - expect(runtimeChatModule.handleChatMessage).toBeDefined(); - expect(typeof runtimeChatModule.handleChatMessage).toBe('function'); + it('exports updateSessionFromChatMessage from runtime-chat module', () => { + expect(runtimeChatModule.updateSessionFromChatMessage).toBeDefined(); + expect(typeof runtimeChatModule.updateSessionFromChatMessage).toBe( + 'function' + ); }); }); diff --git a/src/runtime.ts b/src/runtime.ts index d897b31..987684f 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -6,7 +6,7 @@ import { extractSessionID, normalizeContextPath, sanitizePathForContext, - toExtractableMessages, + filterValidMessages, type MessageWithInfo, } from './message-context.js'; import { extractConnectedMcpCapabilityIDs } from './mcp-tools.js'; @@ -14,7 +14,7 @@ import { createDebugLog, logWarning, type DebugLog } from './debug.js'; import type { SessionStore } from './session-store.js'; import { buildFilterContext } from './runtime-context.js'; import { - handleChatMessage, + updateSessionFromChatMessage, type ChatMessageInput, type ChatMessageOutput, } from './runtime-chat.js'; @@ -133,7 +133,6 @@ export class OpenCodeRulesRuntime { ); } - // Evaluate PreToolUse hooks await this.evaluateAndQueueHooks('PreToolUse', sessionID, toolName, args); } @@ -174,7 +173,7 @@ export class OpenCodeRulesRuntime { } const contextPaths = extractFilePathsFromMessages( - toExtractableMessages(output.messages) + filterValidMessages(output.messages) ); const userPrompt = extractLatestUserPrompt(output.messages); @@ -210,7 +209,12 @@ export class OpenCodeRulesRuntime { input: ChatMessageInput, output: ChatMessageOutput ): Promise { - handleChatMessage(input, output, this.sessionStore, this.debugLog); + updateSessionFromChatMessage( + input, + output, + this.sessionStore, + this.debugLog + ); } private async onSystemTransform( @@ -359,6 +363,17 @@ export class OpenCodeRulesRuntime { mcpPromise, ] as const); + const logSettledError = ( + label: string, + result: PromiseRejectedResult + ): void => { + const message = + result.reason instanceof Error + ? result.reason.message + : String(result.reason); + logWarning(`Failed to query ${label}`, message); + }; + if ( toolResult.status === 'fulfilled' && Array.isArray(toolResult.value?.data) @@ -370,13 +385,7 @@ export class OpenCodeRulesRuntime { `Built-in tools: ${toolResult.value.data.slice(0, 10).join(', ')}${toolResult.value.data.length > 10 ? '...' : ''} (${toolResult.value.data.length} total)` ); } else if (toolResult.status === 'rejected') { - const message = - toolResult.reason instanceof Error - ? toolResult.reason.message - : String(toolResult.reason); - console.warn( - `[opencode-rules] Warning: Failed to query tool IDs: ${message}` - ); + logSettledError('tool IDs', toolResult); } if ( @@ -394,13 +403,7 @@ export class OpenCodeRulesRuntime { this.debugLog(`MCP capability IDs: ${mcpIds.join(', ')}`); } } else if (mcpResult.status === 'rejected') { - const message = - mcpResult.reason instanceof Error - ? mcpResult.reason.message - : String(mcpResult.reason); - console.warn( - `[opencode-rules] Warning: Failed to query MCP status: ${message}` - ); + logSettledError('MCP status', mcpResult); } return Array.from(ids); diff --git a/tui/data/rules.test.ts b/tui/data/rules.test.ts index c274ff4..767792e 100644 --- a/tui/data/rules.test.ts +++ b/tui/data/rules.test.ts @@ -1,15 +1,21 @@ // tui/data/rules.test.ts import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import path from 'path'; -import os from 'os'; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync, chmodSync } from 'fs'; +import path from 'node:path'; +import os from 'node:os'; +import { + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync, + chmodSync, +} from 'node:fs'; import { clearRuleCache } from '../../src/rule-discovery.js'; import { _setStateDirForTesting, writeActiveRulesState, } from '../../src/active-rules-state.js'; import { - ruleSource, + classifyRuleScope, hasConditions, formatConditionSummary, disambiguateNames, @@ -18,38 +24,38 @@ import { } from './rules.js'; // ────────────────────────────────────────────── -// ruleSource +// classifyRuleScope // ────────────────────────────────────────────── -describe('ruleSource', () => { +describe('classifyRuleScope', () => { it('returns "global" when projectDir is null', () => { - expect(ruleSource('/home/user/.config/opencode/rules/foo.md', null)).toBe( - 'global' - ); + expect( + classifyRuleScope('/home/user/.config/opencode/rules/foo.md', null) + ).toBe('global'); }); it('returns "project" for files under projectDir/.opencode/rules/', () => { - expect(ruleSource('/project/.opencode/rules/foo.md', '/project')).toBe( - 'project' - ); + expect( + classifyRuleScope('/project/.opencode/rules/foo.md', '/project') + ).toBe('project'); }); it('returns "project" for files in subdirectories under project rules', () => { - expect(ruleSource('/project/.opencode/rules/sub/deep.md', '/project')).toBe( - 'project' - ); + expect( + classifyRuleScope('/project/.opencode/rules/sub/deep.md', '/project') + ).toBe('project'); }); it('returns "global" for files not under projectDir/.opencode/rules/', () => { expect( - ruleSource('/home/user/.config/opencode/rules/foo.md', '/project') + classifyRuleScope('/home/user/.config/opencode/rules/foo.md', '/project') ).toBe('global'); }); it('does not match partial path prefixes', () => { // /project/.opencode/rules-extra/ should NOT match /project/.opencode/rules/ expect( - ruleSource('/project/.opencode/rules-extra/foo.md', '/project') + classifyRuleScope('/project/.opencode/rules-extra/foo.md', '/project') ).toBe('global'); }); }); diff --git a/tui/data/rules.ts b/tui/data/rules.ts index 1633108..fd809df 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -7,7 +7,7 @@ import { type RuleMetadata, } from '../../src/utils.js'; export { hasConditions }; -import path from 'path'; +import path from 'node:path'; /** Represents a rule as displayed in the sidebar */ export interface SidebarRuleEntry { @@ -73,7 +73,7 @@ export async function loadSidebarRules( } const meta = cached.metadata; - const source = ruleSource(rule.filePath, projectDir); + const source = classifyRuleScope(rule.filePath, projectDir); const isConditional = hasConditions(meta); const conditionSummary = isConditional ? formatConditionSummary(meta!) @@ -103,11 +103,11 @@ export async function loadSidebarRules( disambiguateNames(entries); // Sort: project first, then global. Active rules to top, then alpha by name. - const activeOrder = (v: boolean | null): number => + const sortPriority = (v: boolean | null): number => v === true ? 0 : v === null ? 1 : 2; entries.sort((a, b) => { if (a.source !== b.source) return a.source === 'project' ? -1 : 1; - const activeCmp = activeOrder(a.isActive) - activeOrder(b.isActive); + const activeCmp = sortPriority(a.isActive) - sortPriority(b.isActive); if (activeCmp !== 0) return activeCmp; const nameCompare = a.name.localeCompare(b.name); if (nameCompare !== 0) return nameCompare; @@ -122,7 +122,7 @@ export async function loadSidebarRules( * Uses path.sep boundary check to avoid matching partial prefixes * (e.g., /project/.opencode/rules-extra/ should not match). */ -export function ruleSource( +export function classifyRuleScope( filePath: string, projectDir: string | null ): 'global' | 'project' { diff --git a/tui/index.tsx b/tui/index.tsx index 5364ceb..11a9818 100644 --- a/tui/index.tsx +++ b/tui/index.tsx @@ -1,7 +1,7 @@ // tui/index.tsx /** @jsxImportSource @opentui/solid */ import type { TuiPlugin } from '@opencode-ai/plugin/tui'; -import { SidebarContent } from './slots/sidebar-content'; +import { SidebarContent } from './slots/sidebar-content.js'; const id = 'opencode-rules' as const; diff --git a/tui/slots/sidebar-content.tsx b/tui/slots/sidebar-content.tsx index 404f1c0..36782b1 100644 --- a/tui/slots/sidebar-content.tsx +++ b/tui/slots/sidebar-content.tsx @@ -10,8 +10,8 @@ import { type JSX, } from 'solid-js'; import type { TuiPluginApi, TuiTheme } from '@opencode-ai/plugin/tui'; -import { loadSidebarRules, type SidebarRuleEntry } from '../data/rules'; -import type { RuleMetadata } from '../../src/rule-metadata.js'; +import { loadSidebarRules, type SidebarRuleEntry } from '../data/rules.js'; +import type { RuleMetadata } from '../../src/utils.js'; const metadataFieldDescriptors: Array<{ key: keyof RuleMetadata; From d5bee5c44fbf2e3ccea89f225108298972bb8657 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sun, 26 Apr 2026 18:52:28 +0000 Subject: [PATCH 30/31] refactor(error-consistency): add await to write call, fix type duplication, document exception contracts --- src/active-rules-state.test.ts | 8 ++++++-- src/rule-filter.ts | 14 ++------------ src/runtime.ts | 4 +++- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index a1283e5..230b490 100644 --- a/src/active-rules-state.test.ts +++ b/src/active-rules-state.test.ts @@ -206,8 +206,12 @@ describe('active-rules-state', () => { await writeActiveRulesState(sessionID, matchedPaths); - // Verify directory now exists - await expect(fs.access(testStateDir)).resolves.toBeUndefined(); + // Verify directory now exists — fs.access resolves to null on Bun, undefined on Node + const dirExists = await fs.access(testStateDir).then( + () => true, + () => false + ); + expect(dirExists).toBe(true); }); it('handles writes to different sessions independently', async () => { diff --git a/src/rule-filter.ts b/src/rule-filter.ts index 5ac0012..b1e6d88 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -6,6 +6,7 @@ import { minimatch } from 'minimatch'; import { createDebugLog } from './debug.js'; import { getCachedRule, type DiscoveredRule } from './rule-discovery.js'; import { hasConditions } from './rule-metadata.js'; +import type { RuleMetadata } from './rule-metadata.js'; const debugLog = createDebugLog(); @@ -54,18 +55,7 @@ export function toolsMatchAvailable( * Returns an array of boolean match results (one per declared condition). */ function evaluateConditionChecks( - metadata: { - globs?: string[]; - keywords?: string[]; - tools?: string[]; - model?: string[]; - agent?: string[]; - command?: string[]; - project?: string[]; - branch?: string[]; - os?: string[]; - ci?: boolean; - }, + metadata: RuleMetadata, context: RuleFilterContext, availableToolSet?: Set ): boolean[] { diff --git a/src/runtime.ts b/src/runtime.ts index 987684f..4ba16b0 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -294,7 +294,7 @@ export class OpenCodeRulesRuntime { formattedRules = result.formattedRules; if (sessionID) { - writeActiveRulesState(sessionID, result.matchedPaths); + await writeActiveRulesState(sessionID, result.matchedPaths); } } else { this.debugLog( @@ -472,6 +472,8 @@ export class OpenCodeRulesRuntime { } } + /** Evaluate hooks for a tool invocation and queue matches. + * @throws {Error} When a PreToolUse hook with block:true matches the tool and arguments. */ private async evaluateAndQueueHooks( hookType: 'PreToolUse' | 'PostToolUse', sessionID: string, From d22c78ccdb816e68d20101ad06b792ed6ed28e94 Mon Sep 17 00:00:00 2001 From: Joe Maples Date: Sun, 3 May 2026 13:14:03 -0400 Subject: [PATCH 31/31] fix(bun-compat): achieve 100% pass rate by eliminating vi.mock and adding dependency injection Bun's vi.mock() factory functions were poisoning the module cache globally, causing cascading failures across consecutive test runs: - vi.mock('node:fs/promises') broke readAndFormatRules in integration tests - vi.mock('node:child_process') caused hangs and timeouts - toHaveProperty('a.b.c') triggered Bun's dotted-key handling bug Solution: - git-branch.ts: add optional execFn param for DI - project-fingerprint.ts: add ProjectTagFs interface + nodeFs adapter - git-branch.test.ts: pass inline vi.fn() mocks directly to getGitBranch - project-fingerprint.test.ts: pass inline mock fs objects directly - index.runtime.test.ts: replace toHaveProperty dotted-keys with bracket notation No global module mocks = no inter-run cache pollution. 330/330 tests pass on bun test --run, stable across consecutive runs. --- src/git-branch.test.ts | 225 ++++++++++------- src/git-branch.ts | 7 +- src/index.runtime.test.ts | 8 +- src/project-fingerprint.test.ts | 414 ++++++++++++++++++-------------- src/project-fingerprint.ts | 34 ++- 5 files changed, 401 insertions(+), 287 deletions(-) diff --git a/src/git-branch.test.ts b/src/git-branch.test.ts index 6f497ab..9f382a1 100644 --- a/src/git-branch.test.ts +++ b/src/git-branch.test.ts @@ -1,12 +1,8 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import * as childProcess from 'node:child_process'; +import { describe, it, expect, vi } from 'vitest'; +import type { execFile } from 'node:child_process'; import { getGitBranch } from './git-branch.js'; -vi.mock('child_process'); - -const mockedExecFile = vi.mocked(childProcess.execFile); - type ExecFileCallback = ( error: Error | null, stdout: string, @@ -14,127 +10,176 @@ type ExecFileCallback = ( ) => void; describe('getGitBranch', () => { - beforeEach(() => { - vi.resetAllMocks(); - }); - - afterEach(() => { - vi.restoreAllMocks(); - }); - it('calls execFile with git binary and correct argv', async () => { - mockedExecFile.mockImplementation((file, args, _opts, callback) => { - expect(file).toBe('git'); - expect(args).toEqual(['rev-parse', '--abbrev-ref', 'HEAD']); - (callback as ExecFileCallback)(null, 'main\n', ''); - return {} as childProcess.ChildProcess; - }); - - await getGitBranch('/project'); - expect(mockedExecFile).toHaveBeenCalledTimes(1); + const mockExecFile = vi + .fn() + .mockImplementation((file, args, _opts, callback) => { + expect(file).toBe('git'); + expect(args).toEqual(['rev-parse', '--abbrev-ref', 'HEAD']); + (callback as ExecFileCallback)(null, 'main\n', ''); + return {} as ReturnType; + }); + + await getGitBranch('/project', mockExecFile as unknown as typeof execFile); + expect(mockExecFile).toHaveBeenCalledTimes(1); }); it('passes cwd, timeout, and killSignal in options', async () => { - mockedExecFile.mockImplementation((_file, _args, opts, callback) => { - const options = opts as childProcess.ExecFileOptions; - expect(options.cwd).toBe('/my/project/dir'); - expect(options.timeout).toBe(5000); - expect(options.killSignal).toBe('SIGTERM'); - (callback as ExecFileCallback)(null, 'main\n', ''); - return {} as childProcess.ChildProcess; - }); - - await getGitBranch('/my/project/dir'); + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, opts, callback) => { + expect(opts.cwd).toBe('/my/project/dir'); + expect(opts.timeout).toBe(5000); + expect(opts.killSignal).toBe('SIGTERM'); + (callback as ExecFileCallback)(null, 'main\n', ''); + return {} as ReturnType; + }); + + await getGitBranch( + '/my/project/dir', + mockExecFile as unknown as typeof execFile + ); }); it('returns current branch name when git succeeds', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, 'main\n', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)(null, 'main\n', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBe('main'); }); - it('returns undefined if not a git repository', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - const error = new Error('fatal: not a git repository'); - (callback as ExecFileCallback)(error, '', 'fatal: not a git repository'); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/not-a-repo'); + it('returns null if not a git repository', async () => { + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + const error = new Error('fatal: not a git repository'); + (callback as ExecFileCallback)( + error, + '', + 'fatal: not a git repository' + ); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/not-a-repo', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); - it('returns undefined if command fails', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - const error = new Error('Command failed'); - (callback as ExecFileCallback)(error, '', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + it('returns null if command fails', async () => { + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + const error = new Error('Command failed'); + (callback as ExecFileCallback)(error, '', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); - it('returns undefined for detached HEAD state', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, 'HEAD\n', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + it('returns null for detached HEAD state', async () => { + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)(null, 'HEAD\n', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); it('trims stdout whitespace', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, ' feature/test \n', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)(null, ' feature/test \n', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBe('feature/test'); }); it('tolerates stderr noise when stdout is valid', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, 'develop\n', 'warning: some noise'); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)( + null, + 'develop\n', + 'warning: some noise' + ); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBe('develop'); }); - it('returns undefined when stdout is empty', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, '', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + it('returns null when stdout is empty', async () => { + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)(null, '', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); - it('returns undefined when stdout is only whitespace', async () => { - mockedExecFile.mockImplementation((_file, _args, _opts, callback) => { - (callback as ExecFileCallback)(null, ' \n\t ', ''); - return {} as childProcess.ChildProcess; - }); - - const branch = await getGitBranch('/project'); + it('returns null when stdout is only whitespace', async () => { + const mockExecFile = vi + .fn() + .mockImplementation((_file, _args, _opts, callback) => { + (callback as ExecFileCallback)(null, ' \n\t ', ''); + return {} as ReturnType; + }); + + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); it('never throws on unexpected errors', async () => { - mockedExecFile.mockImplementation(() => { + const mockExecFile = vi.fn().mockImplementation(() => { throw new Error('Unexpected sync error'); }); - const branch = await getGitBranch('/project'); + const branch = await getGitBranch( + '/project', + mockExecFile as unknown as typeof execFile + ); expect(branch).toBeNull(); }); }); diff --git a/src/git-branch.ts b/src/git-branch.ts index 66b4b8c..5e9bfaf 100644 --- a/src/git-branch.ts +++ b/src/git-branch.ts @@ -4,7 +4,10 @@ import { createDebugLog } from './debug.js'; const debugLog = createDebugLog(); const GIT_TIMEOUT_MS = 5000; -export async function getGitBranch(projectDir: string): Promise { +export async function getGitBranch( + projectDir: string, + execFn: typeof execFile = execFile +): Promise { try { const branch = await new Promise(resolve => { const opts: ExecFileOptions = { @@ -12,7 +15,7 @@ export async function getGitBranch(projectDir: string): Promise { timeout: GIT_TIMEOUT_MS, killSignal: 'SIGTERM', }; - execFile( + execFn( 'git', ['rev-parse', '--abbrev-ref', 'HEAD'], opts, diff --git a/src/index.runtime.test.ts b/src/index.runtime.test.ts index 25dd08f..189501a 100644 --- a/src/index.runtime.test.ts +++ b/src/index.runtime.test.ts @@ -208,11 +208,11 @@ describe('OpenCodeRulesPlugin', () => { const hooks = await plugin( mockInput as unknown as Parameters[0] ); - expect(hooks).toHaveProperty('experimental.chat.messages.transform'); - expect(hooks).toHaveProperty('experimental.chat.system.transform'); + expect(hooks['experimental.chat.messages.transform']).toBeDefined(); expect(typeof hooks['experimental.chat.messages.transform']).toBe( 'function' ); + expect(hooks['experimental.chat.system.transform']).toBeDefined(); expect(typeof hooks['experimental.chat.system.transform']).toBe('function'); }); @@ -229,8 +229,8 @@ describe('OpenCodeRulesPlugin', () => { const hooks = await plugin( mockInput as unknown as Parameters[0] ); - expect(hooks).toHaveProperty('experimental.chat.messages.transform'); - expect(hooks).toHaveProperty('experimental.chat.system.transform'); + expect(hooks['experimental.chat.messages.transform']).toBeDefined(); + expect(hooks['experimental.chat.system.transform']).toBeDefined(); }); it('should inject rules into system prompt via system.transform hook', async () => { diff --git a/src/project-fingerprint.test.ts b/src/project-fingerprint.test.ts index 39b391b..d941f06 100644 --- a/src/project-fingerprint.test.ts +++ b/src/project-fingerprint.test.ts @@ -1,221 +1,260 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import * as fs from 'node:fs/promises'; +import { describe, it, expect, vi } from 'vitest'; -import { detectProjectTags } from './project-fingerprint.js'; - -vi.mock('fs/promises'); - -const mockedFs = vi.mocked(fs); +import { detectProjectTags, type ProjectTagFs } from './project-fingerprint.js'; describe('detectProjectTags', () => { - beforeEach(() => { - vi.resetAllMocks(); - }); - - afterEach(() => { - vi.restoreAllMocks(); - }); - it('returns "node" tag when package.json exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('package.json')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('package.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('node'); }); it('returns "python" tag when pyproject.toml exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('pyproject.toml')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('pyproject.toml')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('python'); }); it('returns "go" tag when go.mod exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('go.mod')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('go.mod')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('go'); }); it('returns "rust" tag when Cargo.toml exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('Cargo.toml')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('Cargo.toml')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('rust'); }); it('returns "monorepo" tag when pnpm-workspace.yaml exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('pnpm-workspace.yaml')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('pnpm-workspace.yaml')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('monorepo'); }); it('returns "monorepo" tag when turbo.json exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('turbo.json')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('turbo.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('monorepo'); }); it('returns "browser-extension" tag when manifest.json with browser extension keys exists', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return JSON.stringify({ - manifest_version: 3, - background: { service_worker: 'bg.js' }, - }); - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return JSON.stringify({ + manifest_version: 3, + background: { service_worker: 'bg.js' }, + }); + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('browser-extension'); }); it('does not return "browser-extension" for non-extension manifest.json', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return JSON.stringify({ name: 'some-package', version: '1.0.0' }); - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return JSON.stringify({ name: 'some-package', version: '1.0.0' }); + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).not.toContain('browser-extension'); }); it('returns deterministic sorted tag output', async () => { - mockedFs.access.mockImplementation(async filePath => { - const p = String(filePath); - if ( - p.endsWith('package.json') || - p.endsWith('pyproject.toml') || - p.endsWith('Cargo.toml') - ) { - return; - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + const p = String(filePath); + if ( + p.endsWith('package.json') || + p.endsWith('pyproject.toml') || + p.endsWith('Cargo.toml') + ) { + return; + } + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).toEqual(['node', 'python', 'rust']); expect(tags).toEqual([...tags].sort((a, b) => a.localeCompare(b))); }); it('uses explicit comparator function for sorting', async () => { - const originalSort = Array.prototype.sort; - let comparatorWasFunction = false; - - vi.spyOn(Array.prototype, 'sort').mockImplementation(function ( - this: string[], - compareFn?: (a: string, b: string) => number - ) { - if (typeof compareFn === 'function') { - comparatorWasFunction = true; - } - return originalSort.call(this, compareFn); - }); - - mockedFs.access.mockImplementation(async filePath => { - const p = String(filePath); - if (p.endsWith('package.json') || p.endsWith('pyproject.toml')) { - return; - } - throw new Error('ENOENT'); - }); - - await detectProjectTags('/project'); - expect(comparatorWasFunction).toBe(true); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + const p = String(filePath); + if (p.endsWith('package.json') || p.endsWith('pyproject.toml')) { + return; + } + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); + expect(tags).toEqual(['node', 'python']); }); it('returns empty array when no markers exist', async () => { - mockedFs.access.mockRejectedValue(new Error('ENOENT')); + const fs: ProjectTagFs = { + access: vi.fn(async () => { + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).toEqual([]); }); it('tolerates unreadable files by skipping them', async () => { - mockedFs.access.mockImplementation(async filePath => { - const p = String(filePath); - if (p.endsWith('package.json')) return; - if (p.endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockRejectedValue(new Error('EACCES')); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + const p = String(filePath); + if (p.endsWith('package.json')) return; + if (p.endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('EACCES'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('node'); expect(tags).not.toContain('browser-extension'); }); it('returns unique tags even when multiple markers map to same tag', async () => { - mockedFs.access.mockImplementation(async filePath => { - const p = String(filePath); - if (p.endsWith('pnpm-workspace.yaml') || p.endsWith('turbo.json')) return; - throw new Error('ENOENT'); - }); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + const p = String(filePath); + if (p.endsWith('pnpm-workspace.yaml') || p.endsWith('turbo.json')) + return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async () => { + throw new Error('ENOENT'); + }), + }; - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags.filter(t => t === 'monorepo')).toHaveLength(1); }); it('does not tag browser-extension for manifest with only generic keys like permissions', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return JSON.stringify({ permissions: ['storage'] }); - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return JSON.stringify({ permissions: ['storage'] }); + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).not.toContain('browser-extension'); }); it('does not throw and does not tag for invalid JSON manifest', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return '{ invalid json }'; - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return '{ invalid json }'; + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).not.toContain('browser-extension'); }); @@ -223,52 +262,57 @@ describe('detectProjectTags', () => { const nonObjectPayloads = ['null', '[]', '"string"', '123']; for (const payload of nonObjectPayloads) { - vi.resetAllMocks(); - mockedFs.access.mockImplementation(async filePath => { + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return payload; + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); + expect(tags).not.toContain('browser-extension'); + } + }); + + it('does not tag browser-extension for manifest_version 3 alone without signal keys', async () => { + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { if (String(filePath).endsWith('manifest.json')) return; throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { + }), + readFile: vi.fn(async filePath => { if (String(filePath).endsWith('manifest.json')) { - return payload; + return JSON.stringify({ manifest_version: 3 }); } throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); - expect(tags).not.toContain('browser-extension'); - } - }); + }), + }; - it('does not tag browser-extension for manifest_version 3 alone without signal keys', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return JSON.stringify({ manifest_version: 3 }); - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const tags = await detectProjectTags('/project', fs); expect(tags).not.toContain('browser-extension'); }); it('tags browser-extension for manifest_version 3 with MV3 action key', async () => { - mockedFs.access.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) return; - throw new Error('ENOENT'); - }); - mockedFs.readFile.mockImplementation(async filePath => { - if (String(filePath).endsWith('manifest.json')) { - return JSON.stringify({ manifest_version: 3, action: {} }); - } - throw new Error('ENOENT'); - }); - - const tags = await detectProjectTags('/project'); + const fs: ProjectTagFs = { + access: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) return; + throw new Error('ENOENT'); + }), + readFile: vi.fn(async filePath => { + if (String(filePath).endsWith('manifest.json')) { + return JSON.stringify({ manifest_version: 3, action: {} }); + } + throw new Error('ENOENT'); + }), + }; + + const tags = await detectProjectTags('/project', fs); expect(tags).toContain('browser-extension'); }); }); diff --git a/src/project-fingerprint.ts b/src/project-fingerprint.ts index 8890f0c..4b237ae 100644 --- a/src/project-fingerprint.ts +++ b/src/project-fingerprint.ts @@ -19,7 +19,25 @@ const BROWSER_EXTENSION_SIGNAL_KEYS = [ 'permissions', ]; -async function fileExists(filePath: string): Promise { +export interface ProjectTagFs { + access(filePath: string): Promise; + readFile(filePath: string, encoding: string): Promise; +} + +const nodeFs: ProjectTagFs = { + access: path => fs.access(path), + readFile: (path, encoding) => fs.readFile(path, encoding as BufferEncoding), +}; + +export interface ProjectTagFs { + access(filePath: string): Promise; + readFile(filePath: string, encoding: string): Promise; +} + +async function fileExists( + filePath: string, + fs: ProjectTagFs +): Promise { try { await fs.access(filePath); return true; @@ -29,7 +47,8 @@ async function fileExists(filePath: string): Promise { } async function isBrowserExtensionManifest( - manifestPath: string + manifestPath: string, + fs: ProjectTagFs ): Promise { try { const content = await fs.readFile(manifestPath, 'utf-8'); @@ -44,12 +63,15 @@ async function isBrowserExtensionManifest( } } -export async function detectProjectTags(projectDir: string): Promise { +export async function detectProjectTags( + projectDir: string, + fs: ProjectTagFs = nodeFs +): Promise { const tags = new Set(); const checks = SIMPLE_MARKERS.map(async ([marker, tag]) => { const markerPath = path.join(projectDir, marker); - if (await fileExists(markerPath)) { + if (await fileExists(markerPath, fs)) { tags.add(tag); } }); @@ -58,8 +80,8 @@ export async function detectProjectTags(projectDir: string): Promise { checks.push( (async () => { if ( - (await fileExists(manifestPath)) && - (await isBrowserExtensionManifest(manifestPath)) + (await fileExists(manifestPath, fs)) && + (await isBrowserExtensionManifest(manifestPath, fs)) ) { tags.add('browser-extension'); }