fix: harden transport, JSON fidelity, and profile recovery#80
Conversation
albert20260301
left a comment
There was a problem hiding this comment.
Reviewed the transport timeout boundaries, source-preserving JSON handling, and profile rollback paths. The implementation and regression coverage look sound; focused tests and type-aware lint pass locally.
| } catch (error) { | ||
| if (profileExists(target) && !profileExists(source)) { | ||
| renameSync(profileDir(target), profileDir(source)); | ||
| secrets.moveProfileSecrets(source, target, PROFILE_SECRET_ACCOUNTS); |
There was a problem hiding this comment.
I think a clearer way would be to use a random id for config and secret prefixes, and just point to the id in the profile config.
Or just have profile ids be these random ids, and have a profile display-label (more or less the same)
Map connect and body-read aborts to the CLI TimeoutError contract so streaming and buffered requests consistently exit with network status 9.
Ignore only genuinely missing files and wrap other read, write, update, and clear failures as ConfigurationError. Avoid reporting a successful clear when filesystem removal fails.
Keep append payload text unchanged after validation and serialize typed API integers beyond JavaScript's safe range as exact JSON numeric tokens. Preserve the same digits in query parameters.
Parse JSON-shaped string cells before human rendering and recursively mask password, token, secret, and API key fields while preserving unchanged JSON text.
Restore profile directories, credentials, and active selection when rename or delete steps fail. Make secret moves restore both source and target state after partial deletion failures.
Normalize signs and leading zeroes before raw numeric serialization so every typed integer produces valid JSON while preserving exact magnitude in request bodies and query parameters.
Apply the configured stream read deadline while consuming non-2xx response bodies and always clear the active timer after body completion or failure.
Classify failures as timeouts only when the CLI's internal deadline signal fired. Report peer-originated AbortError failures through the network error path.
Redact sensitive value spans with a source-aware JSON walker instead of parse-and-reserialize so unrelated large numeric identifiers and formatting remain exact.
Snapshot profile config bytes and permissions before quarantine removal, then recreate them during rollback even when recursive deletion removed part or all of the quarantined directory.
Recreate the source profile directory unconditionally after failed deletion, then restore config bytes only when the original profile had a config file.
Collect failed secret, directory, configuration, and active-profile restoration steps. Return a ConfigurationError with the original cause and actionable rollback details when recovery is incomplete.
Exercise nested arrays, escaped sensitive keys, structured sensitive values, whitespace and large-number preservation, and malformed JSON directly against the source-aware redactor.
Compose rename and delete recovery from named rollback steps, snapshot profile state through semantic types, and keep incomplete-recovery diagnostics centralized.
Move source-preserving JSON redaction into a small parser object so cursor ownership, recursive traversal, and lexeme preservation are explicit.
Share key parsing and temporary-file replacement between set and unset operations, while retaining protected file modes and actionable I/O errors.
Use one transport-error classifier and one active timeout setter across buffered and streaming requests, preserving peer-abort diagnostics and read deadlines.
Name canonical integer JSON and field formatting by their wire purpose, and separate property collection from final request-body serialization.
|
The stable-ID follow-up passes focused profile/secrets/configure tests and type-aware lint locally. GitHub currently reports this branch as conflicting with |
64156eb to
86bec67
Compare
The CLI had several correctness and reliability gaps at system boundaries:
Proposed solution
This branch hardens those boundaries by:
ConfigurationErrorwhile continuing to ignore only genuinely missing files.ConfigurationErrorthat retains the original failure as its cause and lists each failed recovery step.Testing
Added regression coverage for: