[codex] Structure keybindings failure diagnostics - #3418
Conversation
Co-authored-by: codex <codex@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved This PR restructures keybindings error diagnostics to use structured logging attributes instead of raw error details. The actual keybinding processing behavior is unchanged - only error reporting format. Changes are well-tested and consistently propagated across the codebase. You can customize Macroscope's approvability policy. Learn more. |
f83f34a
into
codex/audit-relay-errors-wave1
Issue
Keybindings persistence failures were all tagged as parse errors with free-form details and optional causes. Validation and startup logs also emitted formatted causes, raw entries, shortcut fields, and nested issue payloads, making the logs both unbounded and difficult to correlate structurally.
Root cause
The service conflated access, decoding, encoding, and filesystem failures in one parse-oriented representation, then recovered diagnostics by serializing the original values and causes at log boundaries.
Fix
KeybindingsConfigErrorwith a literaloperation, config path, stable derived message, and required exact causecatchTagsat startup and interrupt-awarecatchCauseboundaries for the watcherThis PR is stacked on #3242 so its client-side cause propagation remains intact.
Tests
vp test apps/server/src/keybindings.test.ts packages/contracts/src/keybindings.test.tsvp checkvp run typecheckNote
Medium Risk
Touches config persistence, startup, and client initial-sync error mapping; behavior is mostly diagnostic but decode/write failure shapes and log content change for operators.
Overview
Replaces parse-oriented
KeybindingsConfigError(formerlyKeybindingsConfigParseError) with anoperationliteral (access,read,decode,encode,write,prepare-directory), a requiredcause, and a stable derivedmessageinstead of free-formdetail.Runtime and user-facing issues now use fixed copy for malformed documents vs invalid entries; validation and startup logs emit bounded metadata (stages, indices, input shape flags, cause counts/tags) and omit raw entries, formatted causes, shortcut
key/when, and full issue payloads. Watcher/revalidation paths use interrupt-awarecatchCause; server startup logsoperationviacatchTags. Client RPC maps the renamed error tag unchanged for transient connection handling.Tests assert stable issue text, structured log fields, no sensitive command leakage, and repeatable decode failures on malformed config.
Reviewed by Cursor Bugbot for commit 63321c1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Structure keybindings failure diagnostics with typed operations and safe log attributes
KeybindingsConfigParseErrortoKeybindingsConfigErrorin contracts/src/keybindings.ts and replaces thedetail: stringfield with a typedoperationliteral union (access,read,decode,encode,write,prepare-directory).keybindingsCauseLogAttributes,keybindingsValidationLogAttributes) that emit cause summaries and input shape metadata without embedding raw entry values or full cause strings.MALFORMED_KEYBINDINGS_CONFIG_MESSAGE,INVALID_KEYBINDING_ENTRY_MESSAGE) so user-facing text no longer leaks internal cause details.KeybindingsConfigErrorby tag and logoperationinstead of a raw cause object.KeybindingsConfigErrorinstances now carryoperationinstead ofdetail; any code pattern-matching on the old tagKeybindingsConfigParseErroror readingdetailwill break.Macroscope summarized 63321c1.