mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-17 21:15:40 +02:00
fix(opencode): don't crash the whole session when plugins/lib is missing (#2538)
* fix(opencode): don't crash the whole session when plugins/lib is missing (#2530) * fix: address review feedback (trailing newlines, symptom wording) * fix(opencode): also lazy-load the store in ecc-hooks.ts plugin entrypoint * test(opencode): add regression coverage for missing/present plugins/lib in ecc-hooks * fix(opencode): guard diagnostic log() against unhandled rejection; tighten test assertion * fix(opencode): only publish store after init succeeds; scrub loader errors from warning; catch sync log throws * fix(opencode): scrub raw loader error from changed-files tool; add tool-level regression tests * fix(ci): make OpenCode checks cross-platform --------- Co-authored-by: haelyra <49814733+haelyra@users.noreply.github.com>
This commit is contained in:
co-authored by
haelyra
parent
afa34d1aca
commit
591ab5cbd3
@@ -16,11 +16,6 @@
|
|||||||
import type { PluginInput } from "@opencode-ai/plugin"
|
import type { PluginInput } from "@opencode-ai/plugin"
|
||||||
import * as fs from "fs"
|
import * as fs from "fs"
|
||||||
import * as path from "path"
|
import * as path from "path"
|
||||||
import {
|
|
||||||
initStore,
|
|
||||||
recordChange,
|
|
||||||
clearChanges,
|
|
||||||
} from "./lib/changed-files-store.js"
|
|
||||||
import changedFilesTool from "../tools/changed-files.js"
|
import changedFilesTool from "../tools/changed-files.js"
|
||||||
import dependencyAnalyzerTool from "../tools/dependency-analyzer.js"
|
import dependencyAnalyzerTool from "../tools/dependency-analyzer.js"
|
||||||
|
|
||||||
@@ -80,7 +75,6 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
type HookProfile = "minimal" | "standard" | "strict"
|
type HookProfile = "minimal" | "standard" | "strict"
|
||||||
|
|
||||||
const worktreePath = worktree || directory
|
const worktreePath = worktree || directory
|
||||||
initStore(worktreePath)
|
|
||||||
|
|
||||||
const editedFiles = new Set<string>()
|
const editedFiles = new Set<string>()
|
||||||
|
|
||||||
@@ -110,6 +104,37 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
const log = (level: "debug" | "info" | "warn" | "error", message: string) =>
|
const log = (level: "debug" | "info" | "warn" | "error", message: string) =>
|
||||||
client.app.log({ body: { service: "ecc", level, message } })
|
client.app.log({ body: { service: "ecc", level, message } })
|
||||||
|
|
||||||
|
// Loaded lazily (instead of via a top-level import) so that a missing or
|
||||||
|
// partially-installed `~/.opencode/plugins/lib` directory (e.g. an
|
||||||
|
// interrupted or partial ECC install on Termux/Android) only disables
|
||||||
|
// changed-files tracking, rather than throwing during module evaluation.
|
||||||
|
// This plugin is OpenCode's startup entry point, so a static import
|
||||||
|
// failure here previously crashed the whole plugin -- and with it, the
|
||||||
|
// entire OpenCode session -- before any hooks could load (see #2530).
|
||||||
|
let changedFilesStore: typeof import("./lib/changed-files-store.js") | undefined
|
||||||
|
try {
|
||||||
|
const store = await import("./lib/changed-files-store.js")
|
||||||
|
store.initStore(worktreePath)
|
||||||
|
changedFilesStore = store
|
||||||
|
} catch {
|
||||||
|
// Best-effort diagnostic only: deferred via .then() (rather than
|
||||||
|
// Promise.resolve(log(...))) so that even a *synchronous* throw inside
|
||||||
|
// log() -- not just an async rejection -- is caught here instead of
|
||||||
|
// escaping this catch block. The raw loader error is intentionally not
|
||||||
|
// included in the message since it can contain absolute filesystem
|
||||||
|
// paths; this whole block exists to guarantee startup resilience even
|
||||||
|
// when things go wrong.
|
||||||
|
Promise.resolve()
|
||||||
|
.then(() =>
|
||||||
|
log(
|
||||||
|
"warn",
|
||||||
|
"[ECC] changed-files tracking disabled: could not load the changed-files store. " +
|
||||||
|
"Run `ecc repair --target opencode` to restore the missing files. Other ECC hooks are unaffected."
|
||||||
|
)
|
||||||
|
)
|
||||||
|
.catch(() => {})
|
||||||
|
}
|
||||||
|
|
||||||
const normalizeProfile = (value: string | undefined): HookProfile => {
|
const normalizeProfile = (value: string | undefined): HookProfile => {
|
||||||
if (value === "minimal" || value === "strict") return value
|
if (value === "minimal" || value === "strict") return value
|
||||||
return "standard"
|
return "standard"
|
||||||
@@ -154,7 +179,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
*/
|
*/
|
||||||
"file.edited": async (event: { path: string }) => {
|
"file.edited": async (event: { path: string }) => {
|
||||||
editedFiles.add(event.path)
|
editedFiles.add(event.path)
|
||||||
recordChange(event.path, "modified")
|
changedFilesStore?.recordChange(event.path, "modified")
|
||||||
|
|
||||||
// Auto-format JS/TS files
|
// Auto-format JS/TS files
|
||||||
if (hookEnabled("post:edit:format", ["strict"]) && event.path.match(/\.(ts|tsx|js|jsx)$/)) {
|
if (hookEnabled("post:edit:format", ["strict"]) && event.path.match(/\.(ts|tsx|js|jsx)$/)) {
|
||||||
@@ -198,16 +223,16 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
) => {
|
) => {
|
||||||
const filePath = getFilePath(input.args)
|
const filePath = getFilePath(input.args)
|
||||||
if (input.tool === "edit" && filePath) {
|
if (input.tool === "edit" && filePath) {
|
||||||
recordChange(filePath, "modified")
|
changedFilesStore?.recordChange(filePath, "modified")
|
||||||
}
|
}
|
||||||
if (input.tool === "write" && filePath) {
|
if (input.tool === "write" && filePath) {
|
||||||
const key = input.callID ?? `write-${++writeCounter}-${filePath}`
|
const key = input.callID ?? `write-${++writeCounter}-${filePath}`
|
||||||
const pending = pendingToolChanges.get(key)
|
const pending = pendingToolChanges.get(key)
|
||||||
if (pending) {
|
if (pending) {
|
||||||
recordChange(pending.path, pending.type)
|
changedFilesStore?.recordChange(pending.path, pending.type)
|
||||||
pendingToolChanges.delete(key)
|
pendingToolChanges.delete(key)
|
||||||
} else {
|
} else {
|
||||||
recordChange(filePath, "modified")
|
changedFilesStore?.recordChange(filePath, "modified")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -413,7 +438,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
if (!hookEnabled("session:end-marker", ["minimal", "standard", "strict"])) return
|
if (!hookEnabled("session:end-marker", ["minimal", "standard", "strict"])) return
|
||||||
log("info", "[ECC] Session ended - cleaning up")
|
log("info", "[ECC] Session ended - cleaning up")
|
||||||
editedFiles.clear()
|
editedFiles.clear()
|
||||||
clearChanges()
|
changedFilesStore?.clearChanges()
|
||||||
pendingToolChanges.clear()
|
pendingToolChanges.clear()
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -428,7 +453,7 @@ export const ECCHooksPlugin: ECCHooksPluginFn = async ({
|
|||||||
let changeType: "added" | "modified" | "deleted" = "modified"
|
let changeType: "added" | "modified" | "deleted" = "modified"
|
||||||
if (event.type === "create" || event.type === "add") changeType = "added"
|
if (event.type === "create" || event.type === "add") changeType = "added"
|
||||||
else if (event.type === "delete" || event.type === "remove") changeType = "deleted"
|
else if (event.type === "delete" || event.type === "remove") changeType = "deleted"
|
||||||
recordChange(event.path, changeType)
|
changedFilesStore?.recordChange(event.path, changeType)
|
||||||
if (event.type === "change" && event.path.match(/\.(ts|tsx|js|jsx)$/)) {
|
if (event.type === "change" && event.path.match(/\.(ts|tsx|js|jsx)$/)) {
|
||||||
editedFiles.add(event.path)
|
editedFiles.add(event.path)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,11 +1,5 @@
|
|||||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
||||||
import {
|
import type { ChangeType, TreeNode } from "../plugins/lib/changed-files-store.js"
|
||||||
buildTree,
|
|
||||||
getChangedPaths,
|
|
||||||
hasChanges,
|
|
||||||
type ChangeType,
|
|
||||||
type TreeNode,
|
|
||||||
} from "../plugins/lib/changed-files-store.js"
|
|
||||||
|
|
||||||
const INDICATORS: Record<ChangeType, string> = {
|
const INDICATORS: Record<ChangeType, string> = {
|
||||||
added: "+",
|
added: "+",
|
||||||
@@ -26,6 +20,32 @@ function renderTree(nodes: TreeNode[], indent: string): string {
|
|||||||
return lines.join("\n")
|
return lines.join("\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Loaded lazily (instead of via a top-level import) so that a missing or
|
||||||
|
// partially-installed `~/.opencode/plugins` directory only breaks this one
|
||||||
|
// tool when it's actually invoked, rather than throwing during module
|
||||||
|
// evaluation. `tools/index.ts` re-exports every tool from a single barrel
|
||||||
|
// file, so a static import failure here previously took down the entire
|
||||||
|
// tools module -- and with it, the whole OpenCode session -- on the very
|
||||||
|
// first tool-loading pass (see #2530).
|
||||||
|
type ChangedFilesStore = typeof import("../plugins/lib/changed-files-store.js")
|
||||||
|
let changedFilesStorePromise: Promise<ChangedFilesStore> | undefined
|
||||||
|
|
||||||
|
async function loadChangedFilesStore(): Promise<ChangedFilesStore> {
|
||||||
|
if (!changedFilesStorePromise) {
|
||||||
|
changedFilesStorePromise = import("../plugins/lib/changed-files-store.js").catch(() => {
|
||||||
|
changedFilesStorePromise = undefined
|
||||||
|
throw new Error(
|
||||||
|
"changed-files tool: could not load the changed-files store. " +
|
||||||
|
"This usually means the ~/.opencode/plugins directory is missing or incomplete " +
|
||||||
|
"(an interrupted or partial ECC install can leave tools/ populated without plugins/). " +
|
||||||
|
"Run `node scripts/repair.js --target opencode` (or `ecc repair --target opencode`) " +
|
||||||
|
"from the ECC repo to restore the missing files."
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
return changedFilesStorePromise
|
||||||
|
}
|
||||||
|
|
||||||
const changedFilesTool: ToolDefinition = tool({
|
const changedFilesTool: ToolDefinition = tool({
|
||||||
description:
|
description:
|
||||||
"List files changed by agents in this session as a navigable tree. Shows added (+), modified (~), and deleted (-) indicators. Use filter to show only specific change types. Returns paths for git diff.",
|
"List files changed by agents in this session as a navigable tree. Shows added (+), modified (~), and deleted (-) indicators. Use filter to show only specific change types. Returns paths for git diff.",
|
||||||
@@ -40,6 +60,7 @@ const changedFilesTool: ToolDefinition = tool({
|
|||||||
.describe("Output format: tree for terminal display, json for structured data (default: tree)"),
|
.describe("Output format: tree for terminal display, json for structured data (default: tree)"),
|
||||||
},
|
},
|
||||||
async execute(args, context) {
|
async execute(args, context) {
|
||||||
|
const { buildTree, getChangedPaths, hasChanges } = await loadChangedFilesStore()
|
||||||
const filter = args.filter === "all" || !args.filter ? undefined : (args.filter as ChangeType)
|
const filter = args.filter === "all" || !args.filter ? undefined : (args.filter as ChangeType)
|
||||||
const format = args.format ?? "tree"
|
const format = args.format ?? "tree"
|
||||||
|
|
||||||
|
|||||||
@@ -305,6 +305,44 @@ npm pkg set packageManager="pnpm@8.15.0"
|
|||||||
rm package-lock.json # If using pnpm/yarn/bun
|
rm package-lock.json # If using pnpm/yarn/bun
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### OpenCode Fails to Start on Termux/Android
|
||||||
|
|
||||||
|
**Symptom:** Changed-files tracking silently stops working (a one-time
|
||||||
|
`[ECC] changed-files tracking disabled` warning appears in the OpenCode
|
||||||
|
logs), or (on older versions) `opencode` crashes on startup entirely with a
|
||||||
|
Bun `ResolveMessage`, e.g.:
|
||||||
|
|
||||||
|
```
|
||||||
|
ResolveMessage: Cannot find module '../plugins/lib/changed-files-store.js' from '.../.opencode/tools/changed-files.ts'
|
||||||
|
```
|
||||||
|
|
||||||
|
**Causes:**
|
||||||
|
- The `~/.opencode` install is missing or incomplete for this machine —
|
||||||
|
usually `tools/` and `plugins/` are present but `plugins/lib/` never
|
||||||
|
finished copying (an interrupted install, or a storage/permission hiccup
|
||||||
|
that's more common on Android's filesystem). Both the `changed-files` tool
|
||||||
|
and the `ecc-hooks` plugin depend on `plugins/lib/changed-files-store.js`;
|
||||||
|
since `ecc-hooks.ts` is OpenCode's plugin entry point (loaded once at
|
||||||
|
session startup, before `tools/index.ts`'s barrel file), a missing
|
||||||
|
dependency there used to crash the entire OpenCode session before any
|
||||||
|
hooks could load — not just the one tool.
|
||||||
|
|
||||||
|
**Solutions:**
|
||||||
|
```bash
|
||||||
|
# From the ECC repo, check for and repair missing/incomplete managed files
|
||||||
|
ecc doctor --target opencode
|
||||||
|
ecc repair --target opencode
|
||||||
|
|
||||||
|
# If that reports no drift but plugins/ is still missing on the device,
|
||||||
|
# re-run the ECC installer for the opencode target
|
||||||
|
```
|
||||||
|
|
||||||
|
**Note:** If you're also seeing `ProviderModelNotFoundError: Model not found: openai/gpt-5.5`
|
||||||
|
referencing `~/.config/opencode/oh-my-opencode-slim.json`, that file belongs to the
|
||||||
|
third-party [`oh-my-opencode-slim`](https://github.com/alvinunreal/oh-my-opencode-slim)
|
||||||
|
plugin, not ECC — ECC never writes to `~/.config/opencode/`. Fix the model prefix
|
||||||
|
(`opencode/...` instead of `openai/...`) there, or file it against that project.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Performance Issues
|
## Performance Issues
|
||||||
|
|||||||
@@ -84,6 +84,92 @@ async function main() {
|
|||||||
|
|
||||||
const { ECCHooksPlugin } = await loadPlugin()
|
const { ECCHooksPlugin } = await loadPlugin()
|
||||||
const tests = [
|
const tests = [
|
||||||
|
[
|
||||||
|
"plugin initializes and hooks stay usable when plugins/lib is missing",
|
||||||
|
async () => withTempProject([], async (projectDir) => {
|
||||||
|
const repoRoot = path.join(__dirname, "..")
|
||||||
|
const libDir = path.join(repoRoot, ".opencode", "dist", "plugins", "lib")
|
||||||
|
const backupDir = path.join(
|
||||||
|
repoRoot,
|
||||||
|
".opencode",
|
||||||
|
"dist",
|
||||||
|
"plugins",
|
||||||
|
"lib.missing-store-test-backup"
|
||||||
|
)
|
||||||
|
fs.renameSync(libDir, backupDir)
|
||||||
|
try {
|
||||||
|
const client = createClient()
|
||||||
|
const $ = createFailingShell()
|
||||||
|
|
||||||
|
// Plugin initialization must resolve even though changed-files-store.js
|
||||||
|
// cannot be found -- it must not throw and crash session startup (#2530).
|
||||||
|
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||||
|
|
||||||
|
const disabledWarnings = client.logs.filter(
|
||||||
|
(entry) =>
|
||||||
|
entry.level === "warn" &&
|
||||||
|
entry.message.includes("[ECC] changed-files tracking disabled") &&
|
||||||
|
entry.message.includes("ecc repair --target opencode")
|
||||||
|
)
|
||||||
|
assert.strictEqual(
|
||||||
|
disabledWarnings.length,
|
||||||
|
1,
|
||||||
|
"Expected exactly one warning when plugins/lib/changed-files-store.js cannot be loaded"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Every hook that touches the store must remain callable and must not throw.
|
||||||
|
await hooks["file.edited"]({ path: "src/example.ts" })
|
||||||
|
await hooks["tool.execute.after"]({ tool: "edit", args: { path: "src/other.ts" } }, {})
|
||||||
|
await hooks["session.deleted"]()
|
||||||
|
} finally {
|
||||||
|
fs.renameSync(backupDir, libDir)
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
],
|
||||||
|
[
|
||||||
|
"changed-files tracking records and clears through the plugin hooks",
|
||||||
|
async () => withTempProject([], async (projectDir) => {
|
||||||
|
const client = createClient()
|
||||||
|
const $ = createFailingShell()
|
||||||
|
|
||||||
|
const hooks = await ECCHooksPlugin({ client, $, directory: projectDir })
|
||||||
|
|
||||||
|
assert.ok(
|
||||||
|
!client.logs.some(
|
||||||
|
(entry) => entry.level === "warn" && entry.message.includes("changed-files tracking disabled")
|
||||||
|
),
|
||||||
|
"Did not expect a disabled warning when plugins/lib is present"
|
||||||
|
)
|
||||||
|
|
||||||
|
const storeUrl = pathToFileURL(
|
||||||
|
path.join(__dirname, "..", ".opencode", "dist", "plugins", "lib", "changed-files-store.js")
|
||||||
|
).href
|
||||||
|
const store = await import(storeUrl)
|
||||||
|
|
||||||
|
await hooks["file.edited"]({ path: "src/example.ts" })
|
||||||
|
assert.ok(
|
||||||
|
store
|
||||||
|
.getChangedPaths()
|
||||||
|
.some(
|
||||||
|
(entry) =>
|
||||||
|
entry.path === path.normalize("src/example.ts") &&
|
||||||
|
entry.changeType === "modified"
|
||||||
|
),
|
||||||
|
"Expected file.edited to record a change via the plugin hook"
|
||||||
|
)
|
||||||
|
|
||||||
|
await hooks["tool.execute.after"]({ tool: "edit", args: { path: "src/other.ts" } }, {})
|
||||||
|
assert.ok(
|
||||||
|
store
|
||||||
|
.getChangedPaths()
|
||||||
|
.some((entry) => entry.path === path.normalize("src/other.ts")),
|
||||||
|
"Expected tool.execute.after to record a change for the edit tool"
|
||||||
|
)
|
||||||
|
|
||||||
|
await hooks["session.deleted"]()
|
||||||
|
assert.ok(!store.hasChanges(), "Expected session.deleted to clear tracked changes")
|
||||||
|
}),
|
||||||
|
],
|
||||||
[
|
[
|
||||||
"shell.env detects project markers without shelling out to test -f",
|
"shell.env detects project markers without shelling out to test -f",
|
||||||
async () => withTempProject(
|
async () => withTempProject(
|
||||||
|
|||||||
@@ -234,6 +234,84 @@ async function main() {
|
|||||||
])
|
])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Test changed-files tool
|
||||||
|
if (tools.changedfiles) {
|
||||||
|
tests.push([
|
||||||
|
"changed-files: reports an actionable, scrubbed error when plugins/lib is missing",
|
||||||
|
async () => withTempProject([], async (projectDir) => {
|
||||||
|
const repoRoot = path.join(__dirname, "..")
|
||||||
|
const libDir = path.join(repoRoot, ".opencode", "dist", "plugins", "lib")
|
||||||
|
const backupDir = path.join(
|
||||||
|
repoRoot,
|
||||||
|
".opencode",
|
||||||
|
"dist",
|
||||||
|
"plugins",
|
||||||
|
"lib.missing-store-test-backup"
|
||||||
|
)
|
||||||
|
fs.renameSync(libDir, backupDir)
|
||||||
|
try {
|
||||||
|
const context = createMockContext(projectDir)
|
||||||
|
await assert.rejects(
|
||||||
|
() => tools.changedfiles.execute({}, context),
|
||||||
|
(error) => {
|
||||||
|
assert.ok(error instanceof Error)
|
||||||
|
assert.ok(
|
||||||
|
error.message.includes("ecc repair --target opencode"),
|
||||||
|
"Expected the error to point at the repair command"
|
||||||
|
)
|
||||||
|
assert.ok(
|
||||||
|
!error.message.includes(repoRoot),
|
||||||
|
"Error message must not leak the local filesystem path"
|
||||||
|
)
|
||||||
|
assert.ok(
|
||||||
|
!error.message.includes("Original error"),
|
||||||
|
"Error message must not include the raw underlying loader error"
|
||||||
|
)
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
)
|
||||||
|
} finally {
|
||||||
|
fs.renameSync(backupDir, libDir)
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
])
|
||||||
|
|
||||||
|
tests.push([
|
||||||
|
"changed-files: renders tracked changes once plugins/lib is present",
|
||||||
|
async () => withTempProject([], async (projectDir) => {
|
||||||
|
const repoRoot = path.join(__dirname, "..")
|
||||||
|
const storeUrl = pathToFileURL(
|
||||||
|
path.join(repoRoot, ".opencode", "dist", "plugins", "lib", "changed-files-store.js")
|
||||||
|
).href
|
||||||
|
const store = await import(storeUrl)
|
||||||
|
store.initStore(projectDir)
|
||||||
|
store.clearChanges()
|
||||||
|
store.recordChange("src/example.ts", "modified")
|
||||||
|
store.recordChange("src/new-file.ts", "added")
|
||||||
|
|
||||||
|
try {
|
||||||
|
const context = createMockContext(projectDir)
|
||||||
|
const result = await tools.changedfiles.execute({ format: "json" }, context)
|
||||||
|
const parsed = JSON.parse(result)
|
||||||
|
assert.strictEqual(parsed.changed, true)
|
||||||
|
assert.ok(
|
||||||
|
parsed.files.some(
|
||||||
|
(f) =>
|
||||||
|
f.path === path.normalize("src/example.ts") && f.changeType === "modified"
|
||||||
|
)
|
||||||
|
)
|
||||||
|
assert.ok(
|
||||||
|
parsed.files.some(
|
||||||
|
(f) => f.path === path.normalize("src/new-file.ts") && f.changeType === "added"
|
||||||
|
)
|
||||||
|
)
|
||||||
|
} finally {
|
||||||
|
store.clearChanges()
|
||||||
|
}
|
||||||
|
}),
|
||||||
|
])
|
||||||
|
}
|
||||||
|
|
||||||
// Run all tests
|
// Run all tests
|
||||||
let passed = 0
|
let passed = 0
|
||||||
let failed = 0
|
let failed = 0
|
||||||
|
|||||||
Reference in New Issue
Block a user