Skip to content

fix(claude,server): keep Desktop applied-state markers honest across edits - #5337

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/claude-desktop-applied-state
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/claude-desktop-applied-state

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Native launch fallback: strip CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST only when the admission credential is ours, so a user-owned gateway keeps its guard instead of losing it while the credential stays.
  • Desktop profile rebuilds: carry appliedFingerprint/appliedAt only when the desired profile is unchanged, so a saved edit no longer reports the old on-disk config as applied. The management PUT discards client-supplied markers and restores the server-owned ones only for an identical result.
  • ocx sync: persist the fingerprint of the exact Desktop config just written through the config-mutation lock, so the applied marker advances with the bytes instead of going stale.

Verification

  • bun test tests/clients/desktop-profile.test.ts tests/clients/sync-client-integrations.test.ts tests/claude-integration/claude-cli.test.ts tests/claude-integration/claude-management-api.test.ts — all pass
  • bun x tsc --noEmit — clean

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 56 / 80

이 PR은 Claude Desktop의 “적용됨” 표시가 거짓말을 하던 구멍을 막는다. 예전에는 프로필을 다시 만들 때마다 appliedFingerprint/appliedAt을 무조건 이어 붙였다. 그래서 저장·드래그로 원하는 설정이 바뀌어도 화면은 예전 디스크 설정을 아직 적용된 것처럼 보여 줬다. 이제는 원하는 내용(assignments/defaults)이 같을 때만 표시를 남기고, 내용이 바뀌면 지운다. 관리 API PUT은 클라이언트가 보낸 표시를 버리고, 서버가 갖고 있던 표시만 “내용이 진짜로 같을 때” 다시 붙인다. ocx sync는 Desktop 파일을 쓴 직후, 그 바이트의 fingerprint를 설정 잠금(mutatePersistedConfig) 안에서 같이 저장해서 표시가 파일과 어긋나지 않게 한다. 네이티브 실행 쪽은 CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST를 무조건 지우지 않고, 우리가 넣은 admission 자격증명이 있을 때만 지운다. 사용자가 쓰는 게이트웨이·키가 남는데 호스트 관리 가드만 빠지던 경우를 막는다. 베이스는 dev다. 관련 테스트가 같이 바뀌었고, types.ts/config.ts 분할·중복 tip과는 무관하다.

라인 - sameProfileContent (src/claude/desktop-profile.ts): assignments는 키를 정렬해 비교하지만 defaultsJSON.stringify만 한다. 가족 키 순서가 달라지면 내용은 같은데 “바뀜”으로 보고 표시를 지울 수 있다. 지금은 같은 코드 경로에서 키 순서가 거의 같아서 잘 안 터지지만, 비교 방식이 한쪽만 정렬돼 있다.
라인 - Claude Desktop PUT (src/server/management/agent-settings-routes.ts): 클라이언트가 아무 것도 안 바꾼 것처럼 보여도, reconcile이 카탈로그에 새로 생긴 모델을 assignments에 넣으면 current.profile과 달라져 적용 표시가 지워진다. 의도된 동작일 수 있지만, “저장만 했는데 미적용으로 바뀜”으로 보일 수 있다.
라인 - syncEnabledClientIntegrations (src/server/management/config-routes.ts): Desktop 파일 쓰기와 fingerprint 저장이 한 트랜잭션이 아니다. 쓰기는 됐는데 mutatePersistedConfigunavailable이면 sync는 실패로 보고하고, 디스크 Desktop은 새 내용·설정의 표시는 옛 값으로 남을 수 있다.
라인 - moveDesktopRoute: 가족이 바뀌면 표시를 그냥 버리고, preserveDesktopAppliedState는 안 탄다. 내용이 바뀌는 경로라 결과는 맞고, reconcile/setDefault와 스타일만 다르다.

메인테이너의 판단이 필요한 지점

카탈로그에 모델이 늘어서 reconcile만으로 assignments가 커질 때도 적용 표시를 지울지, “사용자가 고른 라우팅만” 바뀌었을 때만 지울지. 지금 코드는 전자다.

sync에서 Desktop 쓰기는 됐는데 표시 저장만 실패했을 때, 지금처럼 전체 실패로 둘지, 부분 성공(쓰기는 됨·표시만 실패)으로 나눌지.

너의 추천

방향은 맞고 테스트도 핵심을 덮는다. defaults 비교도 키를 정렬하거나 DESKTOP_FAMILIES 순으로 맞춰라. sync의 쓰기/표시 어긋남은 메인테이너가 위 판단만 정하면 된다. 그다음 dev에 머지해도 된다. 닫을 중복·무효 tip은 없다.

이 댓글은 grok-bot이 작성했습니다

@github-actions github-actions Bot added the bug Something isn't working label Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

…edits

- Native launch fallback: strip CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST only when the admission credential is ours, so a user-owned gateway keeps its guard instead of losing it while the credential stays.

- Desktop profile rebuilds: carry appliedFingerprint/appliedAt only when the desired profile is unchanged, so a saved edit no longer reports the old on-disk config as applied. The management PUT discards client-supplied markers and restores the server-owned ones only for an identical result.

- ocx sync: persist the fingerprint of the exact Desktop config just written through the config-mutation lock, so the applied marker advances with the bytes instead of going stale.
@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5590 as a single related-function aggregate.

Source head: 3254286503f4c06a05c6aac2feadd9b4821977bb. Replacement head: 2f61b77dd54e07d67637224a3fc4ccf250ab4739.

The complete source contribution is carried as 4b90189 with matching zero-context stable patch ID d8d19e23f8d711b1c9812e674874cc45a36f5912. Final source review preserves profile-marker ownership, management PUT behavior, native gateway guard ownership and exact written-fingerprint persistence. The replacement additionally fixes concurrent desired-profile edits/deletions and the review concern about defaults key order. The final sync/profile suites passed 55 tests with 653 assertions on pinned Bun 1.4.0; type, structure, privacy and size checks passed. Full hosted CI, independent security review and the recorded Windows timing limits remain tracked on the draft replacement.

Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained.

@luvs01 luvs01 closed this Sep 22, 2026
lidge-jun added a commit that referenced this pull request Sep 23, 2026
… fixes (#5619)

* fix(cursor): bound capability reads and buffered tool budgets (#5533)

Carries #5533 (and the closed #5233 it consolidates) onto current dev.

Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(moonshot): bound normalized tool-schema expansion (#5547)

Carries #5547, which consolidates #5464 and the request-wide inline budget, onto current dev.

Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(moonshot): restore rejected inline budgets and charge nested growth once

A rejected sibling-reference expansion now restores the byte, node and expansion allowances it consumed, and outer growth no longer re-charges nested copies, so later independent expansions in the same request keep their allowance. Documents the provider-driven object type inference as a deliberate tradeoff and rewrites ADR-0355 in English.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* feat(reasoning): consolidate replay, opt-in tag parsing, and summary policy (#5566)

Carries #5566, which consolidates #5449, #5205 and #5491, onto current dev. The provider guide keeps the current bridge replay paragraph and adds the inline-tag and summary paragraphs.

Co-authored-by: Joonsuh Park <trckstr4422@gmail.com>
Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>
Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com>
Co-authored-by: Yum-wu <1172989563@qq.com>

* fix: bound Fernet slot runs, Kiro error-body read, and skill-path line slice (#5310)

Carries #5310 onto current dev. The follow-up commit makes the Fernet run cap fail closed and moves the Kiro regression out of the capped stream suite.

* docs(reasoning): reconcile inline-tag whitespace contract

Interleaved inline-tag parsing preserves answer whitespace; only Kiro single-block mode drops the whitespace after its leading block.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(responses): fail closed on Fernet run overflow and keep the Kiro suite under its cap

A slot with more than 64 structurally valid Fernet runs is now treated as unreadable or omitted as a whole, so no unexamined tail reaches the provider as text. The bounded Kiro fallback error-body regression moves byte for byte into a registered sibling file, and the Kiro, Responses and inbound contracts document the new bounds.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): scan inline think tags with a moving cursor

The parser copied, rescanned and reserved the whole remaining response after every block, so one upstream chunk carrying many short blocks cost quadratic work. It now scans each chunk from an offset and charges the translator budget only for retained carry: undecided leading input or a trailing tag fragment.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): keep undecided leading whitespace incremental

Before the format was decided, every content delta rebuilt, trimmed and re-reserved the whole leading prefix, so a stream of one-character whitespace deltas cost quadratic work. Leading whitespace is now kept in segments whose bytes are reserved once and joined only when the format is decided or the stream flushes.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(meta-muse): consolidate login admission and bounded response handling (#5591)

Carries #5591, which consolidates the closed #5234 and #5432, onto current dev. The provider contract keeps the inline-tag paragraph and adds the Meta Muse admission paragraph.

Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(claude-desktop): keep applied state consistent across profile edits (#5590)

Carries #5590, which consolidates the closed #5337, onto current dev.

Co-authored-by: Epinephrine <luvs01@hanmail.net>
Co-authored-by: luvs01 <luvs01@users.noreply.github.com>

* fix(claude-desktop): commit applied markers only over the observed baseline

Both Desktop writers, provider-change auto-apply and client sync, now capture the desired profile and its applied marker before the Desktop write and commit the new marker only if profile presence, content, fingerprint and timestamp are unchanged. A concurrent edit, deletion or newer marker keeps its state and the write reports a skipped marker. The provider-change path no longer saves a whole stale config snapshot.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude-desktop): commit profile edits against the persisted marker

The Desktop profile PUT built its response from an earlier snapshot and saved that whole snapshot, so a marker committed by another writer during the awaited state build could be replaced by an older one. The edit now commits in one persisted-config mutation that keeps the latest marker for unchanged content and answers 409 when the profile itself changed meanwhile. The Meta Muse overflow test now asserts that the bounded-body limit, not a generic failure, produced the error.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude-desktop): report an unreadable config separately from an edit conflict

A missing or invalid config now answers 500 with its reason; only a concurrent profile change or exhausted rebase answers 409.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* feat(desktop): consolidate consent-based runtime takeover and ownership contracts (#5564)

Carries #5564, which consolidates #5459 and #5457, onto current dev. The review screenshot stays in the pull request description rather than the tree.

Co-authored-by: jun <bitkyc08@gmail.com>
Co-authored-by: sanggyulee <andy53295774@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(desktop): bind takeover stop to the approved runtime and fail closed

Desktop takeover re-resolves ownership immediately before stopping and passes the approved PID, endpoint, config home, CLI version and compatibility token to an opt-in guarded stop. The guard is checked under the ownership mutation lease before any manager or signal stop; the approved PID and endpoint must settle and the service manager must then be proven inactive, otherwise the stop answers approval-changed or manager-still-active and the desktop neither waits for silence nor claims. Unreadable or unparseable stop output is terminal as well. A second unreadable service-state read now blocks takeover, Windows managing-CLI discovery follows PATHEXT with file-only candidates and refuses command-interpreter metacharacters, the claim refusal test uses real sandbox state, and the runtime and desktop contracts record that the claim token is a consistency check rather than consent proof. Plain ocx stop is unchanged.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(desktop): keep plain stop entry points and format the takeover changes

Desktop exit keeps its plain runtime_stop::run entry while takeover uses run_approved, AttachPlan::Ask no longer carries an unread field, the Rust changes follow rustfmt, the plain CLI stop path keeps its literal outcome return, the stop source oracles follow the reader and outcome union that now include the two guarded refusals, and the runtime contract fits its 600-line budget.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(desktop): run takeover seam tests without tokio macros and harden manager and shim checks

The two async takeover seam tests now run on the shell runtime already used by the crate instead of tokio test macros, which this crate does not enable. Windows command-shim probes refuse command-interpreter metacharacters in every recorded argument as well as the executable, and the guarded stop re-inspects the service manager identity immediately before the manager command, answering approval-changed without stopping if it moved.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(responses): keep effort-based reasoning visible after routing

Final-route normalization recomputed hideThinkingSummary without the validated active-effort condition, so routed Chat and Kiro requests with an active effort and an omitted summary still hid raw reasoning. It now uses the same predicate as the parser; explicit "none" and requests without an active effort stay hidden.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): match the running CLI case-insensitively only on Windows

On case-sensitive filesystems a PATH executable that differs only in case is a different file, so it must get its own version probe instead of reporting the running CLI version.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(meta-muse): require the dashboard session for manual login codes

The manual-code continuation now applies the same dashboard-session admission as the login start, so a management token cannot advance a pending Meta Muse login.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): reserve the joined leading-whitespace copy

Joining retained leading whitespace allocated a second copy outside the translator budget; the join is now reserved first and released once the segments are cleared.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): skip CLI probes for an absent runtime and treat failed systemd units as stopped

Resolve no longer spawns managing-CLI version probes when no runtime is live, since takeover is only offered for a live runtime. A systemd unit reported failed with no main PID is stopped, so a guarded stop that leaves it failed succeeds and a leftover failed unit does not block takeover.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): keep failed systemd units fail-closed and assess takeover only for a live runtime in tests

systemd can report failed before an automatic restart, so failed with no main PID is again treated as unknown rather than stopped. The resolve contract tests that assert ownership and takeover fields now use a live runtime, matching the skip of managing-CLI probes when no runtime is live.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

---------

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Co-authored-by: Epinephrine <luvs01@hanmail.net>
Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Joonsuh Park <trckstr4422@gmail.com>
Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>
Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com>
Co-authored-by: Yum-wu <1172989563@qq.com>
Co-authored-by: luvs01 <luvs01@users.noreply.github.com>
Co-authored-by: sanggyulee <andy53295774@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants