Repository navigation
fix(proxy): EPMCDME-15375 - refresh and clean up claude-code-otlp settings backup - #651
Merged
uladzislausvetlakou merged 13 commits intoOct 21, 2026
Merged
Conversation
Key the backup-refresh decision on whether CodeMie already owns part of settings.json, not on whether a backup file happens to exist. A stale backup from a prior connect no longer survives a disconnect + manual edit + reconnect cycle untouched.
…isconnect removeClaudeCodeOtlpConfig no longer restores raw backup bytes. It always derives the result from the current live settings file and deletes it outright when nothing foreign remains, so disconnect can no longer resurrect a value the user changed or removed after the connect that captured the backup. The backup is now also unlinked after every successful disconnect, not just the fully-empty case.
…disconnect claude-code-otlp's disconnect can no longer restore backup content, so the usedBackup field and its CLI warning were dead code.
alreadyManaged previously only looked at hook commands, so a settings file with codemie env keys (incl. the OTLP bearer token) but no codemie hook group was misclassified as unmanaged, causing a raw copyFile backup that could enshrine the live bearer token as the pre-connect original. Now also treated as managed, routing it through the strip-and-rebuild backup path.
…env-managed detection The CR-001 fix (checking for CODEMIE_ENV_KEYS presence) was too broad: under --force, a settings file with a genuinely foreign value under one of the 9 managed env var names was misclassified as already-managed, and stripCodemieManagedContent would then silently drop that foreign value from the rebuilt backup with no other copy. Require the existing value to match what this tool would write, so a foreign value falls through to the raw-copy backup path instead.
Technical analysis, plan, code-review verdicts (final + check), QA report, gate plan, and actual complexity assessment for the claude-code-otlp settings backup refresh/cleanup fix.
The check-round verdict file still showed request-changes from before CR-003 was fixed, since sdlc-light's rule skips a 3rd automated review round after a check-round request-changes in favor of a human decision. Updates the verdict to approve, moves CR-003 to finding_status:resolved, and records the decision history (final -> check -> human approval) so the committed artifact matches the actual, fully-resolved state.
Bondimm
reviewed
Oct 20, 2026
Bondimm
left a comment
Collaborator
There was a problem hiding this comment.
- CI hasn't run on this PR. The
CIruns forbd43e16fand for the earlier heads5c66845eand40ae4176are waiting for maintainer approval (action_required), so there are no Build / Test (Ubuntu) / Test (Windows) results yet. The branch is up to date withmainand merges cleanly. - Checked locally on Windows on
bd43e16f:npm run typecheck, eslint on the four changed.tsfiles, and commitlint on the commits and the title pass.vitest --project unit src/cli/commands/proxypasses (19 files, 451 tests). - Checked against EPMCDME-15375: I replayed the ticket's steps with the real connect and disconnect functions and a temporary home. The backup is gone after the first disconnect, holds
STEP: Bafter the reconnect, and the settings file is deleted after the final disconnect, so AC2 and AC3 hold. AC1 is only partly met (thread below). - Feature flag: none. Every
proxy disconnect --claude-code-otlpnow deletes the backup and never restores it. Revert: code only and clean. After a revert, a disconnect that leaves the file empty would again restore whatever backup is on disk. - Open PRs touching the same files: #618 conflicts with this PR in
disconnect-orchestrator.ts, and it still prints the "Restored the pre-connect backup" warning fromresult.usedBackup, which this PR removes. Whichever merges second needs to drop that block. #634 (more hook events in the same connector), #649 and #654 merge cleanly.
What needs rechecking after the next push:
- CI on the new head, once approved, including Test (Windows);
connect --forceover the user's own OTEL values, then disconnect: the user's values come back;- the backup after a connect while CodeMie is already connected.
…n every managed connect Addresses two review findings on PR codemie-ai#651 for EPMCDME-15375: - disconnect no longer permanently destroys a value --force overwrote at connect time (e.g. a user's own OTEL collector endpoint/headers). The backup's displaced value is put back into the live settings file, but only for keys still holding codemie's own current value - a value the user changed again after connect is left alone, preserving AC3. - the connect-side backup is now refreshed on every connect that modifies an already-managed settings file, not just the first one, closing the AC1 gap (CR-002). A value a prior --force connect displaced into the old backup is carried forward into the refreshed one so it isn't lost before the eventual disconnect. Both paths share a new withDisplacedEnvValues helper instead of duplicating the restore/carry-forward logic. Verified live against isolated $HOME reproductions (not just unit tests), including a full replay of the ticket's original repro sequence; full unit suite (5258 tests) and lint/typecheck/license-check all green.
…E-15375) Closes the remaining findings from the external PR codemie-ai#651 review: env-key ownership is now checked by value everywhere (not just presence), with OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_HEADERS each requiring the other to also look like codemie's own before either is treated as owned (closes the generic-Bearer-prefix and coincidental-suffix false positives on both sides). CODEMIE_ANALYTICS_PROJECT_FILTER is now gated on holding a well-formed allowlist value rather than stripped by key presence alone. CODEMIE_ENV_FIXED_VALUES is a real Record (no Partial + as-cast), so a future key added to CODEMIE_ENV_KEYS without a fixed value is now a compile error. readBackupSettings sanitizes non-string env values from a corrupted/hand-edited backup instead of letting them flow into the live settings file. A failed backup-file unlink (e.g. Windows EPERM) on either connect or disconnect is now best-effort and no longer fails the whole operation, since the important state change has already landed by that point. Documents the backup lifecycle in docs/COMMANDS.md. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
…PMCDME-15375) Shortens several JSDoc blocks and inline comments that were disproportionately long for the code they annotate, while keeping the non-obvious rationale each one exists to record. No behavior change. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
…nnector (EPMCDME-15375) Second pass: shortens isHookEntry/stripCodemieCommandsFromGroups JSDoc and four inline comments in writeClaudeCodeOtlpConfig/removeClaudeCodeOtlpConfig, plus the fs/promises mock-factory comment in the test file. No behavior change. Generated with AI Co-Authored-By: codemie-ai <codemie.ai@gmail.com>
8nevil8
approved these changes
Oct 21, 2026
gokhanozdemir
pushed a commit
to gokhanozdemir/codemie-code
that referenced
this pull request
Oct 21, 2026
…tings backup (codemie-ai#651) Co-authored-by: codemie-ai <codemie.ai@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes EPMCDME-15375:
codemie proxy connect/disconnect --claude-code-otlpleft a stalesettings.json.codemie-backupon disk — created only once on first connect and never refreshed, cleaned up only when the stripped settings became fully empty. A later disconnect could restore an outdated backup, reverting values the user deliberately changed or removed after connecting.Changes
removeClaudeCodeOtlpConfigno longer restores raw backup bytes on disconnect. It always derives the result from the current live settings file and deletes it outright when nothing foreign remains. The backup is unlinked after every successful disconnect, not just the previously-empty-only case.alreadyManagedalso considersCODEMIE_ENV_KEYSpresence inenv(not just hook commands), closing a path where a live bearer token could be raw-copied into the backup.--force) is preserved instead of silently dropped.usedBackupfield and its "Restored the pre-connect backup" CLI warning, since disconnect can no longer restore backup content.connect --forceno longer causes permanent, unrecoverable loss of a value it overwrites (e.g. a user's ownOTEL_EXPORTER_OTLP_ENDPOINT/OTEL_EXPORTER_OTLP_HEADERSpointing at their own collector).removeClaudeCodeOtlpConfigputs a displaced value back into the live file before deleting the backup — only for a key that's still holding codemie's own current value at disconnect time, so a value the user changed again after connect is correctly left alone (AC3).--forceconnect displaced into the old backup is carried forward into the freshly rebuilt one, via a sharedwithDisplacedEnvValueshelper.Additional hardening (follow-up round, on top of the above)
Prompted by an independent review of the above (findings #1–#13 against a prior head), plus a subsequent multi-lens re-review of the fix-up itself:
CLAUDE_CODE_ENABLE_TELEMETRY='1') is no longer misclassified as "already codemie's" and stripped wholesale; and a user's edit to a codemie-managed key after connect is no longer silently erased on disconnect. Both the connect-time backup build and the disconnect-time strip now gate everyCODEMIE_ENV_KEYSdeletion onisCodemieOwnedEnvValue, not key presence.OTEL_EXPORTER_OTLP_ENDPOINT↔OTEL_EXPORTER_OTLP_HEADERSpairing. Neither is treated as codemie-owned unless the other also looks like codemie's own — closes a false positive on the genericAuthorization=Bearer <token>prefix (a foreign collector using the exact same, non-secret default gateway key the daemon falls back to would otherwise have been misclassified) and the symmetric case of a foreign endpoint coincidentally sharing our path suffix.withDisplacedHookshelper (mirroringwithDisplacedEnvValues) restores a malformed (non-array) hook value a--forceconnect once displaced, gated the same way — only while the live file still currently holds codemie's own command for that event. Closes the "Known remaining gap" called out below in the previous round.CODEMIE_ANALYTICS_PROJECT_FILTERis now gated on holding a well-formed allowlist value before being stripped, instead of by key presence alone.CODEMIE_ENV_FIXED_VALUESis a realRecord<FixedCodemieEnvKey, string>(noPartial+as-cast), so a future key added toCODEMIE_ENV_KEYSwithout a fixed value is now a compile error instead of a silent runtime gap.readBackupSettingssanitizes non-stringenvvalues from a corrupted/hand-edited backup file instead of letting them flow into the live settings file.EPERMfrom a locked file) on either connect or disconnect is now best-effort (logged, not thrown) — it no longer fails the whole operation, since the important state change (settings.jsonitself) has already landed by that point.docs/COMMANDS.mdnow documents the backup lifecycle.Testing
proxycommand module: 463/463 passing.$HOME/CODEMIE_HOME(real user settings never touched) — the ticket's original end-to-end repro, every reviewer-reported finding individually reproduced and re-verified fixed, AC1–AC3 regression checks, and the new hardening round's specific false-positive/EPERM/corrupted-backup scenarios.Known remaining items (deliberately out of scope for this PR)
OTEL_EXPORTER_OTLP_ENDPOINT's andHEADERS' ownership checks remain pattern-based (fixed suffix/prefix + sibling pairing), since disconnect never reads live daemon state. This is believed sufficiently narrow after this round's hardening; a coincidental match against both halves of the pattern together is not considered a realistic collision.hooksblock (as opposed toenv, which is now sanitized) is not independently type-validated — hook values are intentionally heterogeneous by design (a displaced "malformed" value can legitimately be any non-array shape the user owned), so a blanket type filter here risks breaking legitimate displaced-hook recovery rather than only rejecting corruption.disconnect-orchestrator.tsfrom PR fix(proxy): resolve merge conflicts for #580 (proxy disconnect support) #618) are tracked separately and not part of this code change.Checklist
main