Skip to content

[Tracking] vps2 chain: rules, port sources, chain-level rows #1992

Open

Description

@easonLiangWorldedtech

Part of the split-plan bookkeeping for the durable per-view state (vps2) chain chain.

What belongs here

  • chain-level rules and design decisions that no single unit owns
  • port sources and the per-unit derivation notes for content that is ported rather than rebased
  • chain-level review rows that CodeRabbit raises against a unit but whose cause spans units
  • vps2 unit issues exist for some findings (for example 1556, 1559, 1561, 1563, 1566); a unit-scope note belongs on that issue when one exists, otherwise here with the unit named in the first line

What does not belong here

  • unit-scope defects: those go on that unit's own issue
  • anything that requires a maintainer decision: raise it in the PR thread instead

Writing rules for this issue

  • no @mention in any comment here: mentions notify people who are not subscribed to this issue
  • do not reference a fork-repo issue number here; a bare number resolves against this repository and silently points at something unrelated
  • when recording an upstream pull request, write it without a leading hash (for example upstream PR 1066)
  • prefer editing an existing comment over posting a new one: edits notify nobody and recompute cross-references

Activity

  1. easonLiangWorldedtech commented on Oct 10, 2026

    @easonLiangWorldedtech
    ContributorAuthor

    Chain-level row for this chain: the Persistence Integrity class also appears on PR 1921.

    A rollback path that discards the boolean result of workspace.applyEdit() or TextDocument.save() reports a completed rollback while unapproved streamed content is still one save from disk. Acceptance: a refused restore fails the rollback and is not followed by a save; a save resolving false propagates; a test asserts the user does not get a successful rollback. Put the contract in the callee so callers cannot forget it.

    Reference: tracking comment 6094179445 (ownership and semantic baseline), shape shipped in commit 9c54765 on PR 1930.

  2. easonLiangWorldedtech commented on Oct 10, 2026

    @easonLiangWorldedtech
    ContributorAuthor

    Registered from the local queue after the fork tracking issue was retired. Three items, unit named in each first line. No
    mentions, no bare issue numbers, reference ids are bare comment ids.

    1. Unit: vps2 e2e unit (not yet opened) - chain level

    Playwright visual regression for the tab-title command contributions. Four editor and title bar commands were contributed
    for opening a Cline view in a new tab: their order and visibility, being hidden from the command palette, and the manifest
    entry need a committed snapshot plus a TabPanelProvider visual test in the e2e layer. That is the e2e unit's scope, not the
    host unit's, because the behaviour depends on the real extension host and the contributed menus.

    The two defects the host units actually fix are already proven at the lowest layer that would have failed for them, in
    src/core/webview/__tests__/webviewMessageHandler.spec.ts, each with a one-to-one negative control: a crafted
    updateSettings payload carrying a host-owned key, and profile deletion routing through the provider mutation path. A
    Playwright run would not catch either, so the coverage is additive rather than a substitute. Prior notes: 6081998034 and
    6081998560.

    2. Units: vps2 f2a and f2c - chain level, one fix in one unit

    renameApiConfiguration still writes through the settings manager directly. The handler deletes the old profile with a
    direct manager call instead of running the rename as one serialized provider-level mutation, so a rename racing another
    provider mutation can leave a sibling view pinned to a name that no longer exists.

    Acceptance: the rename goes through the same queue as upsert and delete, compensates on failure, and re-pins every sibling
    view that was pinned to the old name; red-first test plus a one-to-one negative control. Because the same handler code is
    present in both f2a and f2c, the rule applies: fix it in one unit and cite that unit from the other, so one function does
    not grow two different cleanups inside a stack that must merge in order.

    3. Unit: vps2 f2a - deferred by the lead, registered so it is not lost

    upsertProviderProfile and profile activation are still a non-atomic persistence path. A failure between the shared
    write and the per-view persist leaves the shared selection, the profile list, the mode mapping, the shared provider settings
    and the view-local durable state disagreeing.

    Acceptance: snapshot those five surfaces plus the prior profile settings and id, apply as one compensating transaction that
    rolls back on any failure, serialized against sibling view mutations; red-first test plus a one-to-one negative control.
    Deferred on 2026-10-10 while the unit is green across all required checks and the team is short on working context - the
    change spans five persistence surfaces, so starting it and abandoning it halfway costs more than waiting.

  3. easonLiangWorldedtech commented on Oct 10, 2026

    @easonLiangWorldedtech
    ContributorAuthor

    Chain-level registrations from the frozen fork tracking item

    Five items that were held in a local queue while the old fork tracking issue was frozen. Each is
    chain-level: it is not a defect in one unit's diff, or its owning unit is not the unit that found
    it. Nothing here is a progress update, and none of it was posted anywhere before.

    1. View identity needs a host-issued capability (chain-level decision, not a unit defect).
    The view-state registration accepts a renderer-supplied identifier and restores a persisted entry
    behind it. A consumer unit can normalize, bound and check liveness - PR 1921 does - but minting a
    capability and binding it to a specific view changes the identity primitive itself, which every
    unit of this chain inherits. The hunk predates the units: the adoption logic arrives with the
    chain backbone and the call site belongs to the unit whose scope names the webview launch path.
    Recorded while the old home was frozen as tracking item 6093611358.

    2. Playwright visual regression for the tab panel (owner: the e2e unit).
    Four editor/title tab commands: order and visibility, hidden from the command palette, committed
    snapshot, manifest entry. Acceptance: the e2e unit owns it. The lowest-layer defects in the same
    area are already covered by the webview message handler spec with one-to-one negative controls.
    Previously fork note 6081998034.

    3. Profile rename still bypasses the serialized provider mutation queue (owner: the host webview
    units, both per-view-state units).
    renameApiConfiguration writes through
    providerSettingsManager.deleteConfig directly instead of a serialized provider-level rename
    mutation. Acceptance: rename goes through the same queue as upsert and delete, compensates on
    failure, and re-pins every sibling view still pinned to the old name; red-first test plus
    one-to-one negative control.

    4. Activating upsert is still a non-atomic persistence path (owner: the host webview unit).
    Acceptance: snapshot the prior profile settings and id, profile list, mode mapping, shared
    selection, shared provider settings and view-local durable state, apply as one compensating
    transaction that rolls back on any failure, serialized against sibling view mutations; red-first
    test and one-to-one negative control. Deferred by the lead on 2026-10-10 because the change spans
    five persistence surfaces - recorded so the next unit does not rediscover it, not a request to
    start it.

    5. The repository lost its prettier ignore file on main (chain-wide, needs one decision).
    Between 09e7326 and d1f5845 the ignore file disappeared, so the format check inside the
    compile job now reaches 37 previously ignored files. It has already reddened two units in two
    different chains, each of which had to fix its own spec files to get green. Acceptance: either
    restore the ignore file or land the formatting, decided once, so no further unit rediscovers it.
    The unit-level fallout was fixed in PR 1928 commit b2b3b0d.

  4. easonLiangWorldedtech commented on Nov 10, 2026

    @easonLiangWorldedtech
    ContributorAuthor

    Split plan for PR 2226 (vps2/n1-view-identity), the view-identity unit. All issue and PR numbers below are Zoo-Code-Org/Zoo-Code. This plan is the whole proposal; the decisions marked as maintainer decisions stay in PR 2226's thread.

    Why it splits

    Two separate ceilings are breached, by two different measures, against two different bases.

    1. The CI mutation gate. At the merge base of the branch head bc724b8 with the current main tip 18861b2, which is e943530, the extension package prices 522 changed executable lines against the 500 limit. Tip to head, symmetric, the branch is 213 files +5,198/-8,201; that merge base to head it is 19 files +2,891/-54. The gate refuses, so this unit as it stands cannot pass mutation-diff, and 522 is 22 lines over, the smallest overshoot measured in this chain: the four other units that carry refusals price 1,707 (PR 1560), 2,130 (PR 1562), 2,227 (PR 1567) and 2,328 (PR 1921). Each of those is the extension total in that unit's own mutation-diff job log, priced with the merge ref as head and the main tip as its first parent.
    2. The series size budget. Those same 19 files, at +2,891/-54, total 2,945 additions plus deletions, against a soft ceiling of 400 per unit and a hard ceiling of 1,000 where hard means split again. That is the ceiling this plan answers.

    Reducing 22 lines would clear the CI gate on this unit alone and would not clear the size budget at all. It would also leave every unit above this one priced as it is now. A boundary is the response that lowers the price of everything above it; a shave is not.

    Two things the plan has to get right

    The two shared definitions land in the first unit. isNonPortableSettingsKey (src/core/config/ContextProxy.ts line 83 at this head) and ProviderSettingsNotFoundError (src/core/config/ProviderSettingsManager.ts line 59 at this head) are both absent from the main tip and present at the heads of the three later units: PR 1560 (isNonPortableSettingsKey in ContextProxy.ts 1 occurrence, importExport.ts 2, webviewMessageHandler.ts 2, extension/api.ts 3; ProviderSettingsNotFoundError in ProviderSettingsManager.ts 4 and its spec 2, ClineProvider.ts 3 and its spec 2), PR 1562 (1, 2, 0, 3 and 4, 2 plus ClineProvider.ts 5, ClineProvider.parallelMode.spec.ts 8, ClineProvider.spec.ts 2) and PR 1567 (1, 0, 0, 3 and 4, 2 plus ClineProvider.ts 5, ClineProvider.parallelMode.spec.ts 8, ClineProvider.spec.ts 3). Whichever unit lands first owns the definition and the later ones must rebase and drop their copies. A first unit without them ships three units that each carry their own copy through review, which is the duplicate-helper class this series already has a rule for.

    Order follows the declared merge order, not the cheapest diff. 16 of this unit's 19 files are also in PR 1560's changed set (merge base e943530, 36 files), 16 of 19 in PR 1562's (merge base 73931fe, 36 files) and 18 of 19 in PR 1567's (merge base 73931fe, 50 files). A split chosen to minimise each unit's own diff would cut across content that three other units already carry, and the rebase cost would land on them instead of here.

    Units

    The 19 files partition into five units. Sizes are additions plus deletions measured from merge base e943530 to head bc724b8; the five totals sum to 2,945, the whole priced content.

    unit files and hunks a+d ceiling status
    1, portable keys and typed missing-profile errors 10 files, 19 hunks: packages/types/src/global-settings.ts (2 hunks), packages/types/src/vscode-extension-host.ts (3), packages/types/src/tests/global-settings.test.ts (1), packages/types/src/tests/index.test.ts (2), src/core/config/ContextProxy.ts (2), src/core/config/ProviderSettingsManager.ts (3), src/core/config/importExport.ts (2), src/core/config/tests/ContextProxy.spec.ts (1), src/core/config/tests/ProviderSettingsManager.spec.ts (2), src/core/config/tests/importExport.spec.ts (1) 192 (+188/-4) under 400
    2, message handler and API surface 7 files, 16 hunks: src/core/webview/webviewMessageHandler.ts (3), src/core/webview/tests/webviewMessageHandler.spec.ts (3), src/extension/api.ts (2), src/extension/tests/api-configuration.spec.ts (5), src/extension/tests/api-start-new-task.spec.ts (1), src/tests/extension.spec.ts (1), src/services/language-models/copilotModelBroadcast.ts (1) 141 (+132/-9) under 400
    3, view identity, registration and record hygiene ClineProvider.ts hunks at new lines 74 through 1641 (+716/-7): the viewStateId, wroteUnderTemporaryViewStateId, pendingViewStateClaims, scheduledViewStateWrites, viewStateGeneration, viewLocalState, viewLocalStateWriteVersions and pendingViewStateRestorations fields, plus getPersistedViewStates, prunePersistedViewStates and isViewStateGenerationLive; plus ClineProvider.spec.ts hunks at 15 through 5268 and the added range 5720 to 6124, which is the new describe blocks setViewStateId and persisted viewStates record hygiene (+415/-12) 1,150 over 400, under 1,000, rationale in the unit body
    4, write queue lifetime and restoration ClineProvider.ts hunks at 1909 through 4682 (+40/-20), the call-site hunks including the posted-state fields at 4250 and 4299; plus the spec's added range 6125 to 7296 (+1,172/-0), which is the new describe blocks view state restoration retry, view state claim binding and view state writes that outlive the provider 1,232 over 400, under 1,000, rationale in the unit body
    5, view-local buffer and the configuration boundary ClineProvider.ts hunk at 6216 (+180/-0): assignOrClearField, bumpViewLocalStateWriteVersions, the mutation path and the restore-with-reapply guard; plus the spec's hunks at and after 7297 (+48/-2): the webview-originated configuration boundary block and the two Zoo Gateway profile hunks at 9664 and 9768 230 under 400

    Two of these cuts fall inside a single git hunk rather than between hunks. In the spec file one hunk carries 1,626 added new-side lines (six new describe blocks and 49 new test cases, and outside them the diff adds no test headers at all), so units 3, 4 and 5 must split that hunk at the describe boundaries named above, and the unit 3 boundary in ClineProvider.ts sits inside the 555-line hunk at 742. Each is a patch edit rather than a hunk selection, and they are the only places in this plan where a unit boundary is not a whole hunk.

    The 500-line CI ceiling is not re-derivable by hand, so each unit's price is measured at cut time with the recipe this campaign already uses: build the merge commit of the unit head onto the main tip and price that merge commit through scripts/stryker-diff.mjs, whose base substitution takes the merge commit's first parent. Spec-only content is not charged: the mutation fix in PR 1921, which is one spec file at +74/-16, prices 0 counted lines on the same rule.

    Topology and verification

    Branches are cut from main tip 18861b2 in the order 1, 2, 3, 4, 5, each unit's branch based on the one before it so that branch order and merge order agree, and the content source of record for every unit is PR 2226's head bc724b8. Before any unit is pushed: a per-unit split contract against that source, zdt split verify with its exit code reported, the unit's own mutation preflight on the unit delta, and the merge-commit price above. zdt align check runs across the five units before the integration PR opens, because units 3, 4 and 5 touch one buffer and mechanism divergence is what that produces.

    Not decided here

    Whether to publish five units or fold 4 and 5 into one, whether the intra-hunk cuts are acceptable to the reviewer who has to read them, and whether a maintainer-reviewed narrow exclusion is preferred over splitting at all. Those are maintainer decisions and they belong in PR 2226's thread. Nothing is pushed and no split unit opens until that is settled.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions