Skip to content

Commit b27a9bc

Browse files
authored
refactor(plugin): independent per-load registration with marker-based bootstrap idempotency (#352)
* refactor(plugin): independent per-load registration with marker-based bootstrap idempotency Each call to SystematicPlugin now runs initializePlugin independently and returns its own hooks surface. With multiple OpenCode plugin sources configured (project + user config), each source gets its own systematic_skill tool, its own config handler, and its own chat.system.transform closure. hasLoggedInit moves into per-init closure state, so the process emits N init log lines for N registrations — honest signal that init ran N times. Bootstrap content idempotency is now enforced at injection time, not init time. applyBootstrapContent walks output.system for any prior <SYSTEMATIC_WORKFLOWS>...</SYSTEMATIC_WORKFLOWS> block (non-greedy regex) and replaces it in-place; missing-block falls through to append. Under OpenCode's verified FIFO hook iteration, the last transform to run owns the final block — most-recently-registered plugin wins, which matches the project-after-user load order developers expect. This reverts the plugInOnce singleton from PR #335, which turned out to be over-correction. OpenCode registers tools per-source even with a shared hooks reference, so the singleton's init-dedup had no visible effect on the TUI tool catalog — what it did do in dev setups was silently collapse all loads onto whichever ran first, shadowing later sources. Also removes _resetPluginSingleton from the integration test setup; the singleton itself is deleted in a follow-up commit. 12 new behavior tests cover marker-replacement in every slot, non-greedy boundary, multi-block edge cases, per-invocation distinct references, and the cross-registration integration scenario. * refactor(plugin): delete plugin-singleton module after factory revert The plugInOnce abstraction and its test file are no longer reachable — src/index.ts stopped importing them in the prior commit, and the consumer test files (plugin.test.ts and opencode.test.ts) dropped the _resetPluginSingleton reset calls. Deletes 227 lines of now-dead infrastructure: the singleton helper, its five test cases, and the module-level exports. The architectural rationale lives in docs/brainstorms/2026-05-10-multi-load-plugin-registration-requirements.md and the implementation plan at docs/plans/2026-05-10-002-refactor-multi-load-plugin-registration-plan.md. * docs: sync AGENTS hierarchy and PR #335 solution after singleton removal Updates module count in AGENTS.md and src/lib/AGENTS.md from 16/14 to 15 (post-deletion of plugin-singleton.ts). Appends a 2026-05-10 follow-up section to the PR #335 solution doc noting the singleton was removed and documenting marker-based idempotency as the current correctness contract. Also commits the implementation plan with all four units marked complete. The plan was untracked locally during execution per project convention; landing it here gives the architectural inversion a visible paper trail. * fix(plugin): replace bootstrap marker regex with linear scan to close ReDoS The non-greedy regex /<SYSTEMATIC_WORKFLOWS>[\s\S]*?<\/SYSTEMATIC_WORKFLOWS>/ was flagged by CodeQL as polynomial-time on uncontrolled input — when the opening tag repeats and the closing tag is absent, the engine backtracks through every prefix. With per-load registration now letting any plugin source contribute system prompt fragments, this regex sees content the plugin itself didn't author. Replaces the regex with a small indexOf/slice helper. Fixed literal delimiters never needed regex; the linear scan is provably immune to ReDoS and unchanged in behavior for the existing seven marker-replacement tests. Adds a regression test that runs the helper against 10000 repeated opening markers with no closing tag and asserts completion in well under 1s. Closes CodeQL alerts #42 and #43.
1 parent 39c8ecf commit b27a9bc

9 files changed

Lines changed: 560 additions & 258 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ systematic/
4646
├── src/
4747
│ ├── index.ts # Plugin entry (default export)
4848
│ ├── cli.ts # CLI entry (list/convert/config commands)
49-
│ └── lib/ # 16 core modules (see src/lib/AGENTS.md)
49+
│ └── lib/ # 15 core modules (see src/lib/AGENTS.md)
5050
├── skills/ # 45 bundled skills (SKILL.md format)
5151
├── agents/ # 51 bundled agents (6 categories: design/docs/document-review/research/review/workflow)
5252
├── docs/ # Starlight docs workspace (see docs/AGENTS.md)

docs/plans/2026-05-10-002-refactor-multi-load-plugin-registration-plan.md

Lines changed: 250 additions & 0 deletions
Large diffs are not rendered by default.

docs/solutions/integration-issues/opencode-plugin-factory-duplicate-registration-2026-05-04.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,9 @@ const SystematicPlugin: Plugin = async (input) => {
142142
- Plan document: `docs/plans/2026-05-01-001-fix-idempotent-plugin-registration-plan.md`
143143
- Adjacent upstream pattern (idempotence under cache-busting hook injection): <https://github.com/alvinunreal/oh-my-opencode-slim/issues/415>
144144
- Precedent that initially appeared transferable but used the insufficient whole-hooks pattern: `opencode-copilot-delegate/src/runtime/plugin-singleton.ts`
145+
146+
## 2026-05-10 follow-up: singleton removed
147+
148+
The `plugInOnce` singleton introduced in PR #335 was reverted in a later PR. The duplicate-tool-entry concern that motivated the singleton turned out to be a non-issueOpenCode registers tools per-source regardless of whether the hooks reference is shared, so the singleton's deduplication of the init work had no visible effect on the TUI tool catalog. What it DID do in dev setups with multiple plugin sources was collapse all loads onto whichever ran first, silently shadowing later sources.
149+
150+
The real correctness contract is now marker-based idempotency in `applyBootstrapContent`: each registration applies its bootstrap content by walking `output.system` for any prior `<SYSTEMATIC_WORKFLOWS>...</SYSTEMATIC_WORKFLOWS>` block and replacing it in-place. Under OpenCode's FIFO hook iteration, the last transform to run owns the final block — most-recently-registered plugin wins. The architectural rationale is captured in `docs/brainstorms/2026-05-10-multi-load-plugin-registration-requirements.md` and the implementation plan at `docs/plans/2026-05-10-002-refactor-multi-load-plugin-registration-plan.md`.

src/index.ts

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
} from './lib/bootstrap.js'
99
import { loadConfig } from './lib/config.js'
1010
import { createConfigHandler } from './lib/config-handler.js'
11-
import { plugInOnce } from './lib/plugin-singleton.js'
1211
import { createSkillTool } from './lib/skill-tool.js'
1312

1413
const __dirname = path.dirname(fileURLToPath(import.meta.url))
@@ -18,12 +17,36 @@ const bundledSkillsDir = path.join(packageRoot, 'skills')
1817
const bundledAgentsDir = path.join(packageRoot, 'agents')
1918
const bundledCommandsDir = path.join(packageRoot, 'commands')
2019
const packageJsonPath = path.join(packageRoot, 'package.json')
21-
let hasLoggedInit = false
2220

23-
const applyBootstrapContent = (
21+
const BOOTSTRAP_MARKER_OPEN = '<SYSTEMATIC_WORKFLOWS>'
22+
const BOOTSTRAP_MARKER_CLOSE = '</SYSTEMATIC_WORKFLOWS>'
23+
24+
const findBootstrapMarkerBlock = (
25+
entry: string,
26+
): { start: number; end: number } | null => {
27+
const start = entry.indexOf(BOOTSTRAP_MARKER_OPEN)
28+
if (start === -1) return null
29+
const closeStart = entry.indexOf(
30+
BOOTSTRAP_MARKER_CLOSE,
31+
start + BOOTSTRAP_MARKER_OPEN.length,
32+
)
33+
if (closeStart === -1) return null
34+
return { start, end: closeStart + BOOTSTRAP_MARKER_CLOSE.length }
35+
}
36+
37+
export const applyBootstrapContent = (
2438
output: { system: string[] },
2539
content: string,
2640
): void => {
41+
for (let i = 0; i < output.system.length; i++) {
42+
const entry = output.system[i]
43+
const block = findBootstrapMarkerBlock(entry)
44+
if (block !== null) {
45+
output.system[i] =
46+
entry.slice(0, block.start) + content + entry.slice(block.end)
47+
return
48+
}
49+
}
2750
if (output.system.length > 0) {
2851
output.system[output.system.length - 1] += `\n\n${content}`
2952
} else {
@@ -42,15 +65,8 @@ const getPackageVersion = (): string => {
4265
}
4366
}
4467

45-
/**
46-
* Build the plugin hook surface for a single OpenCode plugin invocation.
47-
*
48-
* Extracted into a named initializer so the default export can wrap it through
49-
* `plugInOnce(...)`, making duplicate factory calls in the same process
50-
* converge on the same hooks promise. See `src/lib/plugin-singleton.ts` for
51-
* the duplicate-load justification.
52-
*/
5368
const initializePlugin = async ({ client, directory }: PluginInput) => {
69+
let hasLoggedInit = false
5470
const config = loadConfig(directory)
5571
// Snapshot bootstrap once per plugin init so the cached system prefix stays
5672
// stable across requests. Custom bootstrap file edits take effect on restart.
@@ -129,13 +145,7 @@ const initializePlugin = async ({ client, directory }: PluginInput) => {
129145
}
130146

131147
const SystematicPlugin: Plugin = async (input) => {
132-
const { hooks } = await plugInOnce({
133-
doInit: () => initializePlugin(input),
134-
})
135-
// hooks is the real plugin hook surface on every invocation — first and
136-
// duplicate alike. Returning it unconditionally keeps every configured
137-
// plugin source functional instead of suppressing duplicates with `{}`.
138-
return hooks
148+
return initializePlugin(input)
139149
}
140150

141151
export default SystematicPlugin

src/lib/AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# src/lib — Core Implementation
22

3-
14 modules implementing plugin logic: discovery, conversion, config, schema validation, and tool registration.
3+
15 modules implementing plugin logic: discovery, conversion, config, schema validation, and tool registration.
44

55
## Data Flow
66

src/lib/plugin-singleton.ts

Lines changed: 0 additions & 109 deletions
This file was deleted.

tests/integration/opencode.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import os from 'node:os'
44
import path from 'node:path'
55
import type { Config, PluginInput } from '@opencode-ai/plugin'
66
import SystematicPlugin from '../../src/index.js'
7-
import { _resetPluginSingleton } from '../../src/lib/plugin-singleton.js'
87

98
const OPENCODE_AVAILABLE = (() => {
109
const result = Bun.spawnSync(['which', 'opencode'])
@@ -109,7 +108,6 @@ describe('SystematicPlugin config hook integration', () => {
109108
let originalHomedir: typeof os.homedir
110109

111110
beforeEach(() => {
112-
_resetPluginSingleton()
113111
originalHomedir = os.homedir
114112
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'systematic-plugin-'))
115113

@@ -126,7 +124,6 @@ describe('SystematicPlugin config hook integration', () => {
126124
afterEach(() => {
127125
// Restore os.homedir before cleanup so nothing snags on a deleted dir.
128126
os.homedir = originalHomedir
129-
_resetPluginSingleton()
130127
delete process.env.OPENCODE_CONFIG_DIR
131128
delete process.env.XDG_DATA_HOME
132129
fs.rmSync(tempDir, { recursive: true, force: true })

tests/unit/plugin-singleton.test.ts

Lines changed: 0 additions & 118 deletions
This file was deleted.

0 commit comments

Comments
 (0)