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 diff --git a/package.json b/package.json index 8ccf0ba..5267ca9 100644 --- a/package.json +++ b/package.json @@ -67,26 +67,32 @@ }, "peerDependencies": { "@opencode-ai/plugin": "^1.3.7", - "@opencode-ai/sdk": "^1.3.7" + "@opencode-ai/sdk": "^1.3.7", + "@opentui/core": "^0.1.97", + "@opentui/solid": "^0.1.97" + }, + "peerDependenciesMeta": { + "@opentui/core": { + "optional": true + }, + "@opentui/solid": { + "optional": true + } }, "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" }, "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" diff --git a/src/active-rules-state.test.ts b/src/active-rules-state.test.ts index 86de4fe..230b490 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) { @@ -61,33 +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']; - writeActiveRulesState(sessionId, matchedPaths); + await writeActiveRulesState(sessionID, matchedPaths); - // Wait for the fire-and-forget write to complete - await waitForFile(getStateFilePath(sessionId)); - - 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()); @@ -125,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', }), @@ -143,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(), }), @@ -154,36 +153,26 @@ describe('active-rules-state', () => { expect(state).toBeNull(); }); - 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)); - - // 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 () => { - 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 () => { - const sessionId = 'ses_no_temp'; + 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); @@ -193,49 +182,42 @@ describe('active-rules-state', () => { }); it('serializes concurrent writes for same session', async () => { - const sessionId = 'ses_concurrent'; + const sessionID = 'ses_concurrent'; // Fire multiple writes concurrently - writeActiveRulesState(sessionId, ['path1']); - writeActiveRulesState(sessionId, ['path2']); - writeActiveRulesState(sessionId, ['path3']); + const first = writeActiveRulesState(sessionID, ['path1']); + const second = writeActiveRulesState(sessionID, ['path2']); + const third = writeActiveRulesState(sessionID, ['path3']); - // Wait for all writes to complete - await waitForFile(getStateFilePath(sessionId)); - - // 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); + 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(); - 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(); + // 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 () => { - 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..146747b 100644 --- a/src/active-rules-state.ts +++ b/src/active-rules-state.ts @@ -2,12 +2,12 @@ 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, logWarning } 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 */ @@ -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; @@ -37,67 +42,62 @@ 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}`); +/** @throws {Error} If sessionID fails validation. */ +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`); } +/** Write matched rule paths to state. @throws {Error} If sessionID fails validation. */ export function writeActiveRulesState( - sessionId: string, + sessionID: string, matchedPaths: string[] -): void { - if (!isValidSessionId(sessionId)) { - debugLog(`Invalid sessionId rejected: ${sessionId}`); - return; +): Promise { + 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); - // Fire-and-forget: catch errors to prevent unhandled rejection - currentWrite.catch(() => { - // Errors already logged in doAtomicWrite - }); + 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 { - // 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( - `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 @@ -109,29 +109,29 @@ async function doAtomicWrite( } } +/** Read active rules state. @throws {Error} If sessionID fails validation. */ export async function readActiveRulesState( - sessionId: string + sessionID: string ): Promise { - if (!isValidSessionId(sessionId)) { - debugLog(`Invalid sessionId rejected: ${sessionId}`); - return null; + if (!isValidSessionID(sessionID)) { + throw new Error(`Invalid sessionID: ${sessionID}`); } - 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; } @@ -144,7 +144,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; } 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/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/git-branch.test.ts b/src/git-branch.test.ts index 7558e29..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 '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'); - expect(branch).toBeUndefined(); + 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'); - expect(branch).toBeUndefined(); + 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'); - expect(branch).toBeUndefined(); + 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'); - expect(branch).toBeUndefined(); + 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'); - expect(branch).toBeUndefined(); + 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'); - expect(branch).toBeUndefined(); + 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 fdff579..5e9bfaf 100644 --- a/src/git-branch.ts +++ b/src/git-branch.ts @@ -1,29 +1,32 @@ -import { execFile, type ExecFileOptions } from 'child_process'; +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 { + projectDir: string, + execFn: typeof execFile = execFile +): Promise { try { - const branch = await new Promise(resolve => { + const branch = await new Promise(resolve => { const opts: ExecFileOptions = { cwd: projectDir, timeout: GIT_TIMEOUT_MS, killSignal: 'SIGTERM', }; - execFile( + execFn( 'git', ['rev-parse', '--abbrev-ref', 'HEAD'], 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); @@ -31,7 +34,8 @@ export async function getGitBranch( ); }); return branch; - } catch { - return undefined; + } catch (err) { + debugLog(`Failed to get git branch: ${err}`); + return null; } } 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 324ef1f..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, @@ -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/index.runtime.test.ts b/src/index.runtime.test.ts index d4e245c..189501a 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, @@ -123,14 +123,9 @@ 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 extractUserPromptFromParts from runtime-chat module', () => { - expect(runtimeChatModule.extractUserPromptFromParts).toBeDefined(); - expect(typeof runtimeChatModule.extractUserPromptFromParts).toBe( + it('should export updateSessionFromChatMessage from runtime-chat module', () => { + expect(runtimeChatModule.updateSessionFromChatMessage).toBeDefined(); + expect(typeof runtimeChatModule.updateSessionFromChatMessage).toBe( 'function' ); }); @@ -151,20 +146,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(); @@ -227,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'); }); @@ -248,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 () => { @@ -1027,7 +1008,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); }); @@ -1073,7 +1054,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); }); @@ -1116,8 +1097,11 @@ describe('utils runtime exports', () => { 'discoverRuleFiles', 'evaluateHooks', 'extractFilePathsFromMessages', + 'getCachedRule', + 'hasConditions', 'parseRuleMetadata', 'promptMatchesKeywords', + 'readActiveRulesState', 'readAndFormatRules', 'serializeToolArgs', 'toolsMatchAvailable', @@ -1126,9 +1110,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.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/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/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 1b55be7..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, @@ -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' }], }, ]); @@ -33,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' }] }, ]); @@ -48,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', () => { @@ -68,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 aec34b8..e9d1b5b 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 { @@ -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); } /** @@ -90,7 +92,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); @@ -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/project-fingerprint.test.ts b/src/project-fingerprint.test.ts index da7e169..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 '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 e60074d..4b237ae 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'], @@ -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'); } diff --git a/src/rule-discovery.ts b/src/rule-discovery.ts index d726cb3..b766bec 100644 --- a/src/rule-discovery.ts +++ b/src/rule-discovery.ts @@ -2,15 +2,15 @@ * Rule file discovery utilities */ -import { stat, readFile, readdir } from 'fs/promises'; -import path from 'path'; -import os from 'os'; -import { createDebugLog } from './debug'; +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, stripFrontmatter, type RuleMetadata, -} from './rule-metadata'; +} from './rule-metadata.js'; const debugLog = createDebugLog(); @@ -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 */ @@ -45,23 +45,21 @@ 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 -): Promise { +): Promise { try { 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,11 +77,8 @@ 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}` - ); - return undefined; + logWarning(`Failed to read rule file ${filePath}`, error); + return null; } } @@ -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..b1e6d88 100644 --- a/src/rule-filter.ts +++ b/src/rule-filter.ts @@ -5,6 +5,8 @@ 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(); @@ -39,26 +41,113 @@ 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 { - 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)); } +/** + * 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: RuleMetadata, + 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 */ @@ -122,116 +211,15 @@ 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 - ); - - if (hasConditions && 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)); - } + const ruleHasConditions = hasConditions(metadata); - // 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); - } + if (ruleHasConditions && metadata) { + const declaredChecks = evaluateConditionChecks( + metadata, + context, + availableToolSet + ); - // Apply combinator: default 'any', or 'all' if specified const mode = metadata.match ?? 'any'; const shouldInclude = mode === 'all' @@ -250,8 +238,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 13e308b..009eea8 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 @@ -82,34 +83,29 @@ 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 { - // Check if content starts with frontmatter +export function parseRuleMetadata(content: string): RuleMetadata | null { if (!content.startsWith('---')) { - return undefined; + return null; } - // Find the closing --- marker const endIndex = content.indexOf('---', 3); if (endIndex === -1) { - return undefined; + return null; } - // Extract the YAML frontmatter const frontmatter = content.substring(3, endIndex).trim(); if (!frontmatter) { - return undefined; + return null; } try { - // Parse YAML using the yaml package const parsed = parseYaml(frontmatter) as ParsedFrontmatter | null; if (!parsed || typeof parsed !== 'object') { - return undefined; + return null; } 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; } @@ -167,15 +161,10 @@ 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 - const message = error instanceof Error ? error.message : String(error); - console.warn( - `[opencode-rules] Warning: Failed to parse YAML frontmatter: ${message}` - ); - return undefined; + logWarning('Failed to parse YAML frontmatter', error); + return null; } } @@ -183,17 +172,33 @@ export function parseRuleMetadata(content: string): RuleMetadata | undefined { * 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(); } + +/** + * 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/runtime-chat.ts b/src/runtime-chat.ts index 372e6f7..298403b 100644 --- a/src/runtime-chat.ts +++ b/src/runtime-chat.ts @@ -14,23 +14,10 @@ export interface ChatMessageOutput { } /** - * 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. + * 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, @@ -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-context.ts b/src/runtime-context.ts index 17bee5d..8cd563e 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; } /** @@ -24,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; @@ -50,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 ); } @@ -67,12 +53,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); @@ -82,15 +73,17 @@ 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 | undefined; + let gitBranch: string | null = null; try { gitBranch = await getGitBranch(projectDirectory); - } catch { - gitBranch = undefined; + } catch (error) { + debugLog(`Failed to get git branch: ${error}`); + gitBranch = null; } const os = process.platform; @@ -122,7 +115,7 @@ export async function buildFilterContext( if (projectTags !== undefined) { context.projectTags = projectTags; } - if (gitBranch !== undefined) { + if (gitBranch !== null) { context.gitBranch = gitBranch; } diff --git a/src/runtime.tool-ids.test.ts b/src/runtime.tool-ids.test.ts index 1459d18..3f08f55 100644 --- a/src/runtime.tool-ids.test.ts +++ b/src/runtime.tool-ids.test.ts @@ -23,14 +23,9 @@ 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 extractUserPromptFromParts from runtime-chat module', () => { - expect(runtimeChatModule.extractUserPromptFromParts).toBeDefined(); - expect(typeof runtimeChatModule.extractUserPromptFromParts).toBe( + 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 27dab6c..4ba16b0 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -6,18 +6,15 @@ import { extractSessionID, normalizeContextPath, sanitizePathForContext, - toExtractableMessages, + filterValidMessages, 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 } from './runtime-context.js'; import { - buildFilterContext, - type BuildFilterContextOptions, -} from './runtime-context.js'; -import { - handleChatMessage, + updateSessionFromChatMessage, type ChatMessageInput, type ChatMessageOutput, } from './runtime-chat.js'; @@ -40,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; @@ -51,7 +61,7 @@ interface OpenCodeRulesRuntimeOptions { } export class OpenCodeRulesRuntime { - private client: unknown; + private client: OpenCodeClient; private directory: string; private projectDirectory: string; private ruleFiles: DiscoveredRule[]; @@ -60,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; @@ -123,7 +133,6 @@ export class OpenCodeRulesRuntime { ); } - // Evaluate PreToolUse hooks await this.evaluateAndQueueHooks('PreToolUse', sessionID, toolName, args); } @@ -164,7 +173,7 @@ export class OpenCodeRulesRuntime { } const contextPaths = extractFilePathsFromMessages( - toExtractableMessages(output.messages) + filterValidMessages(output.messages) ); const userPrompt = extractLatestUserPrompt(output.messages); @@ -200,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( @@ -266,25 +280,21 @@ 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; if (sessionID) { - writeActiveRulesState(sessionID, result.matchedPaths); + await writeActiveRulesState(sessionID, result.matchedPaths); } } else { this.debugLog( @@ -343,14 +353,26 @@ 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); + + 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' && @@ -363,17 +385,17 @@ 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 (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); } @@ -381,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); @@ -448,14 +464,16 @@ export class OpenCodeRulesRuntime { `Executing hook side-effect for session ${sessionID}: ${command}` ); 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}` + this.debugLog( + `Hook side-effect completed for session ${sessionID}: ${command}` ); + } catch (error) { + logWarning('Hook side-effect failed', error); } } + /** 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, 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 }); -} diff --git a/src/test-fixtures.ts b/src/test-fixtures.ts index d1cfb65..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'; // ============================================================================ @@ -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 // ============================================================================ /** diff --git a/src/utils.ts b/src/utils.ts index 9bc2d46..8044cb3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,23 +1,42 @@ /** - * 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 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. + * + * Modules intentionally NOT re-exported (internal implementation): + * - 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 * - message-paths.ts: Message path extraction + * - rule-hooks.ts: Hook evaluation and serialization */ // Re-export from rule-discovery export { discoverRuleFiles, + getCachedRule, clearRuleCache, 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, + type RuleMetadata, +} from './rule-metadata.js'; // Re-export from rule-filter export { @@ -35,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.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 61cbd03..fd809df 100644 --- a/tui/data/rules.ts +++ b/tui/data/rules.ts @@ -1,8 +1,13 @@ // 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 path from 'path'; +import { + discoverRuleFiles, + getCachedRule, + readActiveRulesState, + hasConditions, + type RuleMetadata, +} from '../../src/utils.js'; +export { hasConditions }; +import path from 'node:path'; /** Represents a rule as displayed in the sidebar */ export interface SidebarRuleEntry { @@ -68,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!) @@ -98,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; @@ -117,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' { @@ -128,25 +133,7 @@ export function ruleSource( } /** - * Check if metadata has any conditional fields set. - */ -export function hasConditions(meta: RuleMetadata | 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" */ 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 cef1135..36782b1 100644 --- a/tui/slots/sidebar-content.tsx +++ b/tui/slots/sidebar-content.tsx @@ -10,7 +10,23 @@ import { type JSX, } from 'solid-js'; import type { TuiPluginApi, TuiTheme } from '@opencode-ai/plugin/tui'; -import { loadSidebarRules, type SidebarRuleEntry } from '../data/rules'; +import { loadSidebarRules, type SidebarRuleEntry } from '../data/rules.js'; +import type { RuleMetadata } from '../../src/utils.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(); } });