-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Feature]: diagnostic RepairContext: source path coverage, consoleErrors, and collection timeout #808
Description
Took a closer look at the diagnostic output from #802 as discussed. Really like the direction. Structured JSON with error + source + page state is the right abstraction for AI-driven repair. Here are three things I noticed while tracing the full repair loop.
1. adapter.source is missing for YAML commands
buildRepairContext reads cmd._modulePath, which is only set for manifest-loaded lazy TS commands (discovery.ts:204). YAML commands have no _modulePath, so their diagnostic output ships with empty sourcePath and source fields.
(TS commands work correctly in production: _modulePath is set and source is populated. Earlier test results showing TS commands also missing source were caused by a version mismatch between the global install and local dev build, now corrected.)
Also, when _modulePath is present it points to compiled JS under dist/clis/. An AI agent editing that file would lose changes on next build.
Suggestion: for YAML commands, fall back to cmd.source (which contains the YAML file path). For the .js→.ts mapping, consider resolving dist/clis/x.js back to clis/x.ts when the source file exists.
2. consoleErrors is always []
page.consoleMessages('error') (diagnostic.ts:53) calls the base-page implementation (base-page.ts:101-102) which returns a hardcoded empty array. No subclass overrides it. This means an AI agent reading the diagnostic would conclude "no console errors" even when errors did occur.
Suggestion: either wire up CDP Runtime.consoleAPICalled, or return undefined instead of [] so consumers can distinguish "not collected" from "collected, none found."
3. No timeout on page state collection
collectPageState runs four async operations via Promise.all (diagnostic.ts:48-54). Each has .catch(), but there's no overall timeout. If the CDP connection is hung after a partial failure, the entire error propagation blocks and the user sees the command freeze with no output.
Suggestion: wrap with a short timeout (e.g. 5s via Promise.race), fall back to emitting diagnostic without page state.
Impact-wise: item 1 only affects YAML adapters (TS adapters work fine). Item 3 is a small fix. Item 2 is more involved and fine as a follow-up.