Skip to content

[Architecture]: define config rebase provenance for deletion vs unseen keys #1478

Closed

Description

@lidge-jun

Client or integration

OpenCodex Dashboard / management API

Area

Configuration persistence / provider and model management

Summary

The rebase-on-save landed for #1273 cannot distinguish two situations that look
identical from the merge's point of view:

  • a key is present on disk and absent in the live config because this writer**
    **deleted it;
  • a key is present on disk and absent in the live config because this writer**
    **never held it — another writer added it to the file after our config was
    loaded.

Absence alone does not say which. Both readings are defensible and each breaks
the other's scenario:

Scenario Deletion reading Ignorance reading
live deletes a field, disk still has it (PUT /api/grok/selection with []) correct: field is cleared wrong: field comes back
another writer adds a key after we loaded, we save something unrelated wrong: key is dropped correct: key survives

Two independent fixes for #1462 landed on opposite sides of this. d6e4545b2 on
dev restricts the rebase to keys the baseline knew about, which fixes the Grok
case; the contributor's later head d51bd2856 arms provenance on every config
load, which preserves disk-added keys but leaves the Grok case failing (9 pass /
1 fail on tests/grok-management-api.test.ts). Reimplementing the first rule
inside the second design breaks its own regression, an independently loaded config rebases a non-server save.

Both scenarios currently pass on dev, so this is not an active user-facing
defect — it is an unresolved rule that the next change in this area will hit.

Reproduction

Deletion case, against d51bd2856:

  1. const config = loadConfig(); config.grokExcludedModels = ["a"]; saveConfig(config);
  2. PUT /api/grok/selection with {"excluded": []} on a server whose live
    config was loaded before step 1.
  3. Expected grokExcludedModels to be gone; it is still ["a"].

Ignorance case, against a rebase restricted to baseline-known keys:

  1. const stale = loadConfig(); then, from a second loadConfig(), set
    disabledModels and saveConfig().
  2. stale.contextCapValue = 240_000; saveConfig(stale);
  3. Expected disabledModels to survive; it is dropped.

Version

2.12.x (dev at 62be62222)

Operating system

macOS 15 (arm64); not platform-specific

Provider and model

Not provider-specific

Logs or error output

deletion case:   Expected: undefined      Received: [ "a" ]
ignorance case:  Expected: ["disk/model"] Received: undefined

Anything else

Two directions look viable, and the choice belongs to whoever owns this
subsystem rather than to whichever patch merges last:

  1. Record deletion intent where the deletion happens. There are eight or more
    delete config.<field> sites across the management routes; each would mark
    the removal so the merge can tell intent from absence.
  2. Refresh the baseline on every cooperating write rather than arming it once
    at server startup, so a key added by another writer is no longer invisible to
    this instance.

Context and the full two-scenario analysis:
devlog/_plan/260811_bug_pair_landing/080_wp10_publication_plan.md.

Activity

  1. changed the title [-][Bug]: config rebase cannot distinguish a deleted key from one this writer never held[/-] [+][Architecture]: define config rebase provenance for deletion vs unseen keys[/+] on Aug 11, 2026
  2. added
    enhancementNew feature or request
    and removed
    bugSomething isn't working
    on Aug 11, 2026
  3. added
    roadmapAccepted long-term work item; needs its own project cycle
    and removed
    bugSomething isn't working
    on Aug 12, 2026
  4. Wibias commented on Aug 12, 2026

    @Wibias
    Contributor

    Triage classification update: this is not currently an active user-facing bug. The issue itself records that both conflicting scenarios pass on dev; the open question is the provenance rule we want future config writers to follow.

    I removed the bug label and marked this as roadmap/architecture work. Keep it open as the canonical design decision so the next config-rebase change does not accidentally choose one semantic by patch order.

  5. lidge-jun commented on Aug 14, 2026

    @lidge-jun
    OwnerAuthor

    Closing: this is an architecture decision record (ADR) task — defining config rebase provenance is a design exercise, not a bug fix. Track under roadmap/architecture.

  6. lidge-jun commented on Aug 14, 2026

    @lidge-jun
    OwnerAuthor

    Reopened — reclassification does not justify closing. Keeping open as a valid architecture decision item.

  7. lidge-jun commented on Aug 19, 2026

    @lidge-jun
    OwnerAuthor

    리뷰 · 우선순위 22 / 80

    rebase-on-save가 삭제한 키와, 이 작성자가 본 적 없는 디스크 신규 키를 구분하지 못한다는 아키텍처 이슈입니다. 둘 다 라이브에는 없고 디스크에는 있어 보입니다. 이슈 말대로 지금 사용자 대면 결함은 아니고, 다음 변경이 어느 쪽을 깨뜨릴지 규칙이 없다는 로드맵입니다.

    현재 dev의 src/config.ts는 liveConfigBaseline WeakMap과 armClaudeCodeBaseline()으로 서버 시작 시 베이스라인을 겁니다. 주석은 두 번째 loadConfig가 장기 서버 베이스라인을 갱신하면 안 된다고 합니다. reconcile 경로는 baseline/live/persisted를 비교합니다. 삭제 의도를 표시하는 provenance 필드는 이 심볼들에 없습니다.

    Grok 선택 비우기처럼 라이브가 지운 필드는 사라져야 하고, 다른 작성자가 넣은 disabledModels 같은 키는 unrelated save에 지워지면 안 됩니다. 두 수정이 반대편을 깨뜨린 역사가 이슈에 적혀 있습니다. 지금 양쪽 테스트가 통과해도, 규칙이 없으면 다음 패치가 한쪽을 다시 깨뜨립니다.

    해결방안: 삭제가 일어나는 관리 라우트에서 삭제 의도를 기록하거나, 협력 쓰기마다 베이스라인을 갱신하는 두 방향이 이슈에 있습니다. 어느 쪽을 제품 규칙으로 고를지는 이 서브시스템 소유자가 정해야 합니다. 지금 어느 한 쪽만 다시 넣으면 다른 회귀가 돌아옵니다. 패치는 추측하지 않음.

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

  8. lidge-jun commented on Aug 23, 2026

    @lidge-jun
    OwnerAuthor

    Backlog review 2026-08-23 — retained as roadmap, still accurate against dev.

    The gap is in the merge's inputs, not its code: rebase-on-save cannot tell "this writer deleted the key" from "this writer never saw the key", and no amount of care in the merge function recovers information the writer never recorded. Closing it would be pretending the ambiguity resolved itself.

    Fixing it means adding provenance to persisted config — a schema change with a migration and a compatibility story for every existing install. That earns its own cycle rather than a slot in a backlog sweep.

  9. lidge-jun commented on Aug 25, 2026

    @lidge-jun
    OwnerAuthor

    Closed on dev by #2613 (squash aacd6528d8fbb97dcf022d1420d262652a42f0ff).

    Deletion intent is explicit now. deleteConfigTopLevelKey() records the writer's intent alongside the delete, and a versioned configRebaseProvenance carries it across a stale whole-config rebase, so "the live writer deleted this" and "this key was never in the live config" are no longer the same observation. Ten modules, 27 named deletion paths plus one generic keyed path — enumerated in the design note rather than trusted to the estimate in the issue.

    Compatibility holds in both directions, each with its own test: absent provenance behaves exactly as today and grants no authority; future versions are preserved byte-for-value and grant no deletion authority either; assigning a deleted key clears its tombstone so intent cannot outlive itself.

    One pinned test changed — the Grok deletion case now expresses intent explicitly instead of relying on a bare delete, because that assertion was pinning the ambiguity this issue exists to remove. The Claude hand-edit assertion is untouched.

    Falsified by dropping the provenance-aware key set from the rebase, which reddens only the discrimination test while the other 42 stay green — that is what pins it as discrimination rather than a wider rebase.

    GitHub only auto-closes on merges into main; PRs here target dev, so closing manually.

  10. added a commit that references this issue on Aug 25, 2026
  11. lidge-jun commented on Aug 25, 2026

    @lidge-jun
    OwnerAuthor

    Landed via #2613 at aacd652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestlanded-via-maintainerOriginal PR closed after landing via a maintainer merge trainroadmapAccepted long-term work item; needs its own project cycle

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions