docs: design the single-branch fork overlay + -rigel.N release scheme (RIG-3144) - #39
Open
rigel-mintaka wants to merge 7 commits into
Open
docs: design the single-branch fork overlay + -rigel.N release scheme (RIG-3144)#39rigel-mintaka wants to merge 7 commits into
rigel-mintaka wants to merge 7 commits into
Conversation
… (RIG-3144) The re-sync landed the fork on upstream v18.1.10, but the remaining fork machinery is scattered across separate held branches (release machinery, memtools flake fix, /metrics), which multiplies conflict resolution on every upstream re-sync. This record designs two Matt-ruled directions: 1. **Single overlay.** Carry the fork's entire divergence as one ordered, long-lived `fork/overlay` bookmark on top of the upstream base. Each re-sync resets main to the new upstream tip, rebases the whole delta as a unit, resolves once, and lands back in one merge-commit PR. The overlay shrinks as commits go upstream, until the fork needs no maintenance. 2. **`-rigel.N` releases.** Fork versions are `<upstream-base>-rigel.<N>` (e.g. `18.1.10-rigel.1`). Specifies every release-script change: `validateExplicitVersion` requires the suffix; the `git describe` glob isolates the fork's own `v*-rigel.*` tag lineage from upstream's `v*` tags (the load-bearing fix that keeps the unchanged `compareVersions` monotonic guard correct); `npmDistTag` maps `-rigel.N` to `latest`; the install-test sentinel matcher widens; `omp update` re-points to the fork scope. The record was red-teamed by an adversarial critique before freeze (found and fixed a real correctness bug in the install-test sentinel, plus the update-cli self-update gap). All load-bearing open questions were put to Matt and folded as decisions D1-D6. Ships as its own PR for review; the merge freezes it as the contract the overlay build executes against. Spec-impact: none. Refs RIG-3144 Co-authored-by: Matt Wilkinson <matt@rigel.build>
Addresses all 9 findings from the review pass (2 high, 4 medium, 3 low). Each was independently verified by execution before folding. **High.** The prescribed sentinel fix was wrong: widening `VERSION_SENTINEL_RE` with an optional `(?:_rigel_(\d+))?` group drops the capture, because `native-version.ts:10` rebuilds from groups 1-3 only, so the value truncates to `18.1.10`. That truncation fails again downstream with a misleading diagnosis (`align_native_manifest` rewrites package.json down, then `diskHasExpectedSentinel`'s `.includes()` matches the truncated sentinel as a substring and throws "omp was upgraded while this session was running"). The record now prescribes preserving the full suffix and the T6 assertion pins the exact value, since "returns the version" passed vacuously on the broken result. Task 2b also gained its real blast radius: 41 existing assertions across two `update-cli.test.ts` files hard-pin the upstream literals, and its Verify now runs the actual test bucket, since `ci:check:full` resolves to `check:ts` and executes no tests. **Medium.** The changelog degradation was understated: `ci-release-notes.ts:66`'s heading regex misses `## [18.1.10-rigel.1]`, so every fork release would publish EMPTY GitHub release notes; both regexes fold into Task 4. Fixed the `@rigelbuild/omp` vs `@rigelbuild/omp-coding-agent` literal contradiction, reconciled Task 1's commit count (8 source commits, consistently), and documented tag disposition across the admin reset. **Low.** Noted that the Task 2 smoke refuses at the CLI dispatch and never reaches the rewritten `cmdRelease` error text, corrected Task 1's Verify to name the suites, and repaired a mid-sentence line wrap. Spec-impact: none. Refs RIG-3144 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
upstream-omp/rig-3144-fork-overlay-design
branch
from
September 6, 2026 03:51
f35a370 to
c20163b
Compare
Review round 2 found the round-1 fix was itself defective: both prescribed mechanisms were wrong, and the record forbade the one approach that works. Blind `_`-to-`.` inversion yields `18.1.10.rigel.1`, not `18.1.10-rigel.1`; the sentinel encoding maps both `.` and `-` to `_`, so the inverse cannot recover the separator. Deriving the version from packages/natives/package.json inverts the harness's direction of use: run-ci.sh calls it to discover the addon's actual version so align_native_manifest can reconcile a divergent manifest, and the PR path deliberately fetches the upstream base addon, so manifest-derivation red-gates ci:test:install-methods on every post-release PR. Prescribe the mechanism that satisfies both paths: widen VERSION_SENTINEL_RE and fix the rebuild to carry group 4 in the same edit. Record both rejected mechanisms with their failure modes. Strengthen T6 with the plain-sentinel case, which is what actually fails a manifest-derived implementation. Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
marked this pull request as ready for review
September 6, 2026 04:29
Adds overlay row 10 for the RIG-3339 KDL fix, and a task-state note to the frozen re-sync record. The fable/mythos cache-read defect is upstream's, not the fork's: the file packages/catalog/src/compat/rules/classes/anthropic.kdl is the same git blob (eb2920e) at fork main 08e04ce and at upstream/main b2f25db. A reset sets main to the upstream tip, which still carries the unconditional cache-read 1.0, so a fix landed only as a commit on main is deterministically reverted by the next reset -- silently, with no conflict and no test red. That is the failure class this overlay exists to prevent, so the fix is carried as an overlay row instead. The row records the three ways to get it wrong: cost-patch is one cascade axis that replaces rather than merges, so the 5.1 block must restate all four cost fields; both fable and mythos are affected; and a generation-time fix is a no-op because buildModel reapplies costPatch at runtime. The re-sync record's T1-T8 boxes are the frozen contract and stay untouched, but they read as "nothing has happened" when T1/T2 have in fact executed while T3/T4/T5 have not. Mixed staleness is worse than uniform staleness, because a reader who spot-checks one box generalizes wrongly -- it has already cost a peer real work. A dated task-state note now records what executed, including that the tool-call-id re-lay must not be attempted and that /metrics acceptance is wiring rather than file existence. Co-authored-by: Matt Wilkinson <matt@rigel.build>
… (RIG-3144) Round-3 review of the row-10 delta found two gaps, both of which would have made the record self-defeating when executed literally. The prescription was incomplete. RIG-3339 is three commits, not one, and the record documented only the KDL change. The rebake of models.json is required, not redundant: getBundledModel serves the bundled row verbatim with no buildModel call, and packages/stats prices from exactly that row, so a KDL-only fix leaves the cost-reporting path billing four times the correct cache-read rate. That is the same silent no-conflict no-test-red failure the row exists to prevent. The third commit is the bake/rule parity guard, which is the only automated detector of a stale bundle because neither gen:models nor gen:compat runs in CI. The blanket warning against generated output is narrowed to what it means: do not patch the policy generator, but do rebake. Row 10 was carried by no task. Every other row has an owning task, so an agent executing the record would have produced an overlay branch without the fix. Task 1 now carries all three commits as step 8, and its commit count, interfaces line, verify command, and checkbox move from 8 to 11. Also pins the row to concrete SHAs instead of a branch name, which is a moving target in a record that freezes on merge; states the sim disposition; replaces an incomplete push-guard owner enumeration with the fact that matters; and collapses two duplicate status pointers in the re-sync record. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…-3144) The rebake bullet described fixing "the verbatim bundled-row path" without saying which rows that covers. Verified at the candidate rev: the bundle carries claude-fable-5, claude-fable-5-1 and claude-mythos-5, with no claude-mythos-5-1 row, so the rebake is a single line on fable 5.1 and mythos 5.1 is covered by the KDL commit alone. An implementer could otherwise read the bullet as promising bundle coverage for mythos that cannot exist, and a models.json check structurally cannot verify it. Also records the negative control: the sibling claude-fable-5 correctly stays at cacheRead 1, so a patch that moved both rows would be wrong while still passing a fable-5.1-only assertion. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…G-3144) Matt ruled this does not warrant an overlay entry: it is a small change to cache billing counts on one model family, and the plan is simply to open the fix upstream. Reverts the record to the nine-row, eight-commit shape. The technical analysis was correct but drove a severity judgement it did not support. The reset-reversion mechanism is real -- the KDL blob is identical at fork main and the upstream tip, so a main-only commit is reverted by the next reset -- but row-worthiness depends on the cost of what gets reverted, not on the certainty that it will be. This payload does not clear that bar. The fix ships through the human-action upstream-PR queue alongside the eval-coalesce and strict-null PRs, which is where it always belonged. The task-state note in docs/fork-resync.md is retained: it records what has executed across the whole re-sync and is independent of this row. Co-authored-by: Matt Wilkinson <matt@rigel.build>
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
2 participants
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.
The re-sync landed the fork on upstream v18.1.10, but the remaining fork machinery is scattered across separate held branches (release machinery, memtools flake fix, /metrics), which multiplies conflict resolution on every upstream re-sync. This record designs two Matt-ruled directions:
Single overlay. Carry the fork's entire divergence as one ordered, long-lived
fork/overlaybookmark on top of the upstream base. Each re-sync resets main to the new upstream tip, rebases the whole delta as a unit, resolves once, and lands back in one merge-commit PR. The overlay shrinks as commits go upstream, until the fork needs no maintenance.-rigel.Nreleases. Fork versions are<upstream-base>-rigel.<N>(e.g.18.1.10-rigel.1). Specifies every release-script change:validateExplicitVersionrequires the suffix; thegit describeglob isolates the fork's ownv*-rigel.*tag lineage from upstream'sv*tags (the load-bearing fix that keeps the unchangedcompareVersionsmonotonic guard correct);npmDistTagmaps-rigel.Ntolatest; the install-test sentinel matcher widens;omp updatere-points to the fork scope.The record was red-teamed by an adversarial critique before freeze (found and fixed a real correctness bug in the install-test sentinel, plus the update-cli self-update gap). All load-bearing open questions were put to Matt and folded as decisions D1-D6.
Ships as its own PR for review; the merge freezes it as the contract the overlay build executes against.
Spec-impact: none. Refs RIG-3144
Co-authored-by: Matt Wilkinson matt@rigel.build