Distillation

Date: Sep 9, 2026

• 🔴 (09:45) /home/byk/Code/opencode-lore/quality/REVIEW.md codifies Lore’s pull-request review bar because adversarial review has repeatedly found correctness bugs missed by green test suites, especially skip/early-return branches, lifecycle sequences, and fan-out registry gaps.
• 🔴 [enforced-regression-testing] (09:45) Every adversarial-review finding that exposes a defect must include a deterministic regression test in the same PR: it must fail on the base branch, pass on the fix, and drive the real precondition rather than an artificially constructed state.
• 🔴 [enforced-regression-testing] (09:45) Tests that “test the guard” must fail when the guard is deleted; if a test still passes after guard removal, it does not test the guard.
• 🔴 (09:45) Adversarial-order test setup must reproduce state in the order an adversary would create it: state present before the handling operation, delete-then-recreate across lifecycle boundaries, and content modification while sync is disabled.
• 🔴 (09:45) Fan-out registry contract mapping: SYNCED_TABLES → sync-registry-contract.test.ts, covering registered table shape, pull-only/no-capture invariants, composite-primary-key correctness, sync columns, ID columns, and versioned flags; MIGRATIONS → db.test.ts with one toBe(MIGRATIONS.length) schema-version assertion plus migration-specific tests; AGENTS → agents.test.ts, requiring every agent to have name, displayName, binary, and detect(), with binary as a constant placeholder string.
• 🔴 (09:45) Standing regression batteries are: registry contracts in packages/core/test/sync-registry-contract.test.ts; per-operation sync invariants in packages/core/src/sync-data.ts through assertSyncInvariants; and sequence properties in packages/gateway/test/sync.property.test.ts.
• 🔴 (09:45) assertSyncInvariants covers no pull-only outbox entries, at most 1 profile, and every state row referencing a registered table.
• 🔴 (09:45) packages/gateway/test/sync.property.test.ts covers standing invariants after every operation, prune-floor non-wedging, eventual convergence, no ping-pong, and currentTier correctness.
• 🔴 (09:45) Findings from stale-upsert PRs/issues #856, #861, #866, and #868 establish skip/early-return branches as the highest-risk surface; guards involving NOT EXISTS, latest <> 'upsert', and content matching masked real data-loss bugs, so every skip branch needs a test that makes it skip something it should not.
• 🔴 (09:45) The #833 property-test battery found 4 real sync-engine bugs missed by example tests; all depended on sequences crossing enable/disable/push/pull boundaries.
• 🔴 [enforced-tdd] (09:45) User’s bug-fix standard is failing-first TDD: write the regression test first, confirm it fails on the base branch, then confirm it passes with the fix. Exact directive: “Never trust the fix without seeing it fail.”
• 🔴 (09:45) Memoized arrays shared by multiple callers, such as the syncedColumns PRAGMA cache, should be frozen so accidental mutation throws TypeError in strict mode rather than silently corrupting the cache.
• 🔴 (09:45) In tests that count SQL statements, db().query monkeypatches are silently shadowed by the tracing Proxy; use log.registerSink({ withDbSpan }), as empirically verified in #874.
• 🔴 [enforced-review-workflow] (09:45) Non-trivial changes require 2 separate review passes with distinct prompts: 1. adversarial/correctness review asking “What state makes this break?” and checking enumerated cases, apply-then-revert mutants, property-test stability ×10, hash-equivalence proofs, frozen-array mutation safety, and stale-cache analysis; 2. security/pentest review covering injection vectors, credential leaks, protocol-level attacks, and privilege escalation.
• 🔴 [enforced-review-workflow] (09:45) Adversarial review is mandatory before merge; security review depends on the risk profile. Correctness-critical changes involving the sync engine, hashing, lifecycle, or migrations require adversarial review regardless of size, while mechanical or test-only PRs may skip it at reviewer discretion.
• 🔴 [requested-tests] (09:45) Reviewers must run pnpm test, pnpm run typecheck, pnpm run lint, and pnpm run format:check; they must inspect exit codes because a blank output tail can hide lint or formatting failures.
• 🔴 [enforced-review-workflow] (09:45) Reviewers must run property tests 10 times, confirm the final working tree is unmodified using hash verification, provide a PASS / FAIL / CONCERN / MUST-FIX verdict for each point with file:line evidence, and give an overall MERGE / DO-NOT-MERGE decision; unstructured “LGTM” is insufficient.
• 🔴 (09:45) /home/byk/Code/opencode-v2-pilot/packages/core/src/config/plugin/source.ts:152 logs the warning "configured plugin path must be a directory" with { target: operation.target }.
• 🔴 (09:45) In /home/byk/Code/opencode-v2-pilot/packages/core/src/plugin/module.ts, configured local plugins “always arrive here as directories”; the local-file branch at lines 94–97 remains for legacy auto-discovery of standalone server sources.
• 🔴 (09:45) PluginModule.make in /home/byk/Code/opencode-v2-pilot/packages/core/src/plugin/module.ts creates an unbounded PubSub<void>, tracks watched paths in Set<string>, subscribes through Watcher.Service, waits on a Deferred<void> readiness signal, and forks update processing into the enclosing scope with { startImmediately: true }.
• 🔴 (09:45) watchTarget(file) recursively walks to the nearest existing ancestor when a plugin dependency is missing, returning a watcher input with type: "directory" or "file" based on stat(file).
• 🔴 (09:45) PluginModule.make acquires createPluginSources((file) => runPromise(watch(file))) and releases it through sources.dispose().
• 🔴 (09:45) PluginModule.load treats an absolute operation.target as local; non-local targets use npm.resolve(operation.target) when options?.install === false, otherwise npm.add(operation.target).
• 🔴 (09:45) PluginModule.load resolves local file entrypoints directly to { server: pathToFileURL(operation.target).href } only for the legacy standalone-file case; otherwise it calls Host.resolve(installed ?? { directory: operation.target }).
• 🔴 (09:45) For an unresolved non-local plugin with options?.install === false, PluginModule.load returns { pending: true as const }; otherwise a missing server entrypoint raises PluginModule.LoadError with Plugin entrypoint not found: ${operation.target}.
• 🔴 (09:45) Plugin modules must default-export an object with id and either an effect or setup function; decode failures become PluginModule.LoadError with message "Plugin must export a default definition with an id and an effect or setup function."
• 🔴 (09:45) Promise-style plugin definitions are adapted with PluginPromise.fromPromise(value); native effect-style definitions are used directly.
• 🔴 (09:47) /home/byk/Code/opencode-v2-pilot/packages/plugin/src/promise/adapter.ts keeps compiled RPC endpoint metadata in const compiledEndpoints = new WeakMap<object, CompiledEndpoint>(); each CompiledEndpoint has a readonly decoder array, an encoder, and a noContent flag.
• 🔴 (09:47) makeStreams in /home/byk/Code/opencode-v2-pilot/packages/plugin/src/promise/adapter.ts gives each async iterator its own scope, tracks iterator cleanup callbacks in subscriptions, closes all subscriptions when the plugin unloads, supports optional AbortSignal, and removes each abort listener during close().
• 🔴 (09:47) In /home/byk/Code/opencode-v2-pilot/packages/plugin/src/source.ts, createPluginSources(watch) stores source attempts in Map<string, Source>, cleanup callbacks in Array<() => void>, and in-progress watcher setup in Set<Promise<void>>.
• 🔴 (09:47) createPluginSources().read(entrypoint) waits for all watcher setup, then reuses the prior loaded attempt only when every tracked file’s current digest equals its stored digest.
• 🔴 (09:47) Source tracking records each file as { digest, directory }, starts watch(file), removes the pending promise when settled, and deliberately consumes watcher-setup rejection because setup can be interrupted while module evaluation remains pending.
• 🔴 (09:47) createPluginSources().read() dynamically imports #plugin-source, calls prepareSource(entrypoint, track), records an optional prepared.dispose, and caches prepared.load() before evaluation so unchanged failing modules do not repeat import-time side effects on every filesystem notification.
• 🔴 (09:47) Source fingerprints use Hash.sha256; files hash readFileSync(file), directories hash JSON.stringify(readdirSync(file).sort()), and inaccessible or missing paths use the literal digest "missing".
• 🔴 (09:47) localSource(spec, directory) recognizes file://, ./, ../, and absolute paths, resolving relative filesystem paths against directory; package-like specifiers return undefined.
• 🔴 (09:48) /home/byk/Code/opencode-lore/packages/core/src/log.ts treats debug as enabled only when LORE_DEBUG === "1" or case-insensitively equals "true"; LORE_DEBUG=0 and LORE_DEBUG=false must not enable stderr status output.
• 🔴 (09:48) traceDbQuery<T>(sql, fn) delegates to sink.withDbSpan(sql, fn) when available and otherwise calls fn() directly.
• 🔴 (09:48) Embedded full-screen TUI hosts must call silenceStderr() because any stdout/stderr byte from an in-process gateway corrupts the TUI; this applies to all logger levels, including error, even with LORE_DEBUG=1, while persistent file and registered LogSink output remain active.
• 🔴 (09:48) The stderr-suppression state uses the process-global globalThis key "__loreStderrSilenced" rather than a module-local variable so separately bundled copies of @loreai/core in the same main thread observe the same state.
• 🔴 (09:48) silenceStderr(silenced = true) is process-global and idempotent; isStderrSilenced() reports the current state. Standalone Lore CLI processes do not call it and retain stderr visibility.
• 🔴 (09:49) In /home/byk/Code/opencode-v2-pilot/packages/core/src/plugin.ts, active plugin slots preserve activation-order alignment, including failed setup slots, so prefix diffing remains index-aligned and an unchanged failed revision is not retried.
• 🔴 (09:49) Plugin activation state uses Semaphore.makeUnsafe(1) as the activation lock, Latch.make(true) for readiness, and a tokenized pending set; the latch closes while any readiness hold exists and reopens when the last token is released.
• 🔴 (09:49) Duplicate plugin IDs are fatal during Plugin.activate, raising Error("Duplicate plugin ID: ...").
• 🔴 (09:49) Activation keeps only the unchanged definition prefix alive. Registrations beyond the changed prefix are closed in reverse order, and healthy registrations reordered without revision changes do not authorize retrying an already-failed revision.
• 🔴 (09:49) When a new plugin revision fails to load, activation attempts to restore the previous healthy activation as a fallback; if restoration also fails, it logs "failed to restore plugin; deactivating".
• 🔴 (09:49) Transform failures mark the exact Activation with "Plugin disabled after ${failure.state}.transform failed. Check server logs for details." and a reference of the form err_${crypto.randomUUID().slice(0, 8)}, then enqueue a PendingFailure containing the plugin, scope, failure, refresh operation, reference, and readiness-hold release.
• 🔴 (09:49) Queued transform-failure processing relies on failure state being attached to the exact activation so an old queued item cannot disable a replacement.
• 🔴 (09:49) User finalizers may wait for readiness; exact lifecycle directive: “never join them under the activation lock or readiness hold.”
• 🔴 (09:49) Cleanup of a disabled plugin is scheduled in Effect.ensuring via Scope.close(item.scope, Exit.void) and Effect.forkScoped({ startImmediately: true }), so cleanup is still scheduled if an inventory observer fails and does not block under the lock or hold.
• 🔴 (09:49) Disabled-plugin cleanup failures log "failed to clean up disabled plugin" with "plugin.id", ref, and cause; non-interruption reporting failures log "failed to report disabled plugin"; item.release always runs in a final Effect.ensuring.
• 🔴 (09:49) Closing the plugin service acquires the activation lock, sets closed = true, clears pending readiness tokens, opens the readiness latch, clears active slots, and runs State.shutdown(Scope.close(scope, exit)).
• 🔴 (09:49) /home/byk/Code/opencode-v2-pilot/packages/plugin/src/source.bun.ts uses a monotonically incremented generation initialized with Date.now() and returns { version: String(++generation), load: () => Host.load(entrypoint) }.
• 🔴 (09:49) Bun source traversal excludes dependencies beneath node_modules, deletes require.cache[file] even for absent keys to invalidate rejected evaluations, tracks each visited file, and scans only files matching /\.[cm]?[jt]sx?$/.
• 🔴 (09:49) Bun dependency scanning uses Bun.Transpiler with loader selection tsx, jsx, ts, or js and target "bun"; scan failures produce an empty dependency list while the native runtime loader remains responsible for compilation, package resolution, import attributes, and error reporting.
• 🔴 (09:49) Bun source resolution uses createRequire(file).resolve(requested) for require-call imports and Bun.resolveSync(requested, path.dirname(file)) otherwise; unresolved local dependencies cause track(path.dirname(requested), true) so creation on a later save can trigger recovery.