fix(opencode): report invalid agent/mode configs instead of crashing or dropping them#29635
fix(opencode): report invalid agent/mode configs instead of crashing or dropping them#29635EClinick wants to merge 1 commit into
Conversation
…or dropping them An invalid file in agent/ threw ConfigInvalidError and aborted the whole load (no agents loaded); the same file in mode/ was silently dropped with no error. Both loaders now log, publish a session error, and skip just the bad file while loading the rest. Fixes anomalyco#27133
github-actions
Bot
added
the
needs:compliance
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: Found one potentially related PR: PR #29208: fix(config): catch parse errors gracefully during startup This PR appears to address similar config parsing error handling during startup. It may be worth checking if it overlaps with PR #29635's approach to handling invalid agent/mode configs gracefully. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improves error handling when loading agent and mode markdown configuration files so that invalid files are reported (via log + Bus event) instead of either crashing the entire load or being silently dropped.
Changes:
- Extracted a shared
reporthelper inagent.tsfor surfacing load failures via log and Bus event. - Replaced
ConfigParse.schemawithSchema.decodeUnknownExitinload()and added failure reporting in bothload()andloadMode(). - Added two tests verifying that invalid agent/mode files are reported and don't block sibling valid files.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/opencode/src/config/agent.ts | Centralizes error reporting and avoids crashing/silent-drop on invalid files. |
| packages/opencode/test/config/config.test.ts | Adds test coverage for invalid agent/mode file reporting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await Promise.resolve() | ||
| .then(() => Bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() })) | ||
| .catch(() => {}) |
| const parsed = Schema.decodeUnknownExit(Info)(config, { errors: "all", propertyOrder: "original" }) | ||
| if (Exit.isFailure(parsed)) { | ||
| await report("agent", item, `Failed to parse agent ${item}`, parsed.cause) | ||
| continue | ||
| } | ||
| result[config.name] = parsed.value |
| Bus.subscribe(Session.Event.Error, (evt) => { | ||
| const data = evt.properties.error?.data | ||
| if (data && "message" in data && typeof data.message === "string") errors.push(data.message) | ||
| }) | ||
| await Bun.sleep(10) | ||
| const config = await load() | ||
| await Bun.sleep(10) |
| // publish failure is swallowed — the log line is the durable record, the event is best-effort. | ||
| async function report(label: string, item: string, message: string, err: unknown) { | ||
| log.error(`failed to load ${label}`, { [label]: item, err }) | ||
| const { Session } = await import("@/session/session") |
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
github-actions
Bot
removed
the
needs:compliance
Fixes #27133
Both config loaders in
packages/opencode/src/config/agent.tshandled an invalid file inconsistently:load()(agents) calledConfigParse.schema(...), which throws. One bad file aborted the whole load, so no agents loaded and startup failed withConfigInvalidError.loadMode()(modes) usedSchema.decodeUnknownExitand skipped invalid entries with noelse, so a bad mode was silently dropped with no error.Now both decode with
decodeUnknownExit, and on failure they log, publish aSession.Event.Error, and skip just the bad file while loading the rest. I pulled the shared report-and-skip logic into a small helper, which also removes the duplicated.catchblock the two functions had.How I verified
bun typecheck(opencode package): clean.oxlinton both changed files: no new warnings.test/config/config.test.ts. Both fail on the current code (agent test throwsConfigInvalidError; mode test fails the "error was reported" assertion) and pass with the fix.bun test test/config test/agent: passing.bun dev <dir>against a dir with a valid + invalid file in bothagent/andmode/. Before, startup crashed; after, opencode starts, the valid entries load, and the invalid ones are reported instead of crashing or vanishing.