From c13a884c06af1d213f1a2debd13f13970ab74e45 Mon Sep 17 00:00:00 2001 From: voidborne-d Date: Mon, 20 Apr 2026 13:54:14 +0000 Subject: [PATCH] fix(opencode): cache bootstrap content at module level to eliminate per-step file I/O getBootstrapContent() called fs.existsSync + fs.readFileSync + regex frontmatter parsing on every agent step with zero caching. The experimental.chat.messages.transform hook fires every step in opencode's agent loop (messages are reloaded from DB each step via filterCompactedEffect). A 10-step turn triggered 10 redundant file reads + 10 regex parses for content that never changes during a session. Changes: - Add module-level _bootstrapCache (undefined = not loaded, null = file missing) so the first call reads and parses SKILL.md, all subsequent calls return the cached string with zero filesystem access - Cache the null sentinel when SKILL.md is missing, preventing repeated fs.existsSync probes - Add _testing export (resetCache/getCache) for test infrastructure - Clarify the injection guard comment explaining how it interacts with opencode's per-step message reloading - Add 15 regression tests covering cache behavior, fs call counts, injection guard, missing file sentinel, cache reset, and source audit Fixes #1202 --- .opencode/plugins/superpowers.js | 43 ++- tests/opencode/test-bootstrap-caching.sh | 388 +++++++++++++++++++++++ 2 files changed, 427 insertions(+), 4 deletions(-) create mode 100755 tests/opencode/test-bootstrap-caching.sh diff --git a/.opencode/plugins/superpowers.js b/.opencode/plugins/superpowers.js index 48a2b72d..e32848bc 100644 --- a/.opencode/plugins/superpowers.js +++ b/.opencode/plugins/superpowers.js @@ -46,17 +46,29 @@ const normalizePath = (p, homeDir) => { return path.resolve(normalized); }; +// Module-level cache for bootstrap content. +// The SKILL.md file does not change during a session, so reading + parsing it +// once eliminates redundant fs.existsSync + fs.readFileSync + regex work on +// every agent step. See #1202 for the full analysis. +let _bootstrapCache = undefined; // undefined = not yet loaded, null = file missing + export const SuperpowersPlugin = async ({ client, directory }) => { const homeDir = os.homedir(); const superpowersSkillsDir = path.resolve(__dirname, '../../skills'); const envConfigDir = normalizePath(process.env.OPENCODE_CONFIG_DIR, homeDir); const configDir = envConfigDir || path.join(homeDir, '.config/opencode'); - // Helper to generate bootstrap content + // Helper to generate bootstrap content (cached after first call) const getBootstrapContent = () => { + // Return cached result on subsequent calls + if (_bootstrapCache !== undefined) return _bootstrapCache; + // Try to load using-superpowers skill const skillPath = path.join(superpowersSkillsDir, 'using-superpowers', 'SKILL.md'); - if (!fs.existsSync(skillPath)) return null; + if (!fs.existsSync(skillPath)) { + _bootstrapCache = null; + return null; + } const fullContent = fs.readFileSync(skillPath, 'utf8'); const { content } = extractAndStripFrontmatter(fullContent); @@ -70,7 +82,7 @@ When skills reference tools you don't have, substitute OpenCode equivalents: Use OpenCode's native \`skill\` tool to list and load skills.`; - return ` + _bootstrapCache = ` You have superpowers. **IMPORTANT: The using-superpowers skill content is included below. It is ALREADY LOADED - you are currently following it. Do NOT use the skill tool to load "using-superpowers" again - that would be redundant.** @@ -79,6 +91,8 @@ ${content} ${toolMapping} `; + + return _bootstrapCache; }; return { @@ -98,15 +112,36 @@ ${toolMapping} // Using a user message instead of a system message avoids: // 1. Token bloat from system messages repeated every turn (#750) // 2. Multiple system messages breaking Qwen and other models (#894) + // + // The hook fires on every agent step (not just every turn) because + // opencode's prompt.ts reloads messages from DB each step. The guard + // below prevents duplicate injection on the fresh in-memory objects. 'experimental.chat.messages.transform': async (_input, output) => { const bootstrap = getBootstrapContent(); if (!bootstrap || !output.messages.length) return; const firstUser = output.messages.find(m => m.info.role === 'user'); if (!firstUser || !firstUser.parts.length) return; - // Only inject once + + // Guard: skip if first user message already contains bootstrap. + // Because messages are loaded fresh from DB each step (the previous + // in-memory injection is never persisted), this guard fires on the + // *new* in-memory objects. It still works correctly: the first step + // injects, and on subsequent steps the content-based check catches it + // because we're looking at the same logical first-user message that + // was just injected into (the DB hasn't changed between steps within + // a single turn, so filterCompactedEffect returns the same message + // contents and we re-inject the same bootstrap — the guard prevents + // doubling up within the same messages array). if (firstUser.parts.some(p => p.type === 'text' && p.text.includes('EXTREMELY_IMPORTANT'))) return; + const ref = firstUser.parts[0]; firstUser.parts.unshift({ ...ref, type: 'text', text: bootstrap }); } }; }; + +// Exported for testing — allows tests to reset the cache between runs +export const _testing = { + resetCache: () => { _bootstrapCache = undefined; }, + getCache: () => _bootstrapCache, +}; diff --git a/tests/opencode/test-bootstrap-caching.sh b/tests/opencode/test-bootstrap-caching.sh new file mode 100755 index 00000000..8ebb9b07 --- /dev/null +++ b/tests/opencode/test-bootstrap-caching.sh @@ -0,0 +1,388 @@ +#!/usr/bin/env bash +# Test: Bootstrap Content Caching (#1202) +# Verifies that getBootstrapContent() caches at module level, +# eliminating per-step file I/O and regex parsing overhead. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" + +echo "=== Test: Bootstrap Content Caching (#1202) ===" + +# Source setup to create isolated environment +source "$SCRIPT_DIR/setup.sh" + +# Trap to cleanup on exit +trap cleanup_test_env EXIT + +passed=0 +failed=0 + +pass() { echo " [PASS] $1"; passed=$((passed + 1)); } +fail() { echo " [FAIL] $1"; failed=$((failed + 1)); } + +# ────────────────────────────────────────────────────────────── +# Test 1: Module-level _bootstrapCache variable exists +# ────────────────────────────────────────────────────────────── +echo "Test 1: Module-level cache variable exists..." +if grep -q '_bootstrapCache' "$SUPERPOWERS_PLUGIN_FILE"; then + pass "Module-level _bootstrapCache variable found" +else + fail "_bootstrapCache variable not found in plugin" +fi + +# ────────────────────────────────────────────────────────────── +# Test 2: Cache is checked before fs operations +# ────────────────────────────────────────────────────────────── +echo "Test 2: Cache checked before filesystem access..." +# The pattern: if (_bootstrapCache !== undefined) return should appear +# BEFORE any fs.existsSync / fs.readFileSync in getBootstrapContent +if grep -qP '_bootstrapCache !== undefined.*return' "$SUPERPOWERS_PLUGIN_FILE"; then + pass "Early return on cache hit exists" +else + fail "No early return on cache hit" +fi + +# ────────────────────────────────────────────────────────────── +# Test 3: Cache is populated after file read +# ────────────────────────────────────────────────────────────── +echo "Test 3: Cache populated after successful file read..." +if grep -q '_bootstrapCache =' "$SUPERPOWERS_PLUGIN_FILE"; then + pass "Cache assignment exists" +else + fail "No cache assignment found" +fi + +# ────────────────────────────────────────────────────────────── +# Test 4: Missing file path also cached (null sentinel) +# ────────────────────────────────────────────────────────────── +echo "Test 4: Missing file case also cached (null sentinel)..." +if grep -q '_bootstrapCache = null' "$SUPERPOWERS_PLUGIN_FILE"; then + pass "Null sentinel for missing file path" +else + fail "Missing file path not cached — would re-check fs.existsSync every step" +fi + +# ────────────────────────────────────────────────────────────── +# Test 5: No redundant fs.readFileSync on cached path +# Verify via Node.js: call getBootstrapContent() twice, count reads +# ────────────────────────────────────────────────────────────── +echo "Test 5: Second call returns cached content without file I/O..." + +cat > "$TEST_HOME/test-cache.mjs" <<'TESTEOF' +import { createRequire } from 'module'; +import path from 'path'; +import fs from 'fs'; + +// Monkey-patch fs to count calls +let readCount = 0; +let existsCount = 0; +const origReadFileSync = fs.readFileSync; +const origExistsSync = fs.existsSync; + +fs.readFileSync = function(...args) { + readCount++; + return origReadFileSync.apply(this, args); +}; +fs.existsSync = function(...args) { + existsCount++; + return origExistsSync.apply(this, args); +}; + +// Import the plugin (this sets __dirname based on plugin location) +const pluginPath = process.argv[2]; +const mod = await import(pluginPath); + +// Initialize plugin +const plugin = await mod.SuperpowersPlugin({ client: {}, directory: '.' }); + +// Reset cache for clean test +mod._testing.resetCache(); + +// First call — should hit fs +readCount = 0; +existsCount = 0; +const result1 = await plugin['experimental.chat.messages.transform']( + {}, + { messages: [{ + info: { role: 'user' }, + parts: [{ type: 'text', text: 'hello' }] + }]} +); +const firstReadCount = readCount; +const firstExistsCount = existsCount; + +// Reset cache state check +const cacheAfterFirst = mod._testing.getCache(); + +// Reset counters, call again (should be cached) +// Need to reset the injection guard — create fresh messages +readCount = 0; +existsCount = 0; +const result2 = await plugin['experimental.chat.messages.transform']( + {}, + { messages: [{ + info: { role: 'user' }, + parts: [{ type: 'text', text: 'hello again' }] + }]} +); +const secondReadCount = readCount; +const secondExistsCount = existsCount; + +// Output results +console.log(JSON.stringify({ + firstReadCount, + firstExistsCount, + secondReadCount, + secondExistsCount, + cachePopulated: cacheAfterFirst !== undefined && cacheAfterFirst !== null, + contentIncludesMarker: cacheAfterFirst?.includes('EXTREMELY_IMPORTANT') ?? false +})); +TESTEOF + +RESULT=$(node "$TEST_HOME/test-cache.mjs" "$SUPERPOWERS_PLUGIN_FILE" 2>/dev/null) +if [ $? -ne 0 ]; then + fail "Cache test script failed to execute" +else + SECOND_READ=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.secondReadCount))") + SECOND_EXISTS=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.secondExistsCount))") + CACHE_POP=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.cachePopulated))") + CONTENT_OK=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.contentIncludesMarker))") + + if [ "$SECOND_READ" = "0" ] && [ "$SECOND_EXISTS" = "0" ]; then + pass "Second call made 0 fs.readFileSync and 0 fs.existsSync calls" + else + fail "Second call still made fs calls (read=$SECOND_READ, exists=$SECOND_EXISTS)" + fi + + if [ "$CACHE_POP" = "true" ]; then + pass "Cache populated after first call" + else + fail "Cache not populated after first call" + fi + + if [ "$CONTENT_OK" = "true" ]; then + pass "Cached content contains EXTREMELY_IMPORTANT marker" + else + fail "Cached content missing expected marker" + fi +fi + +# ────────────────────────────────────────────────────────────── +# Test 6: _testing.resetCache() clears the cache +# ────────────────────────────────────────────────────────────── +echo "Test 6: resetCache() allows re-reading from disk..." + +cat > "$TEST_HOME/test-reset.mjs" <<'TESTEOF' +import fs from 'fs'; + +const pluginPath = process.argv[2]; +const mod = await import(pluginPath); + +// Initialize and populate cache +const plugin = await mod.SuperpowersPlugin({ client: {}, directory: '.' }); +await plugin['experimental.chat.messages.transform']( + {}, + { messages: [{ info: { role: 'user' }, parts: [{ type: 'text', text: 'test' }] }] } +); + +const beforeReset = mod._testing.getCache(); +mod._testing.resetCache(); +const afterReset = mod._testing.getCache(); + +console.log(JSON.stringify({ + beforeResetDefined: beforeReset !== undefined, + afterResetUndefined: afterReset === undefined +})); +TESTEOF + +RESULT=$(node "$TEST_HOME/test-reset.mjs" "$SUPERPOWERS_PLUGIN_FILE" 2>/dev/null) +if [ $? -ne 0 ]; then + fail "Reset test script failed to execute" +else + BEFORE=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.beforeResetDefined))") + AFTER=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.afterResetUndefined))") + + if [ "$BEFORE" = "true" ] && [ "$AFTER" = "true" ]; then + pass "resetCache() transitions from defined to undefined" + else + fail "resetCache() did not clear properly (before=$BEFORE, after=$AFTER)" + fi +fi + +# ────────────────────────────────────────────────────────────── +# Test 7: Injection guard prevents double-injection in same array +# ────────────────────────────────────────────────────────────── +echo "Test 7: Injection guard prevents double-injection..." + +cat > "$TEST_HOME/test-guard.mjs" <<'TESTEOF' +const pluginPath = process.argv[2]; +const mod = await import(pluginPath); +mod._testing.resetCache(); + +const plugin = await mod.SuperpowersPlugin({ client: {}, directory: '.' }); + +// Create a message array and inject once +const messages = [{ + info: { role: 'user' }, + parts: [{ type: 'text', text: 'hello' }] +}]; + +await plugin['experimental.chat.messages.transform']({}, { messages }); +const partsAfterFirst = messages[0].parts.length; + +// Call again on the SAME messages array (simulates what would happen +// if the hook fired twice on the same in-memory messages) +await plugin['experimental.chat.messages.transform']({}, { messages }); +const partsAfterSecond = messages[0].parts.length; + +console.log(JSON.stringify({ + partsAfterFirst, + partsAfterSecond, + noDuplication: partsAfterFirst === partsAfterSecond +})); +TESTEOF + +RESULT=$(node "$TEST_HOME/test-guard.mjs" "$SUPERPOWERS_PLUGIN_FILE" 2>/dev/null) +if [ $? -ne 0 ]; then + fail "Guard test script failed to execute" +else + NO_DUP=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.noDuplication))") + FIRST=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.partsAfterFirst))") + + if [ "$NO_DUP" = "true" ]; then + pass "Guard prevented double injection (parts count stable at $FIRST)" + else + fail "Bootstrap injected twice into same message" + fi +fi + +# ────────────────────────────────────────────────────────────── +# Test 8: Missing skill file → null cached (no repeated fs probes) +# ────────────────────────────────────────────────────────────── +echo "Test 8: Missing SKILL.md file produces cached null..." + +cat > "$TEST_HOME/test-missing.mjs" <<'TESTEOF' +import fs from 'fs'; + +const pluginPath = process.argv[2]; +const mod = await import(pluginPath); +mod._testing.resetCache(); + +// Temporarily rename the skill file to simulate missing +const skillDir = new URL('../../skills/using-superpowers/SKILL.md', import.meta.resolve(pluginPath)); +// We can't easily rename, so instead we'll test by checking the cache +// behavior when _bootstrapCache is set to null +mod._testing.resetCache(); + +const plugin = await mod.SuperpowersPlugin({ client: {}, directory: '.' }); + +// Monkey-patch fs.existsSync to return false for SKILL.md +const orig = fs.existsSync; +fs.existsSync = (p) => { + if (typeof p === 'string' && p.includes('using-superpowers')) return false; + return orig(p); +}; + +let readCount = 0; +const origRead = fs.readFileSync; +fs.readFileSync = function(...args) { readCount++; return origRead.apply(this, args); }; + +// First call — should set null cache +const msgs1 = [{ info: { role: 'user' }, parts: [{ type: 'text', text: 'test' }] }]; +await plugin['experimental.chat.messages.transform']({}, { messages: msgs1 }); +const cacheAfterMissing = mod._testing.getCache(); +const firstReadCount = readCount; + +// Second call — should hit null cache, skip fs entirely +readCount = 0; +const msgs2 = [{ info: { role: 'user' }, parts: [{ type: 'text', text: 'test2' }] }]; +await plugin['experimental.chat.messages.transform']({}, { messages: msgs2 }); + +// Restore +fs.existsSync = orig; +fs.readFileSync = origRead; + +console.log(JSON.stringify({ + cacheIsNull: cacheAfterMissing === null, + secondCallReads: readCount, + noInjection: msgs1[0].parts.length === 1 +})); +TESTEOF + +RESULT=$(node "$TEST_HOME/test-missing.mjs" "$SUPERPOWERS_PLUGIN_FILE" 2>/dev/null) +if [ $? -ne 0 ]; then + fail "Missing file test script failed to execute" +else + IS_NULL=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.cacheIsNull))") + SEC_READS=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.secondCallReads))") + NO_INJ=$(echo "$RESULT" | node -e "const d=JSON.parse(require('fs').readFileSync(0,'utf8'));process.stdout.write(String(d.noInjection))") + + if [ "$IS_NULL" = "true" ]; then + pass "Cache set to null for missing file" + else + fail "Cache not set to null for missing file" + fi + + if [ "$SEC_READS" = "0" ]; then + pass "Second call with missing file made 0 fs reads" + else + fail "Second call with missing file still read fs ($SEC_READS times)" + fi + + if [ "$NO_INJ" = "true" ]; then + pass "No bootstrap injected when file missing" + else + fail "Bootstrap somehow injected despite missing file" + fi +fi + +# ────────────────────────────────────────────────────────────── +# Test 9: Source audit — no uncached fs.readFileSync in getBootstrapContent +# ────────────────────────────────────────────────────────────── +echo "Test 9: Source audit — getBootstrapContent caches all fs paths..." + +# Extract getBootstrapContent function body and verify cache pattern +# The function should: check cache → fs.existsSync → fs.readFileSync → assign cache +FUNC_BODY=$(sed -n '/const getBootstrapContent/,/^ };$/p' "$SUPERPOWERS_PLUGIN_FILE") + +# Verify the cache check comes before any fs call +CACHE_LINE=$(echo "$FUNC_BODY" | grep -n '_bootstrapCache !== undefined' | head -1 | cut -d: -f1) +EXISTS_LINE=$(echo "$FUNC_BODY" | grep -n 'fs.existsSync' | head -1 | cut -d: -f1) + +if [ -n "$CACHE_LINE" ] && [ -n "$EXISTS_LINE" ] && [ "$CACHE_LINE" -lt "$EXISTS_LINE" ]; then + pass "Cache check (line $CACHE_LINE) precedes fs.existsSync (line $EXISTS_LINE)" +else + fail "Cache check does not precede fs.existsSync (cache=$CACHE_LINE, exists=$EXISTS_LINE)" +fi + +# ────────────────────────────────────────────────────────────── +# Test 10: JavaScript syntax still valid after changes +# ────────────────────────────────────────────────────────────── +echo "Test 10: Plugin JavaScript syntax remains valid..." +if node --check "$SUPERPOWERS_PLUGIN_FILE" 2>/dev/null; then + pass "Plugin JavaScript syntax is valid" +else + fail "Plugin has JavaScript syntax errors" +fi + +# ────────────────────────────────────────────────────────────── +# Test 11: _testing export exists for test infrastructure +# ────────────────────────────────────────────────────────────── +echo "Test 11: _testing export available..." +if grep -q 'export const _testing' "$SUPERPOWERS_PLUGIN_FILE"; then + pass "_testing export exists" +else + fail "_testing export not found" +fi + +# ────────────────────────────────────────────────────────────── +# Summary +# ────────────────────────────────────────────────────────────── +echo "" +total=$((passed + failed)) +echo "=== Results: $passed/$total passed ===" + +if [ "$failed" -gt 0 ]; then + exit 1 +fi +echo "=== All bootstrap caching tests passed ==="