Skip to content

Instrument high-signal failure paths with structured logs - #96

Merged
jonnyparris merged 1 commit into
mainfrom
polylane/autofix/chat/puh59bqgn5oi-377143f
Aug 26, 2026
Merged

Instrument high-signal failure paths with structured logs#96
jonnyparris merged 1 commit into
mainfrom
polylane/autofix/chat/puh59bqgn5oi-377143f

Conversation

@polylane

@polylane polylane Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Generated by Polylane Autofix after jonnyparris/dodo was connected.

Several failure paths across the coding agent, MCP integration, GitHub verify-gate, skill registry, and user control emitted bare console.* calls with no session or run identifiers; this change routes them through the existing structured log() helper and attaches the identifiers available at each site, while narrowing two payloads that would have forwarded raw or opaque data into the log sink.

Each step below links to the record behind it.

Step Where to look
1. The repository was connected jonnyparris/dodo
2. Connecting the repository kicked off an autofix, and it succeeded Autofix fix_040247…
3. This pull request opened it for review this PR

Before / After

Before: An operator debugging a failed session or workflow saw unstructured console output, often without the session id, run id, repo, or skill identifier needed to correlate the failure to a request. Two paths additionally forwarded a raw GitHub error body or an opaque MCP tool object into the logs.

After: The same failures emit one structured JSON line through the project's existing log() helper with a constant message and the relevant identifiers in structured fields — session id, MCP server id/name, repo/branch/run id, skill dir/name, fork ids — and the GitHub and MCP paths forward only enumerated, sanitized fields. No new dependencies or logging stack were added.

Why this matters

  • src/github-api.ts verify-gate dispatch/poll failures now include runId, repo, branch, and status, so an operator investigating a stuck PR verify can identify the exact workflow run and HTTP status without replaying the queue or grepping raw console text. - src/coding-agent.ts LLM stream, chat, compaction, artifacts, MCP connect, and sync failures now carry sessionId, so a user-reported broken session can be traced from chat to the underlying failure path in one log query. - src/agentic.ts, src/mcp-client.ts, src/mcp-shared.ts, and src/mcp.ts MCP failures now carry MCP id/name and session/fork identifiers, so tool-availability and fork incidents point at the specific server or operation rather than requiring manual tool enumeration. - src/skill-registry.ts skill name mismatch and load failures now carry name, dirName, and dir, so a skill that silently fails to load can be identified by directory without rerunning discovery. - src/user-control.ts SharedIndex token deletion now logs the error object, so a cleanup failure surfaces instead of being silently swallowed.

What changed

  • src/agentic.ts: converted OAuth MCP tool-skip warnings to log() with serverId and name; removed forwarding of the full OAuthToolInfo object. - src/coding-agent.ts: converted nine bare console.* failure sites to log() with sessionId, count, MCP id/name, error, and stack where available. - src/github-api.ts: converted verify-gate dispatch/poll warnings to log() with runId, repo, branch, status; parses GitHub's API message instead of forwarding the raw response body. - src/mcp-client.ts: converted auth-scheme normalization info to log() with mcpId and mcpName. - src/mcp-shared.ts: converted service-mode fallback and chat-reply reaction failures to log() with session, space, and message identifiers plus the error. - src/mcp.ts: converted fork-session artifacts fork failure to log() with sessionId, newId, and the error. - src/skill-registry.ts: converted skill name mismatch and load failure warnings to log() with name/directory fields and the error. - src/user-control.ts: converted SharedIndex token-delete warning to log() with the error.

Validation

  • npm run typecheck — passed. - npm run lint — all modified files are clean; the only remaining errors are pre-existing in src/chat-monitor-agent.ts (unmodified by this diff). - npx vitest run test/github-actions-unit.test.ts test/mcp-acl-unit.test.ts test/skill-registry-unit.test.ts test/user-control.test.ts — 59 tests passed.
Root cause, safety reasoning, and scoping notes

Root cause

The repository already uses src/logger.ts's log(level, message, fields) structured JSON helper across most of the codebase; these paths still used bare console.*, so operators lost the structured sink and correlation fields. The change routes them through the existing idiom, and the one re-leveled site (syncSessionIndex) gains sessionId and the error object while remaining a non-fatal warning path.

Why it's safe

No secrets or credentials are logged. src/github-api.ts forwards only GitHub's sanitized message field, and non-JSON error bodies are omitted rather than logged raw. The opaque OAuthToolInfo object is no longer forwarded; only serverId and name are logged. All log messages are constant strings with dynamic values in structured fields, matching the repository's existing log() idiom. No new dependencies or logging backends were introduced. package-lock.json was reverted after the local dependency install used for validation, so no lockfile change ships.

8 files changed (+110/-33)
  • src/agentic.ts: modified, +10/-2
  • src/coding-agent.ts: modified, +33/-15
  • src/github-api.ts: modified, +25/-3
  • src/mcp-client.ts: modified, +5/-3
  • src/mcp-shared.ts: modified, +20/-4
  • src/mcp.ts: modified, +5/-1
  • src/skill-registry.ts: modified, +9/-4
  • src/user-control.ts: modified, +3/-1

view-autofix


Generated by Polylane.

Co-authored-by: polylane[bot] <277585245+polylane[bot]@users.noreply.github.com>
@polylane polylane Bot added polylane severity:low Polylane autofix severity: low labels Aug 26, 2026
@polylane

polylane Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

The verify check is failing on two steps, both pre-existing on main and unrelated to this PR's changes (.github/workflows/dodo-verify.yml). No code changes were made in response to these failures; fixing them belongs in a separate change against main rather than in this logging PR.

@jonnyparris
jonnyparris merged commit b9b212f into main Aug 26, 2026
3 of 4 checks passed
@jonnyparris
jonnyparris deleted the polylane/autofix/chat/puh59bqgn5oi-377143f branch August 26, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

polylane severity:low Polylane autofix severity: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant