From 7e59af81488ceb21976eeb1c2d66fe21f8bf697b Mon Sep 17 00:00:00 2001 From: Drew Ritter Date: Mon, 27 Apr 2026 11:38:26 -0700 Subject: [PATCH] test(opencode): simplify bootstrap cache coverage --- .opencode/plugins/superpowers.js | 22 +- tests/opencode/run-tests.sh | 2 + tests/opencode/test-bootstrap-caching.mjs | 124 +++++++ tests/opencode/test-bootstrap-caching.sh | 384 +--------------------- 4 files changed, 145 insertions(+), 387 deletions(-) create mode 100644 tests/opencode/test-bootstrap-caching.mjs diff --git a/.opencode/plugins/superpowers.js b/.opencode/plugins/superpowers.js index e32848bc..f2b95f23 100644 --- a/.opencode/plugins/superpowers.js +++ b/.opencode/plugins/superpowers.js @@ -114,8 +114,9 @@ ${toolMapping} // 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. + // opencode's prompt.ts reloads messages from DB each step. Fresh message + // arrays may need injection again, so getBootstrapContent() must not do + // repeated disk work. 'experimental.chat.messages.transform': async (_input, output) => { const bootstrap = getBootstrapContent(); if (!bootstrap || !output.messages.length) return; @@ -123,15 +124,8 @@ ${toolMapping} if (!firstUser || !firstUser.parts.length) return; // 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). + // This prevents double injection when OpenCode passes an already + // transformed in-memory message array through the hook again. if (firstUser.parts.some(p => p.type === 'text' && p.text.includes('EXTREMELY_IMPORTANT'))) return; const ref = firstUser.parts[0]; @@ -139,9 +133,3 @@ ${toolMapping} } }; }; - -// Exported for testing — allows tests to reset the cache between runs -export const _testing = { - resetCache: () => { _bootstrapCache = undefined; }, - getCache: () => _bootstrapCache, -}; diff --git a/tests/opencode/run-tests.sh b/tests/opencode/run-tests.sh index 334e2ef1..d9b100ef 100755 --- a/tests/opencode/run-tests.sh +++ b/tests/opencode/run-tests.sh @@ -44,6 +44,7 @@ while [[ $# -gt 0 ]]; do echo "" echo "Tests:" echo " test-plugin-loading.sh Verify plugin installation and structure" + echo " test-bootstrap-caching.sh Verify bootstrap content caching" echo " test-tools.sh Test use_skill and find_skills tools (integration)" echo " test-priority.sh Test skill priority resolution (integration)" exit 0 @@ -59,6 +60,7 @@ done # List of tests to run (no external dependencies) tests=( "test-plugin-loading.sh" + "test-bootstrap-caching.sh" ) # Integration tests (require OpenCode) diff --git a/tests/opencode/test-bootstrap-caching.mjs b/tests/opencode/test-bootstrap-caching.mjs new file mode 100644 index 00000000..55c4e9eb --- /dev/null +++ b/tests/opencode/test-bootstrap-caching.mjs @@ -0,0 +1,124 @@ +import fs from 'fs'; +import { pathToFileURL } from 'url'; + +const [, , pluginPath, scenario] = process.argv; + +if (!pluginPath || !['present', 'missing'].includes(scenario)) { + console.error('Usage: node test-bootstrap-caching.mjs PLUGIN_PATH present|missing'); + process.exit(2); +} + +let existsCount = 0; +let readCount = 0; + +const originalExistsSync = fs.existsSync; +const originalReadFileSync = fs.readFileSync; + +fs.existsSync = function (...args) { + if (isBootstrapSkillPath(args[0])) { + existsCount += 1; + } + return originalExistsSync.apply(this, args); +}; + +fs.readFileSync = function (...args) { + if (isBootstrapSkillPath(args[0])) { + readCount += 1; + } + return originalReadFileSync.apply(this, args); +}; + +const mod = await import(pathToFileURL(pluginPath).href); +const plugin = await mod.SuperpowersPlugin({ client: {}, directory: '.' }); +const transform = plugin['experimental.chat.messages.transform']; + +const firstOutput = makeOutput(`${scenario} bootstrap first step`); +await transform({}, firstOutput); +const afterFirst = { existsCount, readCount }; + +const secondOutput = makeOutput(`${scenario} bootstrap second step`); +await transform({}, secondOutput); +const afterSecond = { existsCount, readCount }; + +const result = { + scenario, + firstBootstrapParts: countBootstrapParts(firstOutput), + secondBootstrapParts: countBootstrapParts(secondOutput), + firstReadCount: afterFirst.readCount, + secondReadCount: afterSecond.readCount, + firstExistsCount: afterFirst.existsCount, + secondExistsCount: afterSecond.existsCount, +}; + +const failures = scenario === 'present' + ? assertPresentBootstrap(result) + : assertMissingBootstrap(result); + +if (failures.length > 0) { + console.error(JSON.stringify(result, null, 2)); + for (const failure of failures) { + console.error(`FAIL: ${failure}`); + } + process.exit(1); +} + +console.log(JSON.stringify(result, null, 2)); + +function isBootstrapSkillPath(filePath) { + return String(filePath).replaceAll('\\', '/').includes('using-superpowers/SKILL.md'); +} + +function makeOutput(text) { + return { + messages: [{ + info: { role: 'user' }, + parts: [{ type: 'text', text }], + }], + }; +} + +function countBootstrapParts(output) { + return output.messages[0].parts.filter( + (part) => part.type === 'text' && part.text.includes('EXTREMELY_IMPORTANT') + ).length; +} + +function assertPresentBootstrap(result) { + const failures = []; + if (result.firstBootstrapParts !== 1) { + failures.push(`expected first transform to inject one bootstrap part, got ${result.firstBootstrapParts}`); + } + if (result.secondBootstrapParts !== 1) { + failures.push(`expected second transform to inject one bootstrap part, got ${result.secondBootstrapParts}`); + } + if (result.firstReadCount !== 1) { + failures.push(`expected first transform to read SKILL.md once, got ${result.firstReadCount}`); + } + if (result.secondReadCount !== result.firstReadCount) { + failures.push(`expected cached second transform to do no additional reads, got ${result.secondReadCount - result.firstReadCount}`); + } + if (result.secondExistsCount !== result.firstExistsCount) { + failures.push(`expected cached second transform to do no additional exists checks, got ${result.secondExistsCount - result.firstExistsCount}`); + } + return failures; +} + +function assertMissingBootstrap(result) { + const failures = []; + if (result.firstBootstrapParts !== 0) { + failures.push(`expected no bootstrap when SKILL.md is missing, got ${result.firstBootstrapParts}`); + } + if (result.secondBootstrapParts !== 0) { + failures.push(`expected no bootstrap on second missing-file transform, got ${result.secondBootstrapParts}`); + } + if (result.firstReadCount !== 0 || result.secondReadCount !== 0) { + failures.push(`expected missing file path to avoid reads, got ${result.secondReadCount}`); + } + if (result.firstExistsCount < 1) { + failures.push('expected first transform to check whether SKILL.md exists'); + } + if (result.secondExistsCount !== result.firstExistsCount) { + failures.push(`expected missing-file result to be cached, got ${result.secondExistsCount - result.firstExistsCount} extra exists checks`); + } + return failures; +} diff --git a/tests/opencode/test-bootstrap-caching.sh b/tests/opencode/test-bootstrap-caching.sh index 8ebb9b07..958e9940 100755 --- a/tests/opencode/test-bootstrap-caching.sh +++ b/tests/opencode/test-bootstrap-caching.sh @@ -1,388 +1,32 @@ #!/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. +# Verifies the OpenCode transform caches bootstrap content between agent steps. 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 +run_present_file_check() { + node "$SCRIPT_DIR/test-bootstrap-caching.mjs" "$SUPERPOWERS_PLUGIN_FILE" present +} -pass() { echo " [PASS] $1"; passed=$((passed + 1)); } -fail() { echo " [FAIL] $1"; failed=$((failed + 1)); } +run_missing_file_check() { + mv "$SUPERPOWERS_SKILLS_DIR/using-superpowers/SKILL.md" "$TEST_HOME/using-superpowers.SKILL.md.bak" -# ────────────────────────────────────────────────────────────── -# 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 + node "$SCRIPT_DIR/test-bootstrap-caching.mjs" "$SUPERPOWERS_PLUGIN_FILE" missing +} -# ────────────────────────────────────────────────────────────── -# 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 +echo "Test 1: Caches bootstrap after the first successful transform..." +run_present_file_check +echo " [PASS] Bootstrap content is cached while fresh message arrays still receive injection" -# ────────────────────────────────────────────────────────────── -# 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 +echo "Test 2: Caches missing SKILL.md result..." +run_missing_file_check +echo " [PASS] Missing bootstrap file is cached and not re-probed every transform" -# ────────────────────────────────────────────────────────────── -# 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 ==="