From 03c971d29c3c3a9fed7222020e2e20deb832131a Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 4 Sep 2026 18:43:18 +0900 Subject: [PATCH 1/6] fix(providers): advertise the reasoning-effort ladder for native Anthropic models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Native Anthropic models reached Aside — and every other client that keys its effort control off `reasoningEfforts` — with no reasoning-effort control at all, while the SAME Claude models routed through `cursor` or `google-antigravity` had one. The discriminator was never the model: the `anthropic` and `anthropic-apikey` provider entries declared `models` and `modelContextWindows` but no `modelReasoningEfforts`, so the ladder resolved to undefined and the catalog omitted it. The adapter has honored effort all along (`output_config.effort` for adaptive families, translated `thinking.budget_tokens` for the rest), so this was missing advertisement rather than missing capability. Two adjacent defects would have made the fix only half work, both found by adversarial plan audit rather than by the original symptom: - `derive.ts` copied the registry ladder only when the persisted provider had NO map, so one customized model hid the registry's knowledge of every other model. That split the planes apart: routing merges these maps per key, so the wire honored the effort while `/v1/models` and the exports showed nothing. Now a per-model fill, matching `modelInputModalities` directly above it. - `capability.ts` never consulted `noReasoningModels`, unlike every other reader, and discarded a defined-but-empty ladder — which made the evaluator record the permissive "unknown" instead of a known negative. A model the operator explicitly disabled reasoning for could satisfy an effort requirement. Both corrected. The ladder is an opencodex abstraction, not a claim of uniform native `output_config.effort` support: Anthropic documents low|medium|high|max for the 4.6 models and no effort parameter for haiku-4-5, and the adapter's budget translation is what makes five rungs meaningful there. `minimal`, `none` and `ultra` are deliberately excluded — each would offer a control that does not do what it says. Verification: every new assertion was proven red before the change and green after by temporarily reverting the production edit. The management-client-config case is the one that covers the real seam end to end (registry -> enrich -> CatalogModel -> ManagementModelRow -> toExportModel -> buildClientConfig), since fixture-based tests would stay green if a middle hop dropped the field. Confirmed live on an isolated scratch proxy: all nine `anthropic/claude-*` rows now report `supports_reasoning_effort` with the five-rung ladder, and the Aside document emits `reasoning: true` with a `thinkingLevelMap`. --- .../000_research.md | 179 ++++++++++++ .../010_registry_ladder.md | 254 ++++++++++++++++++ .../020_verification_and_pr.md | 71 +++++ .../030_related_empty_ladders.md | 50 ++++ src/providers/derive.ts | 9 +- src/providers/registry.ts | 30 +++ src/routing/capability.ts | 21 +- tests/aside-client.test.ts | 42 +++ tests/management-client-config-route.test.ts | 55 ++++ tests/provider-registry-parity.test.ts | 27 ++ tests/provider-static-model-discovery.test.ts | 31 +++ .../routing-capability-model-matching.test.ts | 70 +++++ 12 files changed, 834 insertions(+), 5 deletions(-) create mode 100644 devlog/_plan/260904_anthropic_effort_ladder/000_research.md create mode 100644 devlog/_plan/260904_anthropic_effort_ladder/010_registry_ladder.md create mode 100644 devlog/_plan/260904_anthropic_effort_ladder/020_verification_and_pr.md create mode 100644 devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md diff --git a/devlog/_plan/260904_anthropic_effort_ladder/000_research.md b/devlog/_plan/260904_anthropic_effort_ladder/000_research.md new file mode 100644 index 0000000000..ad00485888 --- /dev/null +++ b/devlog/_plan/260904_anthropic_effort_ladder/000_research.md @@ -0,0 +1,179 @@ +# 000 — Research: native Anthropic models advertise no reasoning-effort ladder + +## Reported symptom + +Connecting opencodex to Aside shows a reasoning-effort control for every routed +model EXCEPT the Claude ones. The user's phrasing — "claude 모델들만 추론강도 +조절이 나타나지 않는다" — is precise but the word "Claude" is a red herring, and +that matters for the fix: Claude models routed through OTHER providers are fine. + +## Evidence + +`~/.aside/u/0/models.json`, the file Aside actually reads, on 2026-09-04: + +``` +anthropic/claude-fable-5-1 reasoning=None thinkingLevelMap=- +anthropic/claude-opus-4-6 reasoning=None thinkingLevelMap=- +anthropic/claude-opus-5 reasoning=None thinkingLevelMap=- +cursor/claude-fable-5-1 reasoning=True thinkingLevelMap=yes +google-antigravity/claude-opus-4-6-thinking reasoning=True thinkingLevelMap=yes +``` + +`cursor/claude-fable-5-1` and `anthropic/claude-fable-5-1` are the SAME model. +One has an effort control and the other does not, so nothing about Claude itself +can explain it. The discriminator is the provider entry. + +`GET http://127.0.0.1:10100/v1/models` on the live proxy agrees, which locates +the defect upstream of Aside and upstream of the exporter: + +``` +anthropic/claude-fable-5-1 supports_reasoning_effort=absent reasoning_efforts=[] +cursor/claude-fable-5-1 supports_reasoning_effort=True reasoning_efforts=[low,medium,high,xhigh,max] +google-antigravity/claude-opus-4-6-thinking supports_reasoning_effort=True reasoning_efforts=[low,medium,high,max] +``` + +## Causal chain + +Corrected after audit. An earlier draft of this document routed the export +through `src/routing/capability.ts:216`; that function +(`candidateCapabilityEvidence`) feeds POLICY ROUTING and never reaches the +catalog or `ExportModel`. Naming the wrong hop would have produced a test that +guards a seam the bug does not live in, so the real chain is recorded here: + +1. `src/providers/registry.ts` — the `anthropic` entry (line ~1330) and + `anthropic-apikey` entry (line ~1346) declare `models`, + `modelContextWindows` and `defaultModel`, but no `modelReasoningEfforts`. + Every peer provider that shows effort declares one: `cursor` line 1162, + `google-antigravity` line 1861, `xai` line 1281, `kimi` line 1384. +2. `captureProviderGather` clones the configured provider and calls + `enrichProviderFromRegistry` — `src/codex/catalog/provider-fetch.ts:411-420`. + This is where a registry ladder would be merged into the live provider. +3. `configuredReasoningEfforts(prov, model.id)` — + `src/reasoning-effort.ts:148-153`. It returns `[]` for a + `noReasoningModels` member, the per-model map when present, the + provider-wide ladder next, and `undefined` when nothing is declared. + Anthropic hits the last arm. +4. `src/codex/catalog/provider-fetch.ts:749-772` spreads + `reasoningEfforts` onto the `CatalogModel` only when it is not + `undefined`, so the catalog row carries no ladder. +5. `src/server/management/model-rows.ts:150-165` builds the + `ManagementModelRow` from that catalog row, and `toExportModel` + (`model-rows.ts:170-182`) copies `reasoningEfforts` only when present. +6. `normalizeExportModels` (`src/clients/config-export.ts:934-942`) preserves + the object as-is. +7. `buildPiClientConfig` (`config-export.ts:1229-1258`) emits `reasoning: true` + plus `thinkingLevelMap` ONLY when + `Array.isArray(model.reasoningEfforts) && model.reasoningEfforts.length > 0`. + Aside reuses that builder through `buildAsideContribution` + (`config-export.ts:1778-1780`), so the Claude rows are written with no + effort control. + +Every step is behaving as designed. The only missing fact is the ladder itself, +which was never declared for the native Anthropic providers. + +`candidateCapabilityEvidence` still matters, but as BLAST RADIUS rather than as +the defect path — see 010. + +## The adapter already supports effort + +This is the fact that makes the fix a one-place change rather than a feature. +`src/adapters/anthropic.ts` has honored effort for a long time (line ~922): + +- Adaptive families send `thinking: {type:"adaptive"}` plus + `output_config: {effort}`. `adaptiveEffort()` (line 553) maps `minimal` to + `low` because the wire rejects `minimal` with a 400, and accepts + `low|medium|high|xhigh|max`. +- Older families send `thinking: {type:"enabled", budget_tokens}` sized by + `reasoningBudget()` (line 463), which has a distinct budget for every rung: + minimal 1024, low 4096, medium 8192, high 16384, xhigh 24576, max 32000. + +So the proxy has always been willing to send effort for these models; it simply +never told any client the knob existed. A user could reach it by hand-editing +`thinkingLevelMap`, which is exactly the workaround shape that indicates a +missing advertisement rather than a missing capability. + +## Which ladder is correct per family + +`ADAPTIVE_THINKING_FAMILY_MINIMUMS` (line 483) splits the wire shapes, and its +comment records vendor verification against api.anthropic.com: sonnet>=5, +fable (any), opus>=4.7 require adaptive; haiku-4-5 and sonnet-4-5 reject it; +opus-4-6 and sonnet-4-6 accept both. + +`ANTHROPIC_MODELS` (registry line 350) is: claude-fable-5-1, claude-fable-5, +claude-sonnet-5, claude-opus-5, claude-opus-4-8, claude-opus-4-7, +claude-opus-4-6, claude-sonnet-4-6, claude-haiku-4-5. + +Both wire shapes cover the same five rungs, so the honest ladder is the same for +every model in the list: `low, medium, high, xhigh, max`. + +- `minimal` is deliberately EXCLUDED. Adaptive models 400 on it, and the + adapter only survives it by silently rewriting it to `low`. Advertising a rung + that collapses into another rung invites a user to pick a setting that does + nothing. +- `none` is deliberately EXCLUDED. `supportsExplicitThinkingDisable` is seeded + with sonnet>=5 ONLY, and its comment warns that a wrong entry turns a silent + truncation into a 400. Fable in particular always thinks and rejects an + explicit disable. A ladder-wide `none` would advertise an off switch that does + not exist for most of the list. +- `ultra` is not an Anthropic concept; `reasoningBudget` has no case for it and + it would fall through to the medium default. + +## Blast radius of adding the ladder + +`modelReasoningEfforts` is read by more than the catalog, so the change is +checked against each consumer. Two of these turned into required correctness +work rather than mere acknowledgement (010 phases 2 and 3). + +- `src/clients/config-export.ts` — every exporter with an effort concept (pi, + aside, prime, omp, dsh, hermes, openclaw, kimi, zcode) starts emitting the + control. This is the user-visible fix. +- `src/providers/derive.ts:493` — enrichment copies the registry map only when + the persisted provider has NO map + (`if (!prov.modelReasoningEfforts && seed.modelReasoningEfforts)`). One + customized Anthropic model would therefore suppress the registry ladder for + all eight others. The neighbouring `modelInputModalities` line already uses + the per-key `fillRecordOfArrays` for exactly this reason, and its comment + documents the same class of bug. Requests are unaffected because + `routedProviderConfig` merges per key (`src/router.ts:181-189`), so the + symptom would be split-brain: correct on the wire, missing in the catalog. +- `src/routing/capability.ts:216-239` — `candidateCapabilityEvidence` reads the + registry map directly and, unlike `configuredReasoningEfforts` + (`reasoning-effort.ts:148`) and `supportedLadderFor` + (`effort-policy.ts:119-127`), never consults `noReasoningModels`. Today that + is harmless for Anthropic because there is no ladder to report; once one + exists, a model the user explicitly disabled reasoning for would still present + five supported rungs to `src/routing/evaluator.ts:141,218`. +- `src/server/effort-policy.ts:122` — supplies the ladder to the effort CAP. + Note this is not a general per-request clamp: `effortCapAppliesTo` gates it + (`src/server/responses/core.ts:2159-2163`), so it only engages when the user + configured `effortCap`/`subagentEffortCap`. +- `src/routing/compatibility/behavior.ts:194-200` — `reasoning.supported` flips + false to true and `reasoning.efforts` gains five rungs, which changes the + Compatibility Lab behavior fingerprint for Anthropic. That invalidation is + intended: the previous fingerprint recorded a capability the proxy really has. + +### What does NOT change + +`ultra` handling. `src/responses/parser.ts:814-820` already degrades `ultra` to +`max` at parse time, and `mapReasoningEffort` applies the same boundary +(`src/reasoning-effort.ts:209-225`). An earlier draft claimed the new ladder +would newly clamp `ultra`; that assertion is already green today and cannot +demonstrate activation. + +The Codex catalog surface also already synthesizes `max`/`ultra` rungs for any +reasoning-capable routed row (`src/codex/catalog/effort.ts:225-237`), so this +change does not introduce `ultra` there either. + +## Related defects found in the same evidence chain + +Two more rows in the live catalog advertise no ladder. Recorded here and +investigated in 030 rather than silently bundled into this fix: + +- `lidge/qwen3.8-27b-nvfp4` — the provider block has no `models` key at all and + no `modelReasoningEfforts`. +- `opencode-free/muse-spark-1.2-contributor-free` — the provider declares + `modelReasoningEfforts` for `deepseek-v4-flash-free` only, so the muse row + falls through. + +Neither is the reported bug, and each needs its own capability evidence before a +ladder can be asserted honestly. diff --git a/devlog/_plan/260904_anthropic_effort_ladder/010_registry_ladder.md b/devlog/_plan/260904_anthropic_effort_ladder/010_registry_ladder.md new file mode 100644 index 0000000000..ddae82dc5e --- /dev/null +++ b/devlog/_plan/260904_anthropic_effort_ladder/010_registry_ladder.md @@ -0,0 +1,254 @@ +# 010 — Declare the reasoning-effort ladder on the native Anthropic providers + +Work phase: wp2. Consumes 000. + +Scope was one production file in the first draft. The audit disproved that: +declaring the ladder is necessary but not sufficient, because enrichment can +drop it and routing evidence can contradict it. Three production files. + +## Phase 1 — `src/providers/registry.ts` (the advertisement) + +Add one shared constant beside the existing Anthropic constants (after +`ANTHROPIC_MODEL_CONTEXT_WINDOWS`, line ~351): + +```ts +/** + * Every model in ANTHROPIC_MODELS accepts the same five rungs, because both wire + * shapes the adapter emits cover the same range: adaptive families take + * output_config.effort (low|medium|high|xhigh|max) and older families take a + * thinking budget, which reasoningBudget() sizes distinctly for each of those + * five. Deliberately excluded: minimal (adaptive 400s on it and the adapter + * rewrites it to low, so it is not a distinct setting), none (only sonnet>=5 + * accepts an explicit thinking disable; Fable rejects one outright), and ultra + * (not an Anthropic concept; reasoningBudget has no case for it). + */ +const ANTHROPIC_REASONING_EFFORTS = ["low", "medium", "high", "xhigh", "max"]; +const ANTHROPIC_MODEL_REASONING_EFFORTS: Record = Object.fromEntries( + ANTHROPIC_MODELS.map(id => [id, [...ANTHROPIC_REASONING_EFFORTS]]), +); +``` + +Then add `modelReasoningEfforts` to BOTH provider entries, next to the existing +`modelContextWindows` line so the metadata stays visually grouped: + +- `anthropic` (line ~1341): `modelReasoningEfforts: { ...ANTHROPIC_MODEL_REASONING_EFFORTS },` +- `anthropic-apikey` (line ~1356): the same spread. + +Both entries get it. They share `ANTHROPIC_MODELS` and the same adapter, so a +ladder on only one would make the effort control depend on whether the user +signed in with OAuth or an API key — the exact class of inconsistency this unit +is fixing. `tests/provider-registry-parity.test.ts:450-451` already asserts the +two entries agree on `models` and `modelContextWindows`; extend it to the ladder. + +### What the ladder means + +It is an OPENCODEX ladder, not a claim that every model takes +`output_config.effort`. Fable 5/5.1, Sonnet 5, Opus 5 and Opus 4.7/4.8 send the +five values directly; Opus 4.6, Sonnet 4.6 and Haiku 4.5 take the legacy budget +path where the adapter TRANSLATES each rung into `thinking.budget_tokens`. Per +Anthropic's effort documentation the 4.6 models expose `low|medium|high|max` +natively and Haiku 4.5 has no effort parameter at all — the adapter's budget +translation is what makes five rungs meaningful there. That is why the ladder is +uniform: the proxy, not the vendor, defines it, and the adapter honors all five +for every family without a 400 (budgets are clamped below `max_tokens` at +`src/adapters/anthropic.ts:947-956`). + +## Phase 2 — `src/providers/derive.ts` (make enrichment per-model) + +Line 493 today: + +```ts +if (!prov.modelReasoningEfforts && seed.modelReasoningEfforts) prov.modelReasoningEfforts = cloneRecordOfArrays(seed.modelReasoningEfforts); +``` + +becomes the per-key fill already used one line above it for modalities: + +```ts +if (seed.modelReasoningEfforts) { + prov.modelReasoningEfforts = fillRecordOfArrays(seed.modelReasoningEfforts, prov.modelReasoningEfforts); +} +``` + +`fillRecordOfArrays` (line 111) spreads the seed first and the user's map second, +so per-model user entries stay authoritative while untouched models inherit the +registry. Without this, any user who customized ONE Anthropic model keeps the bug +for the other eight, and in a particularly confusing shape: routing merges these +maps per key already (`src/router.ts:181-189`), so the wire would honor the +effort while `/v1/models` and Aside still showed no control. + +Precisely: authoritative for the SAME EXACT KEY. A differently-cased user key can +coexist with the canonical registry key, and `modelRecordValue` +(`src/reasoning-effort.ts:115-126`) resolves an exact match before its +case-folded fallback, so the canonical entry wins the lookup. The direct-contract +pass (`derive.ts:130-147,158-205,562`) then removes folded duplicates and +restores the explicit spelling, which is why +`tests/alibaba-intl-token-plan.test.ts:102-121` stays green. + +This is a general fix, not an Anthropic one — it repairs the same latent bug for +every provider with a registry ladder. Its comment at line 100-107 documents the +identical reasoning for `modelInputModalities`; that precedent is why this rides +in the same PR rather than becoming a separate unit. + +## Phase 3 — `src/routing/capability.ts` (do not contradict `noReasoningModels`) + +Two edits in one file: the guard, and the line-239 spread that would otherwise +discard its result. + +`candidateCapabilityEvidence` (line 216) reads the registry map directly and +never consults `noReasoningModels`, unlike `configuredReasoningEfforts` +(`src/reasoning-effort.ts:148`), `supportedLadderFor` +(`src/server/effort-policy.ts:119-127`) and the compatibility fingerprint +(`src/routing/compatibility/behavior.ts:194-196`). Add the same guard first: + +```ts +const reasoningEfforts = modelInList(provider?.noReasoningModels, modelId) + ? [] + : modelRecordValue(provider?.modelReasoningEfforts, modelId) + ?? modelRecordValue(registryEntry?.modelReasoningEfforts, modelId) + ?? (isNative ? nativeReasoningEfforts(modelId) : undefined); +``` + +The `[]` must SURVIVE into the returned evidence, which the current line 239 +prevents: + +```ts +...(reasoningEfforts !== undefined && reasoningEfforts.length > 0 ? { reasoningEfforts } : {}), +``` + +An empty ladder is dropped, the property is absent, and +`src/routing/evaluator.ts:141-150` and `:218-227` take their +`Array.isArray(ladder)` false branch and record `unknown`. Unknown is +permissive — it is "we could not tell", not "this model has no effort control". +So returning `[]` alone would leave the original defect intact behind a +different code path. + +Change line 239 to preserve a DEFINED ladder, empty or not: + +```ts +...(reasoningEfforts !== undefined ? { reasoningEfforts } : {}), +``` + +Blast radius of that widening, since it affects every provider and not just +Anthropic: a defined-but-empty ladder today comes from an explicit per-model +`[]` in config or registry, which `src/types/provider.ts:465-467` documents as +"intentionally expose no effort control". Every other consumer already reads +`[]` as a known negative — `configuredReasoningEfforts` +(`reasoning-effort.ts:148`) returns `[]` for `noReasoningModels`, +`supportedLadderFor` (`effort-policy.ts:119-127`) does the same, and +`behavior.ts:194-196` reports `reasoning.supported: false`. Routing evidence is +the one surface that silently downgraded that to unknown. Making it agree is the +intent, and the evaluator change is the observable effect: a candidate with an +explicit empty ladder now returns `unsatisfied` for a reasoning-effort +requirement instead of `unknown`. + +Today this cannot misfire for Anthropic because there is no ladder to report. +Phase 1 is exactly what makes it reachable, which is why it belongs in this PR. + +The spread (rather than sharing one object reference) matches how +`modelContextWindows` is already written on these two entries and keeps a +later mutation of one provider from reaching the other. + +## Why no `modelDefaultReasoningEfforts` + +Considered and rejected. Setting a default would change what the proxy SENDS for +callers who specify nothing, which is a behavior change for existing users beyond +the reported bug. The reported bug is that the control is absent, not that its +default is wrong. Anthropic's own defaults stay in force: adaptive models decide +for themselves, and `defaultReasoningEffort()` returns undefined so the adapter +omits the field exactly as it does today. + +Consequence to keep in mind while reading the export: `buildPiClientConfig` +emits no per-model default either (config-export.ts line ~823 documents that the +proxy owns the default), so the client shows a ladder with no preselected rung. +That is the same shape every other routed provider already has. + +## Scope boundary + +IN: the two registry entries plus constants, the `derive.ts` per-model fill, and +the `capability.ts` `noReasoningModels` guard. + +OUT: `src/adapters/anthropic.ts` (already correct — see 000), the exporters in +`src/clients/config-export.ts` (they key off the ladder and need no edit), +`effort-policy.ts` (already correct), and the unrelated empty-ladder providers +in 030 — that investigation ships in its own change, never in this PR. + +## Accept criteria + +1. `GET /v1/models` returns `supports_reasoning_effort: true` and + `reasoning_efforts` of low..max for every `anthropic/claude-*` row, and the + same for `anthropic-apikey` when configured. +2. The Aside export document emits `reasoning: true` and a `thinkingLevelMap` + for those rows, with `off` and `minimal` mapped to `null` (the ladder + declares neither) and `max` mapped to `max`. +3. A provider carrying a PARTIAL persisted `modelReasoningEfforts` still + receives the registry ladder for its untouched models, and the customized + model keeps the user's value. +4. A model listed in `noReasoningModels` reports no effort evidence to routing + even though the registry now declares a ladder. +5. `bun run typecheck` passes. + +The earlier `ultra` criterion is deleted, not weakened: `parser.ts:814-820` +already degrades `ultra` to `max`, so that assertion passes before the change +and proves nothing. + +### Activation scenarios (C-ACTIVATION-GROUNDING-01) + +Both new conditional paths must be shown to fire: + +- Phase 2's per-key fill activates when the persisted provider has a non-empty + `modelReasoningEfforts` missing some registry keys. C constructs exactly that + config; the observable effect is the untouched model's ladder in the enriched + provider. Under the old line the map is returned unchanged, so this assertion + is red before the change. +- Phase 3's guard activates when `noReasoningModels` names a model the registry + gives a ladder. The observable effect is `reasoningEfforts: []` in the + capability evidence AND an `unsatisfied` (not `unknown`) evaluator outcome for + a reasoning-effort requirement. Asserting mere ABSENCE would be a green + no-op — absence is exactly the buggy state — so the assertion is on the + present-and-empty value and the downstream outcome. This is red after phase 1 + alone, which is the point: phase 1 is what makes it reachable. + +## Test plan + +Rewritten after audit. The first draft built an `ExportModel` that already +carried the ladder, which tests the serializer and skips every hop where the bug +actually lives. Tests attach to the subsystem they cover: + +1. `tests/provider-registry-parity.test.ts` — extend the existing anthropic + parity block (line ~440-451): both entries declare a ladder for every id in + `ANTHROPIC_MODELS`, the two agree, and the ladder excludes `minimal`, + `none` and `ultra`. Asserting the exclusions is the point: a future widening + to `minimal` would be collapsed to `low` by `adaptiveEffort`. +2. `tests/aside-client.test.ts` — the anthropic fixture at line 38 currently + carries no ladder and asserts nothing about it. Give it the ladder and assert + `buildClientConfig("aside", ctx)` emits `reasoning: true` with `off` and + `minimal` null. Use the PUBLIC `buildClientConfig` entry point + (`config-export.ts:1963`) — `buildPiClientConfig` and + `buildAsideContribution` are private. Keep a no-ladder row in the same + document asserting neither field appears, so the test fails if someone makes + `reasoning: true` unconditional. +3. Enrichment: a focused test over `enrichProviderFromRegistry` with a partial + persisted `modelReasoningEfforts`, asserting registry fill for untouched + models and user precedence for the customized one. +4. `tests/routing-capability-model-matching.test.ts` — a `noReasoningModels` + case asserting `evidence.reasoningEfforts` EQUALS `[]` (not merely absent), + fed through the evaluator to assert `outcome: "unsatisfied"`, plus a control + case that still reports the five-rung ladder and `satisfied`. +5. `tests/management-client-config-route.test.ts` — the INTEGRATION regression, + and the only test here that covers the seam the bug actually lives in. Items + 1-4 and the aside-client serializer contract all start from hand-built + fixtures, so every one of them would stay green if enrichment, + `CatalogModel`, `ManagementModelRow` or `toExportModel` dropped the field + tomorrow. This case starts from a canonical minimal Anthropic PROVIDER + CONFIG and asserts the resulting Aside document, exercising + `registry -> enrichProviderFromRegistry -> CatalogModel -> ManagementModelRow + -> toExportModel -> buildClientConfig("aside")` end to end. That route file + already drives model loading through the public client-config boundary, so + the fixture cost is small. It is red before phase 1 and green after. + +Run each touched file with `bun test tests/.test.ts`, plus +`bun test tests/anthropic-reasoning.test.ts` to show the wire path is +unaffected, plus `bun run typecheck`. AGENTS.md also recommends +`bun run test:changed` for a change with this many consumers; it selects by +import graph and is NOT the repository-wide suite the user forbade, so it is +included. A bare `bun test` or `bun run test` is not run under any circumstance. diff --git a/devlog/_plan/260904_anthropic_effort_ladder/020_verification_and_pr.md b/devlog/_plan/260904_anthropic_effort_ladder/020_verification_and_pr.md new file mode 100644 index 0000000000..da204c4641 --- /dev/null +++ b/devlog/_plan/260904_anthropic_effort_ladder/020_verification_and_pr.md @@ -0,0 +1,71 @@ +# 020 — Live verification and pull request + +Work phase: wp3. Consumes 010. + +## Why a live check is required + +The unit tests prove the registry declares a ladder and the builder emits the +control. Neither proves the ladder survives the path a real user exercises: +config load, catalog assembly, the `/v1/models` serializer, then the export +writer into the file Aside parses. The reported bug lived precisely in that +seam — every individual component was correct. + +## Steps + +1. Rebuild/restart a SCRATCH proxy. The live proxy on port `10100` is the + user's working instance and must not be disturbed for an experiment: use a + separate `OPENCODEX_HOME` and a scratch port. +2. `curl -s http://127.0.0.1:/v1/models` and confirm the + `anthropic/claude-*` rows now carry `supports_reasoning_effort: true` and + the five-rung ladder. Capture the before/after rows as evidence. +3. Render the Aside export document from the same catalog and confirm + `reasoning: true` plus `thinkingLevelMap` on those rows. Do NOT overwrite + the user's real `~/.aside/u/0/models.json` as part of the fix; render to a + scratch path. Rewriting it is the user's own re-export action. +4. Optional UI confirmation via the Aside surface, if the scratch catalog can be + pointed at without touching the signed-in profile's live config. Skipped + rather than forced: mutating the user's real Aside config to take a + screenshot would change account state for evidence, which is not a trade + worth making when the file-level proof is exact. + +## Commit and push + +The worktree is DETACHED at `072df52e` and the local `dev` ref is behind it, so +"branch off dev" is ambiguous here and could produce a stale base. Bind the base +to a freshly fetched SHA instead: + +1. `git fetch origin dev` and record `FETCH_HEAD`. +2. Confirm the current detached HEAD against it; branch from the fetched SHA. +3. `git switch -c codex/260904-anthropic-effort-ladder ` — created IN this + worktree (WORKTREE-GUARD-01: adopt in place, never relocate or recreate). +4. Preserve the untracked plan directory across the switch. + +- Commit the three production files, the tests, and this devlog unit. +- 030 does NOT ship in this PR (see below). +- Push with `--no-verify` — explicitly authorized by the user for this task. +- The repository-wide suite is explicitly forbidden by the user, so the PR + description states exactly which focused checks were run rather than implying + the full gate passed. Claiming a green full suite that was never run is worse + than reporting a narrower proof. + +## One logical change + +`030_related_empty_ladders.md` is an investigation of unrelated providers. It +stays in the plan unit as a record but its FINDINGS ship separately: a reviewer +judging an Anthropic ladder should not also have to adjudicate lidge and +opencode-free capabilities. If 030 produces a code change, it gets its own PR. + +## Pull request + +Target `dev` (never `main`). Fill all three template sections from +`.github/PULL_REQUEST_TEMPLATE.md`: Summary, Verification, Checklist. No +screenshot is required because the change touches no GUI surface; the title and +description must therefore avoid the word `gui`, which would trip the +screenshot gate in `enforce-target`. + +## Accept criteria + +- Live catalog shows the ladder for `anthropic/claude-*`. +- Export document shows `reasoning: true` and `thinkingLevelMap`. +- PR is open against `dev` with every template section filled, based on the + exact fetched `origin/dev` SHA. diff --git a/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md b/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md new file mode 100644 index 0000000000..0935e7973f --- /dev/null +++ b/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md @@ -0,0 +1,50 @@ +# 030 — Related: other rows advertising no effort ladder + +Work phase: wp4. Consumes 020. Investigation first; a fix only where evidence +supports one. + +## Candidates from the live catalog + +`GET /v1/models` on 2026-09-04 returned an empty ladder for three rows besides +the Anthropic ones. Two are genuine candidates: + +| Row | Provider block state | +|---|---| +| `lidge/qwen3.8-27b-nvfp4` | no `models` key, no `modelReasoningEfforts` | +| `opencode-free/muse-spark-1.2-contributor-free` | `modelReasoningEfforts` declares `deepseek-v4-flash-free` only | + +## The question to answer for each + +Not "does it have a ladder" — the catalog already answers that. The question is +whether the ADAPTER would honor an effort if one were declared, which is what +made the Anthropic case a safe fix. An empty ladder on a model whose adapter +ignores or rejects effort is CORRECT, and adding one there would advertise a +control that silently does nothing. + +So for each candidate: + +1. Which adapter serves it, and does that adapter have an effort path? +2. Is the model reasoning-capable at all, per its own vendor surface? +3. Is it excluded on purpose (a `noReasoningModels` entry, a deliberate empty + `reasoningEfforts: []`)? Several providers in the registry declare + `reasoningEfforts: []` explicitly, which is a positive statement of "no + reasoning", not an oversight. + +`lidge` is a local/self-hosted block with `allowPrivateNetwork` and an API-key +pool, so its capabilities depend on the deployed server rather than a vendor +contract — a ladder claim there needs a live probe, not a guess. + +## Disposition rule + +- Adapter honors effort AND the model reasons -> same fix shape as 010, but as + its own change with its own evidence. It does NOT ride along in the Anthropic + PR; a reviewer evaluating a Claude fix should not have to also adjudicate an + unrelated provider's capabilities. +- Adapter ignores effort, or capability is unproven -> report to the user with + the evidence and leave the catalog honest. An unproven ladder is a worse defect + than a missing one, because the control appears to work. + +## Deliverable + +A written finding per candidate naming the adapter, the capability evidence, and +the verdict. Reported to the user regardless of whether any code changes. diff --git a/src/providers/derive.ts b/src/providers/derive.ts index 2a224476a3..5852e9c89e 100644 --- a/src/providers/derive.ts +++ b/src/providers/derive.ts @@ -490,7 +490,14 @@ export function enrichProviderFromRegistry(name: string, prov: OcxProviderConfig if ((!prov.reasoningEfforts || hasLegacyClinePassReasoningEfforts(name, prov)) && seed.reasoningEfforts) { prov.reasoningEfforts = [...seed.reasoningEfforts]; } - if (!prov.modelReasoningEfforts && seed.modelReasoningEfforts) prov.modelReasoningEfforts = cloneRecordOfArrays(seed.modelReasoningEfforts); + // Per-model fill for the same reason as modelInputModalities above: an all-or-nothing + // copy let ONE customized model hide the registry's ladder for every other model on the + // provider. That split the two planes apart — routing merges these maps per key + // (mergeRecordFill in src/router.ts), so the wire honored the effort while /v1/models and + // every client export showed no effort control at all. + if (seed.modelReasoningEfforts) { + prov.modelReasoningEfforts = fillRecordOfArrays(seed.modelReasoningEfforts, prov.modelReasoningEfforts); + } if (!prov.modelDefaultReasoningEfforts && seed.modelDefaultReasoningEfforts) prov.modelDefaultReasoningEfforts = { ...seed.modelDefaultReasoningEfforts }; if (!prov.reasoningEffortMap && seed.reasoningEffortMap) prov.reasoningEffortMap = { ...seed.reasoningEffortMap }; if (!prov.modelReasoningEffortMap && seed.modelReasoningEffortMap) prov.modelReasoningEffortMap = cloneNestedRecord(seed.modelReasoningEffortMap); diff --git a/src/providers/registry.ts b/src/providers/registry.ts index fd00ba8277..ac2638ae22 100644 --- a/src/providers/registry.ts +++ b/src/providers/registry.ts @@ -349,6 +349,34 @@ export type ProviderConfigSeed = Pick< // always on, per the official models overview and pricing page (platform.claude.com). const ANTHROPIC_MODELS = ["claude-fable-5-1", "claude-fable-5", "claude-sonnet-5", "claude-opus-5", "claude-opus-4-8", "claude-opus-4-7", "claude-opus-4-6", "claude-sonnet-4-6", "claude-haiku-4-5"]; const ANTHROPIC_MODEL_CONTEXT_WINDOWS: Record = { "claude-fable-5-1": 1_000_000, "claude-sonnet-5": 1_000_000, "claude-fable-5": 1_000_000, "claude-opus-5": 1_000_000, "claude-opus-4-8": 1_000_000, "claude-opus-4-7": 1_000_000, "claude-opus-4-6": 1_000_000, "claude-sonnet-4-6": 1_000_000, "claude-haiku-4-5": 200_000 }; +/** + * The effort rungs opencodex exposes for native Anthropic models. Without this the + * providers advertised no ladder at all, so every client that keys its effort control off + * `reasoningEfforts` — Aside and the rest of the Pi-shaped exports — wrote these models + * with no control, while the SAME Claude models routed through `cursor` or + * `google-antigravity` had one. + * + * This is an opencodex ladder, not a claim that each model takes `output_config.effort`. + * The adapter serves two wire shapes (src/adapters/anthropic.ts): adaptive families + * (fable, sonnet >= 5, opus >= 4.7) send the effort directly, while opus 4.6, sonnet 4.6 + * and haiku 4.5 take the legacy path where `reasoningBudget` TRANSLATES each rung into + * `thinking.budget_tokens`. Anthropic documents `low|medium|high|max` for the 4.6 models + * and no effort parameter at all for haiku 4.5; the budget translation is what makes five + * rungs meaningful there, and it clamps below `max_tokens` so none of them 400. + * + * Deliberately excluded, each because advertising it would offer a control that does not + * do what it says: + * - `minimal`: `adaptiveEffort` rewrites it to `low` (the adaptive wire 400s on it), so + * it is not a distinct setting. + * - `none`: only sonnet >= 5 accepts an explicit thinking disable + * (`EXPLICIT_THINKING_DISABLE_FAMILY_MINIMUMS`); Fable rejects one outright. + * - `ultra`: not an Anthropic concept, and it is degraded to `max` at the request + * boundary anyway (src/responses/parser.ts). + */ +const ANTHROPIC_REASONING_EFFORTS = ["low", "medium", "high", "xhigh", "max"]; +const ANTHROPIC_MODEL_REASONING_EFFORTS: Record = Object.fromEntries( + ANTHROPIC_MODELS.map(id => [id, [...ANTHROPIC_REASONING_EFFORTS]]), +); // 260814 GLM-5.3 is registered pre-emptively alongside 5.2 everywhere 5.2 appears. Z.AI's // devpack "How to Switch Models" page (docs.z.ai/devpack/latest-model) lists glm-5.3 and @@ -1340,6 +1368,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ note: "Log in with your Claude account", models: [...ANTHROPIC_MODELS], modelContextWindows: { ...ANTHROPIC_MODEL_CONTEXT_WINDOWS }, + modelReasoningEfforts: { ...ANTHROPIC_MODEL_REASONING_EFFORTS }, defaultModel: "claude-sonnet-5", }, { @@ -1356,6 +1385,7 @@ export const PROVIDER_REGISTRY: readonly ProviderRegistryEntry[] = [ models: [...ANTHROPIC_MODELS], liveModels: true, modelContextWindows: { ...ANTHROPIC_MODEL_CONTEXT_WINDOWS }, + modelReasoningEfforts: { ...ANTHROPIC_MODEL_REASONING_EFFORTS }, defaultModel: "claude-sonnet-5", }, { diff --git a/src/routing/capability.ts b/src/routing/capability.ts index 0681f64860..8495951a0f 100644 --- a/src/routing/capability.ts +++ b/src/routing/capability.ts @@ -213,9 +213,18 @@ export function candidateCapabilityEvidence( || provider?.parallelToolCalls === true || undefined; - const reasoningEfforts = modelRecordValue(provider?.modelReasoningEfforts, modelId) - ?? modelRecordValue(registryEntry?.modelReasoningEfforts, modelId) - ?? (isNative ? nativeReasoningEfforts(modelId) : undefined); + // `noReasoningModels` is a POSITIVE statement that this model has no effort control, and + // every other consumer already reads it that way: configuredReasoningEfforts + // (reasoning-effort.ts), supportedLadderFor (server/effort-policy.ts) and the + // compatibility fingerprint (routing/compatibility/behavior.ts) all check it first. + // Routing evidence did not, which was harmless only while no registry ladder existed to + // contradict it — a provider-level ladder would otherwise report supported rungs for a + // model the operator explicitly disabled reasoning for. + const reasoningEfforts = modelInList(provider?.noReasoningModels, modelId) + ? [] + : modelRecordValue(provider?.modelReasoningEfforts, modelId) + ?? modelRecordValue(registryEntry?.modelReasoningEfforts, modelId) + ?? (isNative ? nativeReasoningEfforts(modelId) : undefined); const tierSupport = provider ? serviceTierSupportForModel(provider, modelId, providerName) @@ -236,7 +245,11 @@ export function candidateCapabilityEvidence( ...(typeof contextWindow === "number" ? { contextWindow } : {}), ...(typeof image === "boolean" ? { image } : {}), ...(typeof tools === "boolean" ? { tools } : {}), - ...(reasoningEfforts !== undefined && reasoningEfforts.length > 0 ? { reasoningEfforts } : {}), + // A DEFINED but empty ladder is known-negative evidence and must survive. Dropping it + // made the evaluator take its `!Array.isArray` branch and record "unknown", which is + // permissive — "we could not tell" rather than "this model has no effort control" — so + // an explicitly disabled model could still satisfy a reasoning-effort requirement. + ...(reasoningEfforts !== undefined ? { reasoningEfforts } : {}), ...(serviceTier !== "unknown" ? { serviceTier } : {}), ...localRemote, ...(typeof encryptedCodexTasks === "boolean" ? { encryptedCodexTasks } : {}), diff --git a/tests/aside-client.test.ts b/tests/aside-client.test.ts index 991a7f8af6..6cb30bea50 100644 --- a/tests/aside-client.test.ts +++ b/tests/aside-client.test.ts @@ -98,6 +98,48 @@ describe("Aside client config", () => { expect(unknown.maxTokens).toBeUndefined(); }); + /** + * The defect this client existed to expose: native Anthropic rows reached Aside with no + * effort control at all, while the SAME Claude models routed through cursor or + * google-antigravity had one. The serializer was never wrong — it emits the control only + * for a non-empty ladder, and the providers advertised none. + * + * This is the SERIALIZER half of the contract. It starts from a hand-built ExportModel, so + * it would stay green if enrichment or the catalog dropped the ladder upstream; the + * end-to-end guard for that seam lives in tests/management-client-config-route.test.ts. + */ + test("an Anthropic row with a ladder gets an effort control, one without stays bare", () => { + const withLadder = buildClientConfig("aside", { + baseUrl: "http://127.0.0.1:10100/v1", + config: CONFIG, + models: [ + { + namespaced: "anthropic/claude-opus-5", + provider: "anthropic", + id: "claude-opus-5", + contextWindow: 1_000_000, + reasoningEfforts: ["low", "medium", "high", "xhigh", "max"], + }, + ], + }) as PiGeneratedConfig; + + const claude = withLadder.providers[OPENCODE_PROVIDER_ID]!.models[0]!; + expect(claude.reasoning).toBe(true); + expect(claude.thinkingLevelMap!.low).toBe("low"); + expect(claude.thinkingLevelMap!.max).toBe("max"); + // The ladder declares neither, so neither is offered as a selectable level. + expect(claude.thinkingLevelMap!.off).toBeNull(); + expect(claude.thinkingLevelMap!.minimal).toBeNull(); + + // Negative control: the fixture's ladderless Anthropic row still gets no control, so this + // fails if anyone makes `reasoning: true` unconditional. + const bare = buildClientConfig("aside", context()) as PiGeneratedConfig; + const bareClaude = bare.providers[OPENCODE_PROVIDER_ID]!.models + .find(model => model.id === "anthropic/claude-opus-5")!; + expect(bareClaude.reasoning).toBeUndefined(); + expect(bareClaude.thinkingLevelMap).toBeUndefined(); + }); + test("native JSON round-trips and never carries a credential", () => { const sentinel = ["sk", "live", "aside", "sentinel"].join("-"); const withKey = { ...CONFIG, apiKeys: [{ key: sentinel }] } as OcxConfig; diff --git a/tests/management-client-config-route.test.ts b/tests/management-client-config-route.test.ts index 9f004b0903..2d65323b4c 100644 --- a/tests/management-client-config-route.test.ts +++ b/tests/management-client-config-route.test.ts @@ -8,6 +8,7 @@ import { seedCodexModelEntitlementsForTests, } from "../src/codex/model-entitlements"; import { handleManagementAPI } from "../src/server/management-api"; +import { loadExportModels } from "../src/server/management/model-rows"; import { OPENCODE_API_KEY_ENV, OPENCODE_CONFIG_SCHEMA, @@ -157,6 +158,60 @@ function toExportModel(row: ModelRow): ExportModel { }; } + +describe("native Anthropic effort ladder reaches the Aside document", () => { + /** + * The end-to-end guard for the defect: native Anthropic rows used to reach Aside with no + * effort control, while the SAME Claude models routed through cursor or google-antigravity + * had one. Every other test for this fix starts from a hand-built ExportModel or registry + * lookup, so all of them would stay green if enrichment, CatalogModel, ManagementModelRow + * or toExportModel dropped the field tomorrow. This one starts from a bare PROVIDER CONFIG + * and asserts the emitted document, so it covers the whole chain: + * + * registry -> enrichProviderFromRegistry -> CatalogModel -> ManagementModelRow + * -> toExportModel -> buildClientConfig("aside") + * + * It calls the production loader directly rather than the ?client=aside route, because that + * route resolves ~/.aside/accounts.json for its destination and this must not depend on the + * developer's real Aside install. + */ + test("a bare Anthropic provider config emits reasoning and a thinkingLevelMap", async () => { + const config = { + port: 10100, + hostname: "127.0.0.1", + defaultProvider: "anthropic", + providers: { + anthropic: { + adapter: "anthropic", + baseUrl: "https://api.anthropic.com", + authMode: "oauth", + liveModels: false, + }, + }, + } as unknown as OcxConfig; + + const models = await loadExportModels(config); + const document = buildClientConfig("aside", { + baseUrl: "http://127.0.0.1:10100/v1", + config, + models, + }) as PiGeneratedConfig; + + const rows = document.providers[OPENCODE_PROVIDER_ID]!.models + .filter(model => model.id.startsWith("anthropic/claude-")); + expect(rows.length).toBeGreaterThan(0); + + for (const row of rows) { + expect(row.reasoning).toBe(true); + expect(row.thinkingLevelMap!.low).toBe("low"); + expect(row.thinkingLevelMap!.high).toBe("high"); + expect(row.thinkingLevelMap!.max).toBe("max"); + // The ladder declares neither sentinel, so neither is offered as a selectable level. + expect(row.thinkingLevelMap!.off).toBeNull(); + expect(row.thinkingLevelMap!.minimal).toBeNull(); + } + }); +}); describe("GET /api/client-config", () => { test("opencode envelope carries the shared builder's exact bytes", async () => { const config = baseConfig(); diff --git a/tests/provider-registry-parity.test.ts b/tests/provider-registry-parity.test.ts index b52a57583a..2ed9856cd6 100644 --- a/tests/provider-registry-parity.test.ts +++ b/tests/provider-registry-parity.test.ts @@ -451,6 +451,33 @@ describe("provider registry parity", () => { expect(KEY_LOGIN_PROVIDERS["anthropic-apikey"].modelContextWindows).toEqual(anthropicOauth?.modelContextWindows); }); + test("Anthropic providers advertise an effort ladder for every model on both auth flows", () => { + const anthropicOauth = PROVIDER_REGISTRY.find(entry => entry.id === "anthropic"); + const apiKey = KEY_LOGIN_PROVIDERS["anthropic-apikey"]; + // The ladder is what every client keys its effort control off. Without it Aside and the + // other Pi-shaped exports wrote these models with no control at all, while the same + // Claude models routed through cursor/google-antigravity had one. + for (const modelId of anthropicOauth?.models ?? []) { + expect(anthropicOauth?.modelReasoningEfforts?.[modelId]).toEqual(["low", "medium", "high", "xhigh", "max"]); + } + // Both entries or neither: the effort control must not depend on whether the user signed + // in with OAuth or an API key. + expect(apiKey.modelReasoningEfforts).toEqual(anthropicOauth?.modelReasoningEfforts); + }); + + test("the Anthropic ladder omits rungs the adapter cannot honor distinctly", () => { + const anthropicOauth = PROVIDER_REGISTRY.find(entry => entry.id === "anthropic"); + for (const efforts of Object.values(anthropicOauth?.modelReasoningEfforts ?? {})) { + // minimal is rewritten to low by adaptiveEffort (the adaptive wire 400s on it), none is + // only accepted by sonnet>=5 and rejected outright by Fable, and ultra is degraded to + // max at the request boundary. Advertising any of them offers a control that does not + // do what it says. + expect(efforts).not.toContain("minimal"); + expect(efforts).not.toContain("none"); + expect(efforts).not.toContain("ultra"); + } + }); + test("Kimi coding aliases preserve model context and capability parity", () => { const codingModels = [ "k3", diff --git a/tests/provider-static-model-discovery.test.ts b/tests/provider-static-model-discovery.test.ts index 93ffee8348..01cc31b883 100644 --- a/tests/provider-static-model-discovery.test.ts +++ b/tests/provider-static-model-discovery.test.ts @@ -28,6 +28,37 @@ describe("static provider model discovery policy", () => { } }); + test("a partial persisted effort map does not suppress the registry ladder", () => { + // Per-model fill, not all-or-nothing. An operator who pinned ONE Anthropic model used to + // hide the registry ladder for every other model on the provider, which split the two + // planes apart: routedProviderConfig merges these maps per key, so the WIRE honored the + // effort while /v1/models and every client export showed no effort control at all. + const config = provider({ + adapter: "anthropic", + baseUrl: "https://api.anthropic.com", + authMode: "oauth", + modelReasoningEfforts: { "claude-opus-5": ["low", "high"] }, + }); + + enrichProviderFromRegistry("anthropic", config); + + // The pinned model keeps the operator's value. + expect(config.modelReasoningEfforts?.["claude-opus-5"]).toEqual(["low", "high"]); + // Every untouched model still inherits the registry ladder. + expect(config.modelReasoningEfforts?.["claude-fable-5-1"]).toEqual(["low", "medium", "high", "xhigh", "max"]); + expect(config.modelReasoningEfforts?.["claude-haiku-4-5"]).toEqual(["low", "medium", "high", "xhigh", "max"]); + }); + + test("native Anthropic models reach an enriched provider with an effort ladder", () => { + // The advertisement itself: without it the Aside/Pi exports wrote these models with no + // effort control, while the same Claude models via cursor/google-antigravity had one. + const config = provider({ adapter: "anthropic", baseUrl: "https://api.anthropic.com", authMode: "oauth" }); + enrichProviderFromRegistry("anthropic", config); + for (const modelId of config.models ?? []) { + expect(config.modelReasoningEfforts?.[modelId]).toEqual(["low", "medium", "high", "xhigh", "max"]); + } + }); + test("stale canonical ClinePass discovery is disabled without replacing saved models", () => { const savedModels = ["cline-pass/kimi-k3", "saved-selector"]; const config = provider({ diff --git a/tests/routing-capability-model-matching.test.ts b/tests/routing-capability-model-matching.test.ts index d8839b1f98..cc00b6d912 100644 --- a/tests/routing-capability-model-matching.test.ts +++ b/tests/routing-capability-model-matching.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { candidateCapabilityEvidence } from "../src/routing/capability"; +import { evaluatePolicyProfile } from "../src/routing/evaluator"; import { PROVIDER_REGISTRY } from "../src/providers/registry"; import { modelRecordValue } from "../src/reasoning-effort"; import { isModelTextOnly } from "../src/vision"; @@ -80,6 +81,75 @@ describe("candidateCapabilityEvidence model matching", () => { expect(evidence.reasoningEfforts).toBeUndefined(); }); + test("noReasoningModels reports an EMPTY ladder, not an absent one", () => { + // Every other consumer treats noReasoningModels as a positive "no effort control": + // configuredReasoningEfforts (reasoning-effort.ts), supportedLadderFor + // (server/effort-policy.ts) and the compatibility fingerprint all check it first. + // Evidence must agree, and the difference between [] and absent is load-bearing: + // absent makes the evaluator record "unknown", which is permissive. + const provider = { + ...providerWithFamilyEntries(), + noReasoningModels: ["gpt-oss:120b"], + } as unknown as OcxProviderConfig; + + const evidence = candidateCapabilityEvidence(configFor(provider), "custom", "gpt-oss:120b"); + expect(evidence.reasoningEfforts).toEqual([]); + + // The sibling that is NOT disabled still inherits the family ladder. + const sibling = candidateCapabilityEvidence(configFor(provider), "custom", "gpt-oss:20b"); + expect(sibling.reasoningEfforts).toEqual(["low", "high"]); + }); + + test("a disabled model is capability-unsatisfied for an effort requirement, not unknown", () => { + // The observable consequence of the case above. With an ABSENT ladder the evaluator took + // its non-array branch and emitted `unknown-capability`, which an "allow" profile lets + // through — so a model the operator explicitly disabled reasoning for could still + // satisfy a reasoning-effort requirement. + function configWithProfile(noReasoning: boolean): OcxConfig { + return { + providers: { + custom: { + adapter: "openai-chat", + baseUrl: "https://example.test/v1", + models: ["gpt-oss:120b"], + modelReasoningEfforts: { "gpt-oss": ["low", "high"] }, + ...(noReasoning ? { noReasoningModels: ["gpt-oss:120b"] } : {}), + }, + }, + routingProfiles: { + effort: { + candidates: [{ provider: "custom", model: "gpt-oss:120b" }], + require: { reasoningEffort: "high" }, + unknownEvidence: { capability: "allow", health: "allow", quota: "allow", cost: "allow" }, + }, + }, + } as unknown as OcxConfig; + } + + const disabled = configWithProfile(true); + const result = evaluatePolicyProfile(disabled, "effort", {}, [ + { + provider: "custom", + model: "gpt-oss:120b", + capability: candidateCapabilityEvidence(disabled, "custom", "gpt-oss:120b"), + }, + ]); + const candidate = result.candidates[0]!; + expect(candidate.exclusions.some(e => e.code === "capability-unsatisfied" && e.detail === "reasoning-effort")).toBe(true); + expect(candidate.exclusions.some(e => e.code === "unknown-capability")).toBe(false); + + // Control: the same profile without noReasoningModels is satisfied by the ladder. + const enabled = configWithProfile(false); + const allowed = evaluatePolicyProfile(enabled, "effort", {}, [ + { + provider: "custom", + model: "gpt-oss:120b", + capability: candidateCapabilityEvidence(enabled, "custom", "gpt-oss:120b"), + }, + ]); + expect(allowed.candidates[0]!.exclusions).toEqual([]); + }); + test("a registry entry covers its tagged siblings with no provider configured", () => { // The three registry lookups (capability.ts lines 170/180/206) are a separate branch // from the configured-provider ones above: they are only reached when the provider is From 85eb585672126cbece92d5e1f19c7444c3c88c1c Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 4 Sep 2026 18:46:50 +0900 Subject: [PATCH 2/6] docs(devlog): record the related empty-ladder findings for lidge and opencode-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both candidates surfaced by the Anthropic investigation resolve to 'no code change', for different reasons worth writing down. lidge/qwen3.8-27b-nvfp4 is not a registry provider at all — the only 'lidge' matches in registry.ts are maintainer-attribution comments. It is an operator custom provider on a private network, currently disabled, whose customModels row simply has no ladder set. No registry knowledge exists to fill it, and a self-hosted endpoint's capabilities depend on its launch flags rather than a vendor contract this repository can assert. opencode-free/muse-spark-1.2-contributor-free falls through because that provider declares modelReasoningEfforts only for the one-element OPENCODE_FREE_DEEPSEEK_MODELS list. Its roster is liveModels:true and heterogeneous, and the DeepSeek entries are declared precisely because they were pinned to a verified wire contract. Asserting a ladder for a discovered model nobody probed would be the same defect this unit exists to avoid, in the opposite direction. --- .../030_related_empty_ladders.md | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md b/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md index 0935e7973f..2525d1a16e 100644 --- a/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md +++ b/devlog/_plan/260904_anthropic_effort_ladder/030_related_empty_ladders.md @@ -48,3 +48,59 @@ contract — a ladder claim there needs a live probe, not a guess. A written finding per candidate naming the adapter, the capability evidence, and the verdict. Reported to the user regardless of whether any code changes. + +## Findings (260904) + +### `lidge/qwen3.8-27b-nvfp4` — NOT A DEFECT, and not this repository's to fix + +`lidge` is not a registry provider. The only two `lidge` matches in +`src/providers/registry.ts` are maintainer-attribution comments (lines 744 and +1781). It is the operator's own **custom provider** in `~/.opencodex/config.json`, +pointed at `http://100.100.125.116:8081/v1` on a private network, and it is +currently `disabled: true`. The model is a `customModels` row whose +`reasoningEfforts` is unset. + +There is no registry ladder that could fill it, and there should not be: it is a +self-hosted vLLM-style endpoint whose capabilities depend on the deployed server +and its launch flags, not on a vendor contract this repository can assert. The +correct fix is operator-side — set the ladder on the custom model, which the +management API already supports (`src/server/management/model-routes.ts` reads +`reasoningEfforts` on a custom-model PUT). + +Verdict: **no code change.** Report to the user as a configuration note. + +### `opencode-free/muse-spark-1.2-contributor-free` — NOT A DEFECT under the fix rule + +The provider DOES declare `modelReasoningEfforts`, but only for +`OPENCODE_FREE_DEEPSEEK_MODELS` — a one-element list, `deepseek-v4-flash-free` +(registry line 616). Every other Zen free model, including the muse row, falls +through with no ladder. + +That is not the Anthropic shape. Zen's free roster is `liveModels: true`: +discovered at runtime, changing on the vendor's schedule, and heterogeneous — +DeepSeek thinking models sit beside models with no reasoning at all. The +DeepSeek entries are declared precisely because they were pinned to a verified +wire contract (`modelReasoningEffortMap`, `preserveReasoningContentModels`, +issues #950/#994). Asserting a ladder for a discovered model whose upstream +effort support nobody verified would be exactly the failure mode this unit's +own rule forbids: an advertised control that may do nothing. + +Note also that `muse-spark-1.2-contributor-free` on the Zen tier is a different +route from `meta-muse/muse-spark-1.3`, which DOES carry a ladder +(`META_MUSE_REASONING_EFFORTS`, registry line 1508). So the capability is +already advertised where it was verified. + +Verdict: **no code change without a live probe** of the Zen route's effort +handling. Recorded as a candidate, not a defect. + +### What the two have in common + +Neither is the reported bug. The Anthropic case was a first-party provider with a +known vendor contract and an adapter that already honored effort — every fact +needed to assert the ladder was in the repository. These two are a private +self-hosted endpoint and a live-discovered free roster; in both the missing +ladder is an honest "unknown" rather than a lost fact. + +The general `derive.ts` per-model fill shipped in the Anthropic PR does help +both classes going forward: any operator who pins one model on these providers +will no longer suppress whatever registry knowledge exists for the others. From f1cdd10ceabf40472a0d668f9ca602d4c6b6152f Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 4 Sep 2026 22:16:48 +0900 Subject: [PATCH 3/6] docs(devlog): design the release version line so dev never inherits red MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every release forces `dev` to catch up. `dev-version-bump.yml` records four hand repairs in its own header, history shows "move dev to 2.4x.0" once per release, and while `dev` trails the highest tag `tests/release-version-line.test.ts` fails on `dev` AND on every open pull request — an inherited red a contributor cannot fix from their own diff. This unit designs the fix; no production file changes. ima2-gen solves the same problem with one atomic push of main+dev+tag, which is not portable here: `Protect dev` requires review and code-owner sign-off, and trading branch protection for chore removal is a bad exchange. What the design landed on, after the audit forced two retractions: - The per-release `dev` commit CANNOT be deleted. It is structural, following from `Protect dev` + `release.ts:494` allowedBranches + a monotonically advancing tag set. The first draft claimed otherwise and was wrong. - So the commit MOVES instead: the pre-move opens and merges the version PR BEFORE the release rather than after it. Same count of reviewed commits, no red window. - Ancestry is explicitly NOT a property this design maintains. An earlier draft asserted it; `release.ts:559-591` creates the release commit after promotion, so it is a descendant and can never be an ancestor. The assertion was withdrawn along with the test that would have enforced it. - Option A rides along: `--bump patch|minor|major` replaces a hand-passed version, with channel-specific algebra so a future preview tag cannot drag a stable bump onto the wrong core. - Publishing a preview for a higher core CLOSES the older stable patch line. This is a deliberate policy restriction, enforced at the publication boundary rather than only in the helper, and it is recorded as policy because history contains real counterexamples where a lower stable patch shipped after a higher-core preview. Six audit rounds: FAIL(5) -> FAIL(2) -> FAIL(3) -> FAIL(2) -> FAIL(1) -> PASS. Each blocker was verified against real code before folding, not relayed on trust. The measurements that changed the design are recorded in `000_research.md` §11 so the next reader does not re-derive a retracted claim. --- .../000_research.md | 342 +++++++++++++ .../260904_release_version_line/001_design.md | 141 ++++++ .../010_phase1_version_algebra.md | 191 +++++++ .../020_phase2_bump_input.md | 366 ++++++++++++++ .../030_phase3_premove.md | 473 ++++++++++++++++++ .../040_phase4_invariant_and_docs.md | 132 +++++ .../050_migration.md | 108 ++++ .../060_rollback_and_failure_modes.md | 119 +++++ 8 files changed, 1872 insertions(+) create mode 100644 devlog/_plan/260904_release_version_line/000_research.md create mode 100644 devlog/_plan/260904_release_version_line/001_design.md create mode 100644 devlog/_plan/260904_release_version_line/010_phase1_version_algebra.md create mode 100644 devlog/_plan/260904_release_version_line/020_phase2_bump_input.md create mode 100644 devlog/_plan/260904_release_version_line/030_phase3_premove.md create mode 100644 devlog/_plan/260904_release_version_line/040_phase4_invariant_and_docs.md create mode 100644 devlog/_plan/260904_release_version_line/050_migration.md create mode 100644 devlog/_plan/260904_release_version_line/060_rollback_and_failure_modes.md diff --git a/devlog/_plan/260904_release_version_line/000_research.md b/devlog/_plan/260904_release_version_line/000_research.md new file mode 100644 index 0000000000..ffe8972979 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/000_research.md @@ -0,0 +1,342 @@ +# 000 — Research: the release version line and the catch-up chore + +Design-only unit. Nothing here is implemented; this document records the current +state with file:line evidence, the exact recurrence, and the disposition of the +three options. Phase designs live in the decade documents (`010`+). + +Verified against `codex/260904-anthropic-effort-ladder` at `85eb58567` on +2026-09-04. Every line number below was read in this worktree, not recalled. + +## 1. Observed state, today + +| Surface | Value | Evidence | +|---|---|---| +| `dev` `package.json` | `2.43.0` | `git show origin/dev:package.json` line 3 | +| `main` `package.json` | `2.42.0` | `git show origin/main:package.json` line 3 | +| `preview` `package.json` | `2.43.0-preview.20260904` | `git show origin/preview:package.json` line 3 | +| highest git tag | `v2.42.0` -> `48f818664` | `git tag --sort=-v:refname` | +| npm `latest` | `2.42.0` | `npm view @bitkyc08/opencodex dist-tags --json` | +| npm `preview` | `2.40.0-preview.20260902` | same | +| `dev` vs `main` | `main` IS an ancestor of `dev`; `dev` is 25 commits ahead | `git merge-base --is-ancestor`, `git rev-list --count` | +| `preview` vs `dev` | `preview` is NOT an ancestor of `dev` | `git merge-base --is-ancestor` | + +Two facts in that table matter more than they look. + +**The npm `preview` dist-tag is three minor lines behind the `preview` branch.** +The branch carries `2.43.0-preview.20260904`, npm carries `2.40.0-preview.20260902`, +and the tag set contains no `v2.41.0-preview.*` or `v2.42.0-preview.*` at all. Those +two preview version lines were opened on the branch and never published. So on +`preview`, the in-tree version already does **not** mean "the version this branch +published" — it means "the version this branch is open for". That reading is not a +proposal; it is the reading `preview` has been operating under for at least two +cycles. The scheme in `030` generalises it rather than inventing it. + +**`dev` at `2.43.0` is currently legal only because `v2.43.0` does not exist yet.** +The moment `2.43.0` is published, `dev` is red — see §3. + +## 2. The recurrence + +Six commits, one per release, all doing the same thing: + +``` +ee2d19ad4 chore(release): move dev to 2.43.0 after v2.42.0 (PR #3434) +162d11e18 fix(release): move dev to 2.42.0 after v2.41.0 (PR #3354) +272ff6b11 fix(release): move dev to 2.41.0 after v2.40.0 (PR #3265) +3e0f99a19 chore(release): move dev to 2.40.0 after the v2.39.0 release (PR #3127) +71bd7bec6 chore(release): move dev to 2.39.0 after the v2.38.0 release (PR #3076) +a8c3a9633 chore(release): move dev to 2.38.0 after the v2.37.0 release (PR #3045) +``` + +Behind those sit the four hand repairs the tooling's own header names — +`32529c2b2`, `e4a85d134`, `076ad3036`, `befcac3e1` +(`scripts/bump-dev-version.ts:14`). `e4a85d134` is the one that ADDED the detector, +and two more repairs followed it. The script says so itself at +`scripts/bump-dev-version.ts:15-16`: "visibility was never the missing piece". + +There is a **third** version-line commit per train that the problem statement does +not name, and it must be in scope or the design under-counts the chore: + +``` +3959e6d04 chore(release): promote main v2.42.0 onto preview and open 2.43.0-preview +``` + +Its body states the cause in the same vocabulary: "The version could not stay at +2.42.0-preview.20260903: v2.42.0 has published, and compareReleaseTags ranks that +prerelease BEHIND its own stable release (-1), which is what +tests/release-version-line.test.ts fails on." + +So the real per-train cost is **three** version-line pull requests +(`promote-preview-*`, `promote-main-*`, `dev-version-*`), of which one +(`dev-version-*`) is pure post-hoc catch-up and one (`promote-preview-*`) is +post-hoc catch-up wearing a promotion's clothes. + +## 3. Why it is worse than a chore + +`tests/release-version-line.test.ts:88-120` compares `package.json` against the +highest local tag. Three outcomes: + +- strictly ahead -> pass (line 112-119) +- equal -> legal **only** if that tag names HEAD (`tagPointsAtHead`, lines 68-81, applied at 100-110) +- behind -> fail + +On `dev` after a stable publish, `package.json` equals the highest tag on a commit +that tag does not name, so the equality branch fails. The test runs in the ordinary +test jobs, and `tests/ci-workflows.test.ts:156-166` deliberately pins +`fetch-tags: true` on `test`, `platform-macos` and `platform-windows` so the tag +set is never empty. That is inherited red on `dev` **and on every pull request +opened against `dev`**, unfixable from a contributor's own diff. + +The blast radius is not limited to CI colour. `tests/release-version-line.test.ts:8-29` +records the two real failure modes: `assertChannelVersionMovesForward` +(`scripts/release.ts:342-370`) refuses to cut from such a tree, and merging `dev` +into `main` resolves `package.json` to `main`'s side and silently republishes an +already-published version. + +## 4. Where the coupling actually lives + +One line creates the whole problem: + +> `.github/workflows/release.yml:175-184` — `test "$PKG" = "$RELEASE_VERSION"` + +The in-tree version must EQUAL the version being published. Combined with +`tests/release-version-line.test.ts`'s rule that in-tree must be **strictly ahead** +of every tag except on the tagged commit itself, the two constraints force a +state change on every branch that shares content with the release commit, the +instant the tag appears. `main` and `preview` can absorb it (see §5); `dev` +cannot, because it is protected and a bot cannot merge into it. + +The asymmetry is the design's whole lever, and it is documented in the code: +`scripts/release.ts:112-130` explains that `main` and `preview` carry rulesets +whose admin bypass is `pull_request`, and that the carve-out is a dedicated write +deploy key registered as a `DeployKey` bypass actor **on those two rulesets**. +`.github/workflows/dev-version-bump.yml:12-15` states the converse for `dev`: +"It does not push to `dev`. It opens a pull request and a human merges it, because +ruleset `Protect dev` requires an approving review and code-owner sign-off that a +bot cannot supply." + +**`main` and `preview` are machine-writable. `dev` is not.** Any scheme that +requires `dev`'s version line to move in response to a publish is therefore +structurally a human chore with a red window in front of it. + +## 5. Current mitigation and why it does not close the hole + +`.github/workflows/release.yml:67-80` CALLS `dev-version-bump.yml` after a +successful publish. The workflow decides a version +(`scripts/bump-dev-version.ts:101-142`), proves it is unused by running the +detector in a `dev` checkout (`.github/workflows/dev-version-bump.yml:94-101`), +and opens a pull request (lines 103-187). + +It is a **prepared** repair, not a repair — the script's own header says so at +`scripts/bump-dev-version.ts:22-24`: "Until they do, the red persists." Two further +documented gaps, both in `MAINTAINERS.md:83-90`: the called workflow body resolves +from the caller's ref so it only takes effect once promoted to `main`, and a pull +request opened with `GITHUB_TOKEN` starts no `pull_request` workflows, so the bump +PR arrives with no CI at all. + +## 6. Option C — rejected, and not revisited here + +ima2-gen sends `main`, `dev` and the tag to one SHA in a single atomic push +(`/Users/jun/Developer/new/700_projects/ima2-gen/.github/workflows/release.yml:209-221`). +That works there because `dev` is machine-writable there. In opencodex it requires +relaxing `Protect dev`. Trading branch protection for chore removal is a bad +exchange, and the decision is already made: **out of scope, no phase proposes it.** + +Worth carrying over from that repository anyway, because they are independent of +the atomic push: the release version is *computed* from a bump keyword +(`scripts/release-cut.mjs:169-185`), immutability is asserted before anything is +pushed (`assertCuttable`, lines 106-112), and the stable tag is a certificate that a +preview build already proved the exact SHA (`assertPreviewProof`, lines 115-122). + +## 7. Option A — good, folded in, not sufficient + +Today the maintainer hand-passes a version string: `scripts/release.ts:487-491` +parses `args[0]` as the version, and `.github/workflows/release.yml:9-14` takes it +as a dispatch input that must equal `package.json`. + +Accepting `--bump patch|minor|major` and computing the number removes a class of +typo and makes "what is the next version" a function rather than a maintainer +judgment call. That is real value and `020` adopts it. + +It does **not** fix the root cause. Whether the string `2.43.0` arrives typed or +computed, `release.yml:175-184` still demands the tree equal it, and `dev` still +has to move afterwards. Option A shortens the chore's input; it does not delete +the chore. + +## 8. The complete consumer set + +Searched with `rg` for `bump-dev-version`, `dev-version-bump`, +`release-version-line`, and for readers of `package.json.version` under `src/`. +The full list, including three consumers the task brief did not name: + +| Consumer | Role | Named in brief | +|---|---|---| +| `scripts/release.ts` | version arg, branch gate, channel/unused guards, bump+commit+push | yes | +| `scripts/bump-dev-version.ts` | the catch-up decision | yes | +| `.github/workflows/release.yml` | equality check, branch/version coupling, dist-tag, bump call | yes | +| `.github/workflows/dev-version-bump.yml` | opens the catch-up PR | yes | +| `tests/release-version-line.test.ts` | the invariant | yes | +| `tests/bump-dev-version.test.ts` | pins the catch-up rule | yes | +| `tests/ci-workflows.test.ts` | pins release.yml shape (`636-830`) and `fetch-tags` (`156-166`) | yes | +| `tests/release-helper.test.ts` | pins release.ts call order (`353-731`) | yes | +| **`scripts/release-notes.ts`** | `compareReleaseTags`, `selectReleaseBaseline`, `previousReleaseNotesTag` (`86-134`) | **no** | +| **`scripts/build-release-changelog.ts`** | baseline selection + notes text (`542-577`, `646-648`) | **no** | +| **`MAINTAINERS.md:76-90`** | documents the chore as policy | **no** | +| **`src/update/index.ts:49,59-64`** | reads in-tree version; `updateTag()` derives the channel from it | **no** | +| **`src/cli/version-skew.ts:34-46`** | compares CLI version against live proxy | **no** | +| **`src/server/management-api.ts:89`, `src/client/machine-listener.ts:21`** | report in-tree version at runtime | **no** | +| **`docs-site/src/content/docs/contributing.md:98-100`** (+7 locales) | documents `bun run release ` | **no** | +| **`structure/06_docs-and-release.md:181,240,253`** | architecture SoT for the release path | **no** | + +`src/update/index.ts:59-64` is the one that changes user-visible behaviour rather +than tooling, and it is the sharpest constraint on any scheme that lets the in-tree +version drift away from the published one — see `010` §4 and `050` §3. + +## 9. Two comparators, one question + +`compareReleaseVersions` (`scripts/release.ts:303-337`) and `compareReleaseTags` +(`scripts/release-notes.ts`) both order releases. `bump-dev-version.ts:57` imports +the *latter*, and `tests/release-version-line.test.ts:27-29` records why: importing +`scripts/release` from a test kills the runner, because it parses `process.argv` +and calls `process.exit` at module scope (`scripts/release.ts:482-491`). + +So the repository's ordering rule is implemented twice and the tests can only reach +one of them. That is a foundation defect, not a style nit: every phase below +depends on both agreeing. `010` fixes it first for that reason. + +## 10. Open questions + +Stated rather than papered over. + +1. **Is the `preview` npm gap deliberate?** npm `preview` is `2.40.0-preview.20260902` + while the branch is at `2.43.0-preview.20260904` and no matching tags exist. Either + the last two preview cuts were abandoned, or previews stopped being published. The + design in `030` is correct under both readings, but the migration in `050` differs. + I could not determine which from the repository alone. +2. **Does anything outside this repository consume the tag-to-`package.json` identity?** + Trusted-publishing provenance attests the workflow and commit, not file equality, so + B1 (`010` §5) survives it in principle. I did not verify against a published + attestation, so B1's cost is asserted from the npm docs model, not measured. +3. **`gui/package.json` and `docs-site/package.json`** are `0.0.0` / `0.0.1` and + unpublished (`devlog/_plan/260827_dev_hardening/010_wp2_version_line.md:8-13`). I + re-confirmed no second product version exists in tracked source. If one is added + later this design does not cover it. +## 11. Audit round 1 — resolved facts (2026-09-04) + +Amendments from an independent review whose findings I verified. These supersede +the corresponding open questions above. + +### 11.1 Open question 2 — RESOLVED, provenance does not bind the tree + +**Verified against the published attestation for `v2.42.0`**, not reasoned from the +model. The SLSA predicate binds: + +- `subject` = the **tarball's** sha512 +- `workflow` = `.github/workflows/release.yml` +- `resolvedDependencies` = git commit `48f8186647d9ffb108d226dcfa91a64225aae2a7` + +It does **not** assert that the tarball byte-matches the git tree. npm additionally +reports `gitHead=48f8186...`, and `rg` finds **no** non-devlog consumer of `gitHead` +in `scripts/`, `tests/` or `.github/`. + +So a publish-time divergence between tarball and tree would be an expectation +problem, not a broken attestation. Recorded as settled; the hedging in the original +`060` §5 is withdrawn. (This matters less than it did — the revised scheme in +`001_design.md` no longer creates such a divergence at all.) + +### 11.2 The catch-up PR is the ancestry path — structural finding + +This one invalidates the original scheme's central claim and is worth stating in +full, because §2 above under-read its own evidence. + +``` +ee2d19ad4 parent: 48f8186 (single parent — the v2.42.0 release commit) +c116dc532 merge of ee2d19ad4 into dev +``` + +`ee2d19ad4` has **exactly one parent**, and that parent is the release commit. The +catch-up branch is cut *from* the release commit, so merging it is the **only path** +by which the release commit becomes an ancestor of `dev`. +`git merge-base --is-ancestor v2.42.0 origin/dev` returns true today *because of* +that pull request, not incidentally to it. + +Consequence for any scheme that deletes the catch-up: it deletes the ancestry +propagation too. `scripts/release.ts:494` (`allowedBranches`), `:584` (pushes only +the release branch) and `.github/workflows/release.yml:412-421` (pushes only the +tag) confirm nothing else ever moves `dev`. + +**Therefore: a reviewed commit into `dev` is required whenever a release would +otherwise leave `dev` at or behind the new tag.** It follows from `Protect dev` plus +a monotonically advancing tag set, and no in-tree version convention can remove it. + +> **Correction (audit round 3).** An earlier wording here said "at least one reviewed +> commit per release", which is too strong. A preview cut, or a stable hotfix below +> `dev`'s line, needs none: `decideDevVersion` returns `changed: false` in exactly +> that case (`scripts/bump-dev-version.ts:120-126`). `001_design.md` §1 carries the +> corrected rule. + +> **Superseded (audit round 3).** The paragraph above this correction described the +> catch-up pull request as the ancestry carrier into `dev`. That observation is +> factually true of `ee2d19ad4` but was **withdrawn as a design obligation**: +> measured across all 226 release tags, 10 are not ancestors of `origin/dev` (every +> one a preview), so "release tags are ancestors of dev" is already false today. The +> design does not preserve or assert ancestry — see `001_design.md` §0. + +### 11.3 Preview succession can be a fixed point + +Live state: `origin/preview` = `2.43.0-preview.20260904`, npm `preview` = +`2.40.0-preview.20260902`, highest stable tag = `v2.42.0`. + +Publishing `2.43.0-preview.20260904` today gives +`nextDevelopmentVersion(X) = 2.43.0`, and a same-day stamp regenerates +`2.43.0-preview.20260904` — i.e. `N(X) == X`. Any successor function must be proven +**strictly monotonic**, not merely well-formed. + +Second, related hazard: computing a bump from the **preview dist-tag alone** starts +from `2.40.0-preview.20260902` and can propose a `2.41.*` candidate that is behind +the published stable `v2.42.0`. A preview candidate must be computed against the +union of the stable tag set and the preview channel. + +### 11.4 The compatibility manifest hashes `package.json` + +Read directly: `scripts/generate-compatibility-version.ts:15` lists +`REQUIRED_ROOT_FILES = ["package.json", "bun.lock", "scripts/model-metadata.source.json"]`, +and `buildCompatibilityVersionManifest` (lines 44-83) hashes each file's +**working-tree bytes** via `git ls-files` + `sha256`. + +Chain: `package.json:52` `prepublishOnly` -> `build:gui` (line 49) -> `prepare:package` +(line 50) -> `prepare-package.ts:4` -> `generateCompatibilityVersionManifest`. +Separately `gui/vite.config.ts:7` bakes the root `package.json` version into the GUI +bundle as `__APP_VERSION__`. + +So **the version string is an input to Compatibility Lab route identity and to the +GUI bundle**, and both are regenerated by `prepublishOnly`/`prepack` — i.e. *after* +any pre-publish working-tree inspection. This is what makes publish-time version +rewriting far more invasive than it appears, and it is a decisive argument against +the original `030`. + +### 11.5 Consumer inventory — additions + +Missing from §8, found by the reviewer and confirmed: + +| Consumer | Nature | +|---|---| +| `gui/vite.config.ts:7` | bakes root version into the GUI bundle (`__APP_VERSION__`) | +| `scripts/generate-compatibility-version.ts:15` | `package.json` bytes feed compatibility identity | +| `bin/ocx.mjs` | launcher version reporting | +| `src/server/gui-static.ts`, `src/cli/help.ts` | version display surfaces | +| `scripts/openai-provider-option-runtime-child.ts` | reads package version | +| `src/cli/star-prompt.ts:196` | per-version star deferral ("at most once per version") | + +Most are display surfaces covered by the source-checkout caveat. The compatibility +manifest is **not** — it is a content-addressed identity, and a version change moves +it. The star deferral is a behavioural one: it re-arms per version string. + +### 11.6 Comparator fallback is load-bearing + +`scripts/release-notes.ts:66-70`: when either side fails `parseReleaseTag`, +`compareReleaseTags` falls back to `localeCompare(..., { numeric: true })` rather +than throwing. `scripts/build-release-changelog.ts:137` admits any `/^v\d/` tag into +the candidate set. A single malformed historical tag therefore sorts harmlessly +today; a throwing comparator would newly abort release-note generation. + +Any consolidation must preserve the fallback at the `compareReleaseTags` boundary. diff --git a/devlog/_plan/260904_release_version_line/001_design.md b/devlog/_plan/260904_release_version_line/001_design.md new file mode 100644 index 0000000000..287e602072 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/001_design.md @@ -0,0 +1,141 @@ +# 001 — Design: move `dev` before the release so red is never inherited + +This document is the current plan in full. It supersedes two earlier versions of +itself; the history lives in `000_research.md` §11 and is not needed to implement. + +## 0. The contract, stated once + +**This design has exactly one goal: `dev` and its open pull requests never inherit a +version-line failure.** + +It does **not** preserve, restore, or assert any ancestry relationship between +release tags and `dev`. That claim appeared in an earlier draft and is **withdrawn** +— it was both unnecessary and unachievable. Reviewer option (i), chosen deliberately: + +- **Unnecessary.** The finding that `ee2d19ad4`'s single parent is the `v2.42.0` + release commit proves the catch-up PR *happened to be* the ancestry carrier. It + does not show anything requires ancestry. Nothing in the build, test, release or + promotion path reads it. +- **Unachievable under this design.** `scripts/release.ts:559-591` creates and pushes + the release commit on `main` *after* promotion. Under a pre-move, `dev` moves and + is promoted first, so the release commit is a **descendant** of the promoted state + and can never be its ancestor. +- **Already false today.** Measured across all 226 release tags: **10 are not + ancestors of `origin/dev`**, every one a preview tag (`v2.33.0-preview.20260825`, + `v2.34.0-preview.20260827`, `v2.36.0-preview.20260829`, `v2.36.0-preview.20260830`, + `v2.39.0-preview.20260901`, `v2.40.0-preview.20260902`, among others). An + "every release tag is an ancestor of dev" assertion fails on today's repository + before any of this lands. + +So: **release commits live on `main` and are not carried into `dev`. `dev` receives +the version line, not the commit.** That is the honest description of what this +repository does, and this design does not change it. + +## 1. What cannot be removed + +A version-line commit into `dev` is required before any release that would otherwise +leave `dev` at or behind the new tag. This follows from three verifiable facts: + +1. `Protect dev` requires an approving review and code-owner sign-off; a bot cannot + merge (`.github/workflows/dev-version-bump.yml:12-15`). +2. Nothing in the release path writes to `dev`: `scripts/release.ts:494` + (`allowedBranches = ["main", "preview"]`), `:584` (pushes only that branch), + `.github/workflows/release.yml:412-421` (pushes only the tag). +3. The invariant requires the in-tree version to outrank every tag + (`tests/release-version-line.test.ts:88-120`). + +**The precise rule, corrected:** *one reviewed `dev` move before any release that +would otherwise leave `dev` at or behind the resulting tag.* Not "one per release". +A preview cut, or a stable hotfix, needs **no** `dev` commit when `dev` already +outranks it — `decideDevVersion` returns `changed: false` in exactly that case +(`scripts/bump-dev-version.ts:120-126`). With `dev` at `2.44.0`, releasing +`2.43.1` or `2.44.0-preview.*` requires nothing. + +Option C (ima2-gen's atomic push to `dev`) is the only thing that removes the +commit entirely, and it is rejected: it trades branch protection for a chore. + +## 2. The mechanism + +``` +today: publish vX -> dev is RED -> open PR -> review -> merge -> green +after: open PR -> review -> merge -> promote -> publish vX -> never red +``` + +Same pull request, same script, same rule. It runs before the release instead of +reacting to it, and a gate in `release.yml` refuses to publish when it has not. + +Nothing about npm, tags, provenance, packing or the compatibility manifest changes. +The release commit still carries the published version, so +`.github/workflows/release.yml:175-184` stays exactly as it is. + +## 3. Version semantics — unchanged + +This design changes **when** `dev`'s version moves, not what any version means. + +| Point | Meaning | Changed? | +|---|---|---| +| `dev` | next unpublished version this line works toward | no | +| release commit (`main`) | the version being published | no | +| release commit (`preview`) | the prerelease being published | no | +| git tag `vX` | names the commit whose `package.json` says `X` | no | +| npm tarball | `X`, packed from the tree | no | +| **timing of dev's move** | **before the release, not after** | **yes** | + +`tagPointsAtHead` (`tests/release-version-line.test.ts:68-81`) is **retained**: the +release commit still equals its own tag, so the exception is still load-bearing. + +## 4. Phase map + +``` +010 shared version algebra, channel-aware and fallback-preserving [foundation] + | +020 --bump, computed with channel-specific semantics [needs 010] + | +030 pre-move: open the dev PR before the release + readiness gate [needs 010] + | +040 documentation + retained invariant [needs 020 + 030] +``` + +`020` and `030` are independent of each other; either may land first. `040` needs +**both** — it documents the patch-line policy `020` implements and the ordering `030` +enforces. `050` covers migration, `060` rollback and failure modes. + +## 5. Consumer reconciliation + +| Consumer (file:line) | Disposition | +|---|---| +| `scripts/release.ts:303-337` `compareReleaseVersions` | delegates to shared module (`010`) | +| `scripts/release.ts:342-370` channel-forward guard | survives — argument-driven | +| `scripts/release.ts:372-391` unused-version guard | survives — argument-driven | +| `scripts/release.ts:494-511` branch gate | survives | +| `scripts/release.ts:559-591` bump/commit/push | survives — still commits `X` | +| `scripts/release.ts:615` dispatch | survives — no new input | +| `release.yml:175-184` equality check | **survives unchanged** | +| `release.yml:357-368` publish | survives unchanged | +| `release.yml:39-80` bump call | replaced by a readiness gate (`030`) | +| `dev-version-bump.yml` | repurposed: opener, not repairer (`030`) | +| `scripts/bump-dev-version.ts` | retained, retargeted (`030`) | +| `tests/bump-dev-version.test.ts` | retained, extended (`030`) | +| `tests/release-version-line.test.ts` | retained; **assertions unchanged**, header comment only (`040`) | +| `tests/ci-workflows.test.ts` | workflow assertions (`030`) | +| `tests/release-helper.test.ts` | `--bump` cases (`020`) | +| `scripts/release-notes.ts:66-70` | survives; **fallback preserved** (`010`) | +| `scripts/build-release-changelog.ts:137` | survives — tag-driven | +| `gui/vite.config.ts:7` | survives — no pack-time version change | +| `scripts/generate-compatibility-version.ts:15` | survives — no pack-time mutation | +| `src/cli/star-prompt.ts:196` | survives | +| `src/update/index.ts:49,59-64` | survives — tag checkout reports `X` | +| `MAINTAINERS.md:76-90` | documentation (`040`) | +| `structure/06_docs-and-release.md` | SoT sync (`040`) | + +## 6. Honest assessment + +This moves one pull request earlier. It does not delete work, and it does not +maintain ancestry. + +What it buys: the inherited red — the one contributor-facing harm — stops existing, +and a gate makes forgetting the pre-move a blocked release rather than a silent +failure that ten releases in a row have paid for. + +What it costs: a release now has an ordering requirement that a maintainer must +follow, enforced by a gate that can refuse at an inconvenient moment (`060` §3). diff --git a/devlog/_plan/260904_release_version_line/010_phase1_version_algebra.md b/devlog/_plan/260904_release_version_line/010_phase1_version_algebra.md new file mode 100644 index 0000000000..fa0054ecd7 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/010_phase1_version_algebra.md @@ -0,0 +1,191 @@ +# 010 — Phase 1: one version algebra, in a testable module + +Foundation. No behaviour change; this phase gives phases 2-4 a single, importable +definition of ordering and succession. + +Depends on: nothing. Everything else depends on this. + +## 1. The defect this closes + +The repository orders releases in two places: + +- `compareReleaseVersions` — `scripts/release.ts:303-337` (throws on bad input) +- `compareReleaseTags` — `scripts/release-notes.ts:66-79` (falls back on bad input) + +Tests can only reach the second. `tests/release-version-line.test.ts:27-29` records +why: `scripts/release.ts` parses `process.argv` and calls `process.exit` at module +scope (`:482-491`), so importing it from a test kills the runner. +`compareReleaseVersions` is therefore exercised only through a subprocess fixture +(`tests/release-helper.test.ts:708-723`, three cases). + +## 2. Two comparators, deliberately + +The two behaviours are **not** an accident to be unified. They serve different +callers and both are correct: + +```ts +/** + * Strict ordering for release DECISIONS. Throws on unparseable input, because a + * decision must fail closed: scripts/release.ts:305-307 records that Number() on a + * garbage core yielded NaN and made the forward guard pass any candidate. + */ +export function compareVersions(left: string, right: string): number; + +/** + * Lenient ordering for TAG SETS, which contain whatever history contains. Falls back + * to numeric-aware locale compare exactly as release-notes.ts:66-70 does today. + */ +export function compareTagsLenient(left: string, right: string): number; +``` + +Collapsing them onto one throwing function would be a live regression: +`scripts/build-release-changelog.ts:137` admits any `/^v\d/` tag into its candidate +set, so a single malformed historical tag — harmless today — would newly **abort +release-note generation**. + +## 3. File change map + +| Path | Action | +|---|---| +| `scripts/version-line.ts` | **NEW** — the algebra | +| `scripts/release-notes.ts` | MODIFY — `compareReleaseTags` delegates to `compareTagsLenient` | +| `scripts/release.ts` | MODIFY — delete the duplicate, re-export `compareVersions` | +| `scripts/bump-dev-version.ts` | MODIFY — use the shared `nextDevelopmentVersion` | +| `tests/version-line.test.ts` | **NEW** | +| `tests/bump-dev-version.test.ts` | unchanged — see §7 | + +## 4. `scripts/version-line.ts` + +Pure at the module level: no I/O, and nothing that runs on import. That is what makes +it importable from a test, which is the whole reason it exists — +`scripts/release.ts` is unimportable precisely because it parses `process.argv` and +exits at module scope (`tests/release-version-line.test.ts:27-29`). + +`030` later adds a small CLI to this file behind an `import.meta.main` guard, the +same pattern `scripts/release-notes.ts` uses. That guard is what keeps the module +importable, so it does not weaken this property — but every exported function must +stay free of `process.exit` so a caller decides what a failure means. + +```ts +export interface ParsedVersion { + major: number; minor: number; patch: number; + prerelease: readonly string[] | null; +} + +/** Optional leading v, optional prerelease, optional (ignored) build metadata. */ +export function parseVersion(raw: string): ParsedVersion | null; + +export function compareVersions(left: string, right: string): number; +export function compareTagsLenient(left: string, right: string): number; + +/** + * The version a development line carries once \`released\` exists. + * + * X.Y.Z-preview.* -> X.Y.Z (befcac3e1) + * X.Y.Z (stable) -> X.(Y+1).0 (e4a85d134, 076ad3036, 32529c2b2) + * + * Lifted from scripts/bump-dev-version.ts:106-108. The rule was got wrong once in + * design — "increment the released minor" — and befcac3e1 disproves it, so the + * prerelease row is load-bearing rather than an edge case. + */ +export function nextDevelopmentVersion(released: string): string; +``` + +`nextDevelopmentVersion` takes one argument. `decideDevVersion`'s second parameter +answers "is dev already ahead?" (`scripts/bump-dev-version.ts:120-126`), which stays +in that script because `030` still needs it. + +`020` extends this module with `nextStableRelease` and `nextPreviewRelease`. They are +not part of this phase. + +## 5. `scripts/release-notes.ts` + +`compareReleaseTags` keeps its name, signature and **exact current behaviour** — +`tests/release-version-line.test.ts:4` and `scripts/bump-dev-version.ts:57` import it: + +```diff ++import { compareTagsLenient } from "./version-line"; ++ + export function compareReleaseTags(a: string, b: string): number { +- const pa = parseReleaseTag(a); +- const pb = parseReleaseTag(b); +- if (!pa || !pb) return a.localeCompare(b, undefined, { numeric: true, sensitivity: "base" }); +- /* ... core/prerelease comparison ... */ ++ return compareTagsLenient(a, b); + } +``` + +`compareTagsLenient` accepts an optional `v` prefix, which is what +`scripts/bump-dev-version.ts:68-70` (`asTag`) works around today; that helper's own +comment (lines 61-67) records the `vv2.36.0` double-prefix bug the workaround caused. +Accepting both forms in the parser removes the class. + +## 6. `scripts/release.ts` + +```diff +-export function compareReleaseVersions(left: string, right: string): number { +- /* lines 303-337 */ +-} ++export { compareVersions as compareReleaseVersions } from "./version-line"; +``` + +The alias keeps `assertChannelVersionMovesForward` (`:360`) and the three +`tests/release-helper.test.ts` cases (`708-723`) untouched. + +## 7. `scripts/bump-dev-version.ts` + +Retained — `030` retargets it. Here it only stops owning the rule: + +```diff ++import { nextDevelopmentVersion } from "./version-line"; ++ + export function decideDevVersion(released: string, current: string): BumpDecision { +- const candidate = rel.prerelease === null +- ? \`\${rel.major}.\${rel.minor + 1}.0\` +- : \`\${rel.major}.\${rel.minor}.\${rel.patch}\`; ++ const candidate = nextDevelopmentVersion(released); +``` + +The ahead-check, the atomic rewrite and the CLI are unchanged, so +`tests/bump-dev-version.test.ts` stays green **without edits**. That is the proof the +extraction was faithful, and it is this phase's primary gate. + +## 8. IN / OUT + +IN: creating `scripts/version-line.ts`; redirecting the three callers; adding +`tests/version-line.test.ts`. + +OUT: release behaviour, workflow YAML, the invariant test's logic, `--bump`, the +`020` resolvers. A workflow file in this phase's diff should be rejected. + +## 9. Accept criteria + +1. `bun test tests/version-line.test.ts` — new. Covers the `decideDevVersion` rows + from `tests/bump-dev-version.test.ts:44-96` re-expressed against + `nextDevelopmentVersion`, the build-metadata and unparseable cases from + `tests/release-helper.test.ts:708-723`, and the lenient/strict distinction: + `compareTagsLenient("vNOTAVERSION", "v2.42.0")` returns a number, + `compareVersions` on the same input throws. Both in one test so the distinction + cannot be optimised away later. +2. `bun test tests/bump-dev-version.test.ts` — **unchanged file, still green.** +3. `bun test tests/release-notes.test.ts` — green; the file that would catch a + fallback regression (948 lines). +4. `bun test tests/release-version-line.test.ts` — green. +5. `bun test tests/release-helper.test.ts` — green. +6. `bun run typecheck`. + +All six exist and read the changed files directly: `bun run typecheck` is +`bun x tsc --noEmit`, which covers `scripts/` under the root tsconfig, and each +`bun test` names its file as a direct argument. Verified to exist and be correctly +targeted, not verified to pass — no code exists yet. + +## 10. Activation grounding + +| Path | Trigger | Observable | +|---|---|---| +| `parseVersion` returns null | `"not-a-version"`, `"2.36"`, `"garbage"` (the strings at `tests/bump-dev-version.test.ts:92-96`) | `compareVersions` throws `not parseable` | +| lenient fallback | `compareTagsLenient("vNOTAVERSION", "v2.42.0")` | returns a number, no throw | +| prerelease succession | `nextDevelopmentVersion("2.36.0-preview.20260829")` | `"2.36.0"`, not `"2.37.0"` | + +Row 1 matters specifically because `scripts/release.ts:305-307` documents that the +NaN path once made the forward guard accept anything. diff --git a/devlog/_plan/260904_release_version_line/020_phase2_bump_input.md b/devlog/_plan/260904_release_version_line/020_phase2_bump_input.md new file mode 100644 index 0000000000..6d39ca1d21 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/020_phase2_bump_input.md @@ -0,0 +1,366 @@ +# 020 — Phase 2: `--bump`, with channel-specific algebra + +`scripts/release.ts` accepts `--bump patch|minor|major` as an alternative to a typed +version string. The resolved version `X` then flows exactly where the typed string +went: same guards, same commit, same dispatch. + +Depends on: `010`. Independent of `030`. + +## 1. Scope + +The helper still commits `X` to `package.json`, still commits `release: vX`, and +still dispatches `version=X` with no additional input. This phase adds an input +*spelling*, not a new release layout. + +## 2. File change map + +| Path | Action | +|---|---| +| `scripts/version-line.ts` | MODIFY — add the channel-specific resolvers (§4) | +| `scripts/release.ts` | MODIFY — argument parsing (§3) | +| `tests/version-line.test.ts` | MODIFY — resolver cases (§6) | +| `tests/release-helper.test.ts` | MODIFY — CLI cases (§7) | +| `docs-site/src/content/docs/contributing.md` (+7 locales) | MODIFY — document `--bump` | + +`scripts/version-line.ts` and `tests/version-line.test.ts` are created by `010` and +extended here. + +## 3. Argument resolution + +Replaces `scripts/release.ts:487-491`. The typed form is unchanged; the new branch +resolves `X` from a bump kind. + +``` +// --bump and an explicit version are mutually exclusive; exactly one is required. +// Kind is validated before any network call. +// +// Channel-specific: a stable bump and a preview bump do NOT share a base, and both +// resolvers take the full tag/channel picture. See §4. +const version = explicit ?? (tag === "preview" + ? nextPreviewRelease({ + kind: bumpKind!, + stableTip, stableTags, + previewTip, previewTags, + stamp: utcStamp(), + }) + : nextStableRelease({ + kind: bumpKind!, + stableTip, stableTags, + previewTags, // needed for the §4.0 refusal check + })); +``` + +Resolution must sit **after** the branch gate (`scripts/release.ts:494-511`) so +`tag` is known and a wrong-branch invocation aborts before any network call, and +after `packageName` (`:513`). Tags come from `git tag --list 'v*'`, partitioned into +stable and preview by `parseVersion`; the two channel tips come from the single +`npm view dist-tags --json` call the script already makes at `:343`. + +## 4. Channel-specific algebra + +### 4.0 The cross-channel ordering contract — READ FIRST + +**Contract (i), chosen: ordering stays global, and publishing a higher-core preview +CLOSES older stable patch lines. This is a deliberate release-policy RESTRICTION, +not the preservation of an unused capability.** + +Stated first because every resolver below depends on it. Probed against the real +comparator in this repository: + +``` +compareReleaseTags("v2.42.1", "v2.43.0-preview.1") = -1 +compareReleaseTags("v2.43.1", "v2.44.0-preview.1") = -1 +compareReleaseTags("v2.42.1", "v2.42.0-preview.9") = 1 +``` + +A prerelease of a **higher core** outranks a stable **lower** core. So once +`v2.43.0-preview.1` is tagged, `2.42.1` is below the highest tag, and two things +reject it: the global-floor assertion in §4.4, and +`tests/release-version-line.test.ts:88-120` — **unchanged by this unit** — which +would reject the resulting release commit as behind the highest tag. + +**Therefore a `patch` bump is refused when a preview tag exists for a core above the +base.** The resolver raises an explanatory error instead of returning a version that +cannot be released. In plain terms: **opening a preview for `2.43.0` ends the +`2.42.x` patch line.** A fix after that point ships as part of `2.43.0`. + +#### What this gives up — measured, not assumed + +An earlier draft of this document claimed the capability was essentially unused, +"apart from `v2.32.1`". **That claim was false and is retracted.** The measurements: + +- **103 of 143 stable tags have `patch > 0`** (`git tag --list 'v*'`, prereleases + excluded). Patch releases are the historical norm, not an exception. +- The exact pattern this policy forbids — a **lower stable patch published AFTER a + higher-core preview** — has happened at least three times, verified by commit + timestamp: + +| higher-core preview | then a lower stable patch | +|---|---| +| `v2.6.24-preview.20260705` @ 2026-07-05 18:04:58 | `v2.6.23` @ 2026-07-05 19:40:28 | +| `v2.6.26-preview.20260705` @ 2026-07-05 20:15:33 | `v2.6.24` @ 2026-07-05 20:15:40 | +| `v2.7.39-preview.20260724` @ 2026-07-24 15:12:26 | `v2.7.37` @ 2026-07-24 15:23:24 | + +These counterexamples are kept in the plan deliberately, so nobody re-derives the +retracted "unused capability" claim from a fresh look at recent history. + +#### The actual rationale + +Three things, none of which is "nobody used it": + +1. **The current global invariant already disallows it.** + `tests/release-version-line.test.ts:88-120` refuses any tree behind the highest + tag, with no channel awareness. The three rows above predate that test's current + form. This plan does not impose a new restriction; it makes an existing one + **explicit and legible at the point of use**, instead of letting a maintainer + discover it from a confusing failure two steps later. +2. **Recent trains have converged on `.0` stable releases.** The last patch release + is `v2.32.1` (2026-08-25); every release since has been `X.Y.0`. The restriction + binds a workflow the repository is not currently using, even though it certainly + used it before. +3. **The alternative weakens the unit's central guard.** (ii) requires the invariant + itself to become channel-aware, i.e. changing the one file this unit has been + careful not to weaken — the rule that makes a stale `dev` detectable at all. + +So the honest framing: this is a **policy decision to keep the invariant simple**, +paid for with a capability the repository exercised in the past and has not +exercised recently. It is not free, and a maintainer who wants patch lines back +should read §4.0a rather than assume nothing was lost. + +#### 4.0a If patch lines must be reopened + +That is contract (ii), and it is a separate unit: the invariant becomes +channel/branch-aware, `tests/release-version-line.test.ts` changes with it, and the +release-note baseline selection (`scripts/build-release-changelog.ts:129-141`) needs +re-examination because it currently filters candidates by global ordering too. +Changing only the bump resolver is insufficient — that was this document's round-3 +error and it is recorded here so the next attempt starts from the right scope. + +If a stable patch line ever genuinely must survive an open preview, that is a +separate unit per §4.0a, argued on its own evidence. + +#### Enforcement lives at the publication boundary, not here + +The resolver's refusal is **advisory**: it only fires when a maintainer uses +`--bump`. A stable patch SHA that passed CI *before* a higher-core preview was +tagged can still be dispatched manually afterwards, bypassing this function +entirely. `030` §5a adds the real gate in `release.yml`, after the fresh tag fetch. +A policy only the happy path honours is not a policy. + +### 4.1 Why a single floor is wrong + +A single `max(tags ∪ channel)` floor produces three concrete failures: + +1. `latest=2.42.0` with an existing `v2.43.0-preview.1` makes the global floor the + preview, so `--bump minor` yields **2.44.0** and skips the intended 2.43.0. +2. From floor `2.42.0`, `--bump minor` gives `2.43.0`; feeding that to a successor + required to return something strictly greater **cannot** produce + `2.43.0-preview.*`, because a prerelease ranks *below* its own stable core. +3. A stable bump computed from a global floor lands on a future preview core, which + is not a stable version at all. + +So the channels get separate functions with separate bases. + +### 4.2 The resolvers + +```ts +/** + * The next STABLE release. Base is the stable line only: the newest of the 'latest' + * dist-tag and the stable tag set. A future same-core PREVIEW must not raise this + * base — v2.43.0-preview.1 existing means 2.43.0 is being worked toward, not + * consumed. + * + * REFUSES kind="patch" when a preview tag exists for a core above the base: per + * §4.0 the result would rank below the highest tag and could never be released. + * previewTags is an input ONLY for that refusal check; it never raises the base. + */ +export function nextStableRelease(input: { + kind: "patch" | "minor" | "major"; + stableTip: string | null; // npm 'latest' + stableTags: string[]; // tags with no prerelease component + previewTags: string[]; // refusal check only +}): string; + +/** + * The next PREVIEW release: a core outranking the newest stable, then a prerelease + * outranking every existing preview on that core. + * + * Two-step by necessity. A preview is BELOW its own stable core, so it can never be + * derived by bumping a global floor — the result would either collide with a + * published preview or rank behind the stable it precedes. + * + * kind selects the CORE, exactly as for a stable release; the prerelease suffix is + * then attached to it. Without kind, patch/minor/major would all resolve + * identically and the flag would be silently ignored. + * + * Both tips AND both tag sets are required. npm metadata and the tag set can + * disagree — the live repository is in exactly that state (npm preview + * 2.40.0-preview.20260902 vs origin/preview 2.43.0-preview.20260904, with no + * matching tag) — and a resolver seeing only one source cannot advance past a + * partial publication. + */ +export function nextPreviewRelease(input: { + kind: "patch" | "minor" | "major"; + stableTip: string | null; // npm 'latest' + stableTags: string[]; // the core floor + previewTip: string | null; // npm 'preview' + previewTags: string[]; + stamp: string; // YYYYMMDD, supplied by the caller +}): string; +``` + +### 4.3 How `nextPreviewRelease` resolves + +1. **Core.** `base = max(stableTip, newest stable tag)`, then apply `kind`: + `minor` -> `X.(Y+1).0`, `major` -> `(X+1).0.0`, `patch` -> `X.Y.(Z+1)`. + For `minor` this equals `nextDevelopmentVersion(base)`; the other kinds are + precisely why `kind` must be an input. +2. **The incumbent.** Compute + `incumbent = max(previewTip, ...previewTags)` **restricted to the resolved core**, + using the strict comparator. Both sources feed one maximum: that is what makes an + npm/tag disagreement safe, and neither source alone is sufficient (§6 rows 7-8). + When no preview exists on that core, the candidate is `-preview.` and + the remaining steps do not apply. +3. **Succession from the incumbent, not from the stamp.** Compare the supplied + `stamp` against the incumbent's stamp: + + | supplied stamp vs incumbent's | candidate | + |---|---| + | strictly newer | `-preview.` (bare) | + | equal | `-preview..`, where `n` is the incumbent's ordinal (absent = 1) | + | older | **throw** a clock-regression error naming both stamps | + + Deriving the ordinal from the **incumbent's** ordinal is what makes `.3` -> `.4` + work; a hard-coded `.2` would collide as soon as a third same-day cut happened. + The ordinal ordering is SemVer's: numeric identifiers compare numerically, and a + longer identifier set outranks a shorter one when all preceding identifiers are + equal — the comparator at `scripts/release.ts:323-335` already implements it. + + The **older** row is a real state, not a hypothetical: a runner with a skewed + clock, or a maintainer passing an explicit stamp, can produce it. Silently + emitting a behind candidate would leave the global assertion (§4.4) to catch it + with a message that names versions rather than the actual cause, so it fails here + with the diagnosis instead. + +A `patch` preview inherits the §4.0 refusal for the same reason a stable patch +does: if a higher core is already previewed, a lower-core prerelease cannot outrank +it. + +**Post-condition, asserted in code:** the returned candidate strictly outranks the +incumbent. With step 3 this holds by construction; asserting it turns a future +algorithm edit into a test failure rather than a bad publish. + +### 4.4 Validation, not bumping + +The candidate is checked against the **global** floor before being returned: + +```ts +// Must outrank everything published by any route. An ASSERTION, not an input to the +// computation — mixing channels at computation time is what produces §4.1's +// failures. +if (compareVersions(candidate, globalFloor) <= 0) throw new Error(...); +``` + +Because §4.0 refuses the cases that would fail it, this should never fire in normal +use. It is a backstop: if it fires, the resolver and the invariant disagree, and the +release must stop rather than proceed on a version the repository will reject two +steps later. + +## 5. What survives untouched + +`assertUnusedReleaseVersion` (`:372-391`) and `assertChannelVersionMovesForward` +(`:342-370`) both take the version as an argument and never read `package.json`. +Both survive unchanged and run against the resolved `X`. The branch gate +(`:494-511`) and the bump/commit/push block (`:559-591`) are untouched. + +## 6. Resolver cases — `tests/version-line.test.ts` + +Each case discriminates a specific wrong implementation. + +| Case | Fixture | Expected | Kills | +|---|---|---|---| +| future preview does not raise a stable bump | `latest=2.42.0`, preview tag `v2.43.0-preview.1`, `minor` | `2.43.0` | §4.1 failure 1 | +| **patch refused above an open preview** | `latest=2.42.0`, preview tag `v2.43.0-preview.1`, `patch` | **throws**, message names the preview | §4.0; an implementation returning `2.42.1` | +| patch allowed with no higher preview | `latest=2.42.0`, no preview tags above `2.42.0`, `patch` | `2.42.1` | over-broad refusal | +| preview after a stable | `latest=2.42.0`, no preview tags on 2.43.0, `minor` | `2.43.0-preview.` | §4.1 failure 2 | +| same-core preview ordinal | as above, tag `v2.43.0-preview.20260904`, same stamp | `2.43.0-preview.20260904.2` | the fixed point | +| **preview kind is honoured** | `latest=2.42.0`, `major` | `3.0.0-preview.` | dropping `kind` — every kind returning 2.43.0 | +| **ordinal continues from the incumbent** | tag `v2.43.0-preview.20260904.3` exists, same stamp | `2.43.0-preview.20260904.4` | a hard-coded `.2` | +| **preview tip ahead, stamp EQUAL to it** | `previewTip=2.43.0-preview.20260910` (no matching tag), no preview tags on the core, `stamp=20260910` | `2.43.0-preview.20260910.2` | reading tags only — a tags-only build sees no incumbent and returns the bare stamp | +| **preview tags ahead, stamp EQUAL to them** | `previewTip=2.40.0-preview.20260902`, tag `v2.43.0-preview.20260910` on the core, `stamp=20260910` | `2.43.0-preview.20260910.2` | reading the npm tip only — a tip-only build sees no same-core incumbent and returns the bare stamp | +| **clock regression is refused** | incumbent stamp `20260910`, supplied `stamp=20260904` | **throws**, message names both stamps | silently returning a behind candidate | +| stable floor from tags, not the preview channel | `previewTip=2.40.0-preview.20260902`, stable tags to `v2.42.0` | core is `2.43.0`, never `2.41.*` | channel-only base | +| preview-to-stable promotion | `latest=2.42.0`, tag `v2.43.0-preview.20260904`, stable `minor` | `2.43.0` | treating the preview as consumed | +| monotonicity post-condition | any preview input with an incumbent | `compareVersions(result, incumbent) > 0` | silent no-op successors | + +**Rows 8 and 9 are the pair that force both sources to be read, and their stamps are +pinned deliberately.** An earlier version of these rows left the stamp unspecified, +so a tags-only implementation could pass the "tip ahead" row purely because the test +stamp happened to be newer than the tip — the assertion would hold for the wrong +reason. Fixing the supplied stamp **equal** to the incumbent's removes that escape: +the expected value (`...2`) is reachable only by an implementation that actually +found the incumbent in that row's source. An older stamp would work equally well; +equal is used because it also exercises the ordinal path. + +Row 11 is today's live state (`000_research.md` §11.3). + +## 7. CLI cases — `tests/release-helper.test.ts` + +1. `--bump minor` with `npmLatest: "9.9.9"` runs `npm version 9.10.0 + --no-git-tag-version` and dispatches `version=9.10.0`. +2. `--bump` plus an explicit version is rejected before any command runs. +3. An invalid `--bump` kind is rejected before any command runs. +4. **The tag set is consulted, not only the channel:** `npmLatest: "9.9.0"` with a + `v9.9.5` tag, **`--bump patch`** must yield `9.9.6`. + + `patch` is deliberate. With `minor` both bases yield `9.10.0`, so the assertion + would pass against an implementation that never read the tags. `patch` + discriminates: channel-only gives `9.9.1`, tag-aware gives `9.9.6`. +5. `--bump` on `preview` produces a string matching + `^\d+\.\d+\.\d+-preview\.\d{8}(\.\d+)?$`. +6. **The §4.0 refusal reaches the operator:** `--bump patch` with a higher-core + preview tag exits non-zero, prints the explanatory message, and logs no + `npm version` or `git commit` call. + +The fixture already shims `git` (`tests/release-helper.test.ts:100-120`); cases 4, +5 and 6 need a `tag --list` response added to it — a fixture extension, not a new +harness. + +## 8. IN / OUT + +IN: argument parsing in `scripts/release.ts`, the two resolvers in +`scripts/version-line.ts`, their tests, the contributing docs (including the §4.0 +consequence, which is operator-visible policy). + +OUT: every workflow file; the bump/commit/push block; the dispatch shape; anything +in `030`; any change to `tests/release-version-line.test.ts`. + +## 9. Accept criteria + +1. `bun test tests/version-line.test.ts` green, with all eleven §6 rows. +2. `bun test tests/release-helper.test.ts` green, with the six §7 cases. +3. `bun run typecheck`. +4. `bun run privacy:scan` — this phase edits the file holding the SSH-target + assembly (`scripts/release.ts:154-219`), whose comments record that a literal + remote reads as an email address to the scanner. +5. Manual: `bun scripts/release.ts --bump minor` on a non-release branch aborts at + the branch gate (`:511`) before any network call. Not automated; no existing test + covers "aborts before a network call on a wrong branch". + +All five commands exist and read the changed files directly. Verified to exist and +be correctly targeted, not verified to pass — no code exists yet. + +## 10. Activation grounding + +| Path | Trigger | Observable | +|---|---|---| +| stable resolver | §7 case 1 | `npm version 9.10.0`, `version=9.10.0` dispatched | +| tag floor consulted | §7 case 4 | `9.9.6`, not `9.9.1` | +| §4.0 patch refusal | §6 row 2, §7 case 6 | throw/exit naming the blocking preview tag | +| patch still allowed otherwise | §6 row 3 | `2.42.1` | +| preview kind honoured | §6 row 6 | `3.0.0-preview.*` for `major` | +| npm tip vs tags | §6 rows 7-8 | candidate outranks whichever source is ahead | +| ordinal disambiguation | §6 row 5 | `...20260904.2` | +| both-forms rejection | §7 case 2 | non-zero exit, empty call log | +| invalid kind rejection | §7 case 3 | non-zero exit, empty call log | +| global-floor backstop | candidate below floor | throw naming both versions | diff --git a/devlog/_plan/260904_release_version_line/030_phase3_premove.md b/devlog/_plan/260904_release_version_line/030_phase3_premove.md new file mode 100644 index 0000000000..fcaf2ae979 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/030_phase3_premove.md @@ -0,0 +1,473 @@ +# 030 — Phase 3: move `dev` BEFORE the release, not after + +`.github/workflows/dev-version-bump.yml` stops being a **repairer** that reacts to a +publish and becomes an **opener** that runs before one. Same script, same rule, same +reviewed pull request into `dev` — different moment. A gate in `release.yml` refuses +to publish when it has not run. + +Depends on: `010`. Independent of `020` — either may land first. + +## 1. The change + +``` +today: publish vX -> dev is RED -> open PR -> review -> merge -> green +after: open PR -> review -> merge -> promote -> publish vX -> never red +``` + +The number of `dev` commits does not grow: a pre-move is needed only when the +release would otherwise leave `dev` at or behind the new tag (`001_design.md` §1). +A preview cut, or a stable hotfix below `dev`'s line, needs none — +`decideDevVersion` returns `changed: false` and no pull request is opened +(`scripts/bump-dev-version.ts:120-126`). + +What disappears is the interval during which `dev` and every open pull request carry +a failure no contributor can fix. + +## 2. File change map + +| Path | Action | +|---|---| +| `.github/workflows/dev-version-bump.yml` | MODIFY — trigger, input normalization, freeness check | +| `.github/workflows/release.yml` | MODIFY — delete the post-publish call; add the readiness gate (§5) and the ordering gate (§5a) | +| `scripts/version-line.ts` | MODIFY — add an `import.meta.main` CLI: `assert-ahead` (§6) and `assert-releasable` (§5a) | +| `tests/bump-dev-version.test.ts` | MODIFY — intended-version cases | +| `tests/ci-workflows.test.ts` | MODIFY — trigger, routing, and both gate assertions | +| `tests/version-line.test.ts` | MODIFY — `assert-releasable` ordering cases (§10 criterion 6) | + +`scripts/release.ts` is **not** in this map and needs no change: the readiness gate +reads state the dispatch already carries, and no new dispatch input is introduced. + +## 3. Trigger, and the one normalized input + +The workflow today accepts `released-version` via `workflow_call` +(`dev-version-bump.yml:39-46`) and its decision step reads exactly that at line 86. + +`workflow_call` is **removed**: §5 deletes its only caller, and a reusable-workflow +entry point with no caller is dead configuration. Its capability — repairing a +release that published without a pre-move — survives as an explicit `mode`, which is +reachable and testable rather than dependent on another workflow remembering to call +it. + +```yaml +on: + workflow_dispatch: + inputs: + intended-version: + description: "Version about to be released (pre-move), or one already published (repair)" + required: true + type: string + mode: + description: "pre-move (default) or repair — repair allows an already-published version" + required: false + default: pre-move + type: choice + options: [pre-move, repair] +``` + +Even with one event the value is still **normalized into one output before the +decision step**, and a test asserts the routing. That is not ceremony: the +decision step, the freeness check and the PR body are three consumers, and having +them read the raw input independently is how a renamed or added input silently +reaches only some of them — the defect this unit already hit once. + +```yaml +jobs: + open-bump-pr: + steps: + # ... checkout, bun setup, install ... + + - name: Refuse a dispatch from a non-default ref + run: | + # A dispatched run executes the SELECTED ref's body. Pin it to the default + # branch so a feature branch cannot run its own version of this job with + # contents: write (dev-version-bump.yml:35-38). + test "$GITHUB_REF" = "refs/heads/${{ github.event.repository.default_branch }}" || { + echo "::error::this workflow may only be dispatched from the default branch" + exit 1 + } + + # ONE value downstream. Both events terminate here; every later step reads + # steps.target.outputs.version and nothing else. Without this the dispatch path + # would reach bump-dev-version.ts with an empty argument and open no pre-move. + - name: Resolve the target version + id: target + env: + INTENDED: ${{ inputs.intended-version }} + MODE: ${{ inputs.mode }} + run: | + set -euo pipefail + target="${INTENDED:-}" + if [ -z "$target" ]; then + echo "::error::intended-version was not supplied" + exit 1 + fi + echo "version=${target}" >> "$GITHUB_OUTPUT" + # Explicit if, not "${MODE:+x}${MODE:-y}": that form concatenates to + # "x" when MODE is populated, because the second expansion falls + # back to MODE's own value rather than to nothing. + if [ "${MODE:-pre-move}" = "repair" ]; then + echo "mode=repair" >> "$GITHUB_OUTPUT" + else + echo "mode=pre-move" >> "$GITHUB_OUTPUT" + fi +``` + +The decision step then reads the normalized value instead of the raw input: + +```diff + - name: Decide the version dev should carry + id: decide + env: +- RELEASED_VERSION: ${{ inputs.released-version }} ++ RELEASED_VERSION: ${{ steps.target.outputs.version }} + run: | + set -euo pipefail + bun scripts/bump-dev-version.ts "${RELEASED_VERSION}" package.json +``` + +Both remaining consumers — the freeness check (§4) and the pull-request body +(`dev-version-bump.yml:103-187`) — take the same normalized value, so the generated +PR names the version that was actually dispatched. + +The §4 freeness assertion applies to `mode: pre-move` only. `mode: repair` +deliberately permits an already-published version, which is exactly the old catch-up +behaviour, retained for the case where a release somehow publishes without a +pre-move. + +## 3a. The generated copy must match the mode + +The commit subject and pull-request body are written for the catch-up world and say +so: `fix(release): move dev to ${NEXT_VERSION} after ${RELEASED_VERSION}` +(`dev-version-bump.yml:155,162`), and a body asserting that +"`${RELEASED_VERSION}` published, so `dev` would otherwise keep a version at or +behind a released one" (lines 166-169). + +In pre-move mode every one of those statements is false: nothing has published, and +`dev` is not behind anything. Shipping that text would make the pull request argue +for itself with a reason the reviewer can see is untrue — which is how a reviewer +learns to skim these. + +```yaml + if [ "${MODE}" = "repair" ]; then + subject="fix(release): move dev to ${NEXT_VERSION} after ${TARGET_VERSION}" + reason="\`${TARGET_VERSION}\` has published, so \`dev\` is carrying a version at or behind a released one and \`tests/release-version-line.test.ts\` fails on \`dev\` and on every pull request opened against it. This is the post-publish repair." + else + subject="chore(release): open dev at ${NEXT_VERSION} before releasing ${TARGET_VERSION}" + reason="\`${TARGET_VERSION}\` is about to be released. Merging this first means \`dev\` already outranks the new tag when it lands, so neither \`dev\` nor any open pull request ever inherits the version-line failure. \`release.yml\` refuses to publish until this has merged." + fi +``` + +The Verification and Checklist sections (lines 175-186) are mode-independent and +unchanged. The freeness evidence differs — pre-move proves the target is *not yet* +published (§4), repair proves the chosen version is unused — so that one sentence +follows `mode` too. + +## 4. Freeness, retargeted + +`decideDevVersion(released, current)` (`scripts/bump-dev-version.ts:101-142`) asks +"given that `released` exists, what should `dev` carry?" The pre-move asks the same +question about a version that has not published yet. The rule is unchanged — +`nextDevelopmentVersion` keys off the version's *shape* +(`scripts/bump-dev-version.ts:38-42`), not its published-ness. + +What must change is the freeness gate. `dev-version-bump.yml:94-101` runs +`tests/release-version-line.test.ts`, which compares against the local tag set; in a +pre-move the release tag does not exist yet, so it proves less than it does today. + +```yaml + - name: Prove the intended version is not already released + if: ${{ steps.target.outputs.mode == 'pre-move' }} + env: + INTENDED: ${{ steps.target.outputs.version }} + run: | + set -euo pipefail + git fetch --force --tags origin + if git rev-parse -q --verify "refs/tags/v${INTENDED#v}" >/dev/null; then + echo "::error::v${INTENDED#v} already exists; this is a catch-up, not a pre-move" + exit 1 + fi + if npm view "@bitkyc08/opencodex@${INTENDED#v}" version >/dev/null 2>&1; then + echo "::error::${INTENDED#v} is already on npm" + exit 1 + fi +``` + +A pre-move whose target already exists is a catch-up wearing the wrong name and must +fail loudly. `${INTENDED#v}` strips an optional `v` so both spellings work, matching +`asTag`'s tolerance in the script (`scripts/bump-dev-version.ts:68-70`). + +## 5. Readiness gate, replacing the post-publish call + +`release.yml:39-80` currently calls the bump workflow after publishing. That job and +its 28-line comment are deleted, along with the `permissions` block at lines 75-77 +that existed only for it. In its place, a pre-flight assertion in the `publish` job: + +```yaml + - name: Require dev to be ready for this release + env: + RELEASE_VERSION: ${{ inputs.version }} + run: | + set -euo pipefail + git fetch origin dev --tags + dev_version="$(git show origin/dev:package.json | bun -e 'console.log(JSON.parse(await Bun.stdin.text()).version)')" + # dev must ALREADY outrank the version about to be tagged, or publishing + # opens the inherited-red window this design exists to close. + bun scripts/version-line.ts assert-ahead "$dev_version" "$RELEASE_VERSION" +``` + +## 6. The invocation, specified + +An earlier draft left this open with a `scripts/version-line.js` path that does not +exist. It is settled here: **`bun scripts/version-line.ts `**, using the +`import.meta.main` guard pattern the repository already relies on +(`scripts/bump-dev-version.ts:144`, and `scripts/release-notes.ts`, whose CLI is +guarded exactly so a test can import the module without executing it — +`tests/release-version-line.test.ts:27-29`). + +Two subcommands are needed, one per gate: `assert-ahead` for the readiness gate (§5) +and `assert-releasable` for the ordering gate (§5a). Both are thin wrappers over +exported pure functions, so the policy is unit-testable without a subprocess and the +CLI is testable for the wiring the pure function cannot cover. + +```ts +/** + * The ordering policy enforced at the publication boundary: a candidate must + * strictly outrank every release tag. + * + * dryRunTagSha/headSha preserve release.yml:311-313's deliberate exception — a dry + * run whose tag already points at THIS commit is a legitimate re-run, not a + * regression. Without it this gate would break every post-release dry run. + * + * Pure: returns the offending tag rather than exiting, so a test can assert the + * policy and the caller decides what a violation means. + */ +export function assertReleasable(input: { + candidate: string; + tags: readonly string[]; + /** True when this tag already names the commit under release and it is a dry run. */ + allowExistingTagAtHead?: boolean; +}): { ok: true } | { ok: false; blockedBy: string }; + +// Kept behind import.meta.main so importing this module from a test never executes a +// CLI, which is the property that made release-notes.ts importable and release.ts not +// (tests/release-version-line.test.ts:27-29). +if (import.meta.main) { + const [command, ...rest] = process.argv.slice(2); + + if (command === "assert-ahead") { + const [left, right] = rest; + if (compareVersions(left!, right!) <= 0) { + console.error(`::error::origin/dev carries ${left}, which does not outrank ${right}. Run the dev pre-move before releasing.`); + process.exit(1); + } + process.exit(0); + } + + if (command === "assert-releasable") { + const [candidate, ...flags] = rest; + // Tag set on stdin: §5a pipes `git tag --list 'v*'` in. Reading it here rather + // than spawning git keeps this module free of process spawning, matching how + // release.yml:256-259 already pipes the tag list into scripts/release-notes.ts. + const tags = (await Bun.stdin.text()) + .split("\n").map(line => line.trim()).filter(Boolean); + const verdict = assertReleasable({ + candidate: candidate!, + tags, + allowExistingTagAtHead: flags.includes("--allow-existing-tag-at-head"), + }); + if (!verdict.ok) { + console.error(`::error::${candidate} does not outrank the current tag set (blocked by ${verdict.blockedBy}). Opening a preview for a higher core closes older stable patch lines — see devlog/_plan/260904_release_version_line/020 §4.0.`); + process.exit(1); + } + process.exit(0); + } + + console.error("usage: bun scripts/version-line.ts assert-ahead | assert-releasable [--allow-existing-tag-at-head]"); + process.exit(1); +} +``` + +An earlier draft of this section specified only `assert-ahead` while §5a already +invoked `assert-releasable`. Implemented literally, every command but the first +would have fallen through to the usage error and **exit 1 — blocking every dry run +and every publish**, the exact inverse of the gate's purpose. Both subcommands are +specified here for that reason, and criterion 8 tests the CLI's stdin and exit +behaviour rather than only the pure function, because the pure function alone would +not have caught it. + +Bun is already installed in this job by `./.github/actions/setup-project-bun` +(`release.yml:143-144`), and the workflow already runs `bun` directly +(`bun scripts/build-release-changelog.ts`, `release.yml:346`), so this adds no new +runtime dependency. Adding the CLI to `scripts/version-line.ts` is why that file +appears in this phase's change map. + +## 5a. Enforcing the closed-patch policy at the publication boundary + +`020` §4.0 refuses a stable patch bump when a higher-core preview exists, but that +refusal lives in `nextStableRelease` and only fires when a maintainer uses +`--bump`. **It is bypassable, and not hypothetically:** + +1. a stable patch commit gets green exact-head CI **before** any higher-core preview + tag exists; +2. the preview publishes, creating `vX.(Y+1).0-preview.*`; +3. a maintainer dispatches Release manually for that already-green stable SHA. + +`nextStableRelease` never runs. `release.yml` refreshes tags during preflight +(`release.yml:303`, `git fetch --force --tags origin`) but the checks that follow +only test **duplicate** metadata — tag exists, GitHub release exists, npm version +exists (`:305-336`). Nothing tests current **ordering**. So the exact state §4.0 +promises to refuse can still publish. + +The gate therefore belongs after that fetch, in the same step or immediately after +it, using the shared strict comparator: + +```yaml + - name: Refuse a release the current tag set already outranks + env: + RELEASE_VERSION: ${{ inputs.version }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + set -euo pipefail + # Runs AFTER the preflight tag fetch, so it sees tags created since this + # commit's CI run. The resolver in scripts/version-line.ts enforces the same + # policy, but only when --bump is used; a manual dispatch of an + # already-green SHA bypasses it entirely. This is the enforcement point. + # + # The --allow-existing-tag-at-head flag preserves release.yml:311-313's + # deliberate dry-run exception: re-running a dry run for an already-tagged + # commit is legitimate, and a strict "outranks every tag" test would reject + # it because the candidate EQUALS its own tag. Only granted when the tag + # names this exact commit, matching the existing check's condition. + allow="" + existing_tag_sha="$(git rev-parse -q --verify "refs/tags/v${RELEASE_VERSION}^{commit}" || true)" + if [ "$DRY_RUN" = "true" ] && [ -n "$existing_tag_sha" ] && [ "$existing_tag_sha" = "$GITHUB_SHA" ]; then + allow="--allow-existing-tag-at-head" + fi + git tag --list 'v*' | bun scripts/version-line.ts assert-releasable "$RELEASE_VERSION" $allow +``` + +`assert-releasable` reads the tag set on stdin and refuses when the candidate does +not strictly outrank every existing tag — the same question +`tests/release-version-line.test.ts` asks of the tree, asked here of the version +about to be published, at the last moment before it becomes irreversible. + +**The one exception is inherited, not invented.** `release.yml:311-313` already +permits a dry run when the release tag exists **and points at this exact commit**, +treating it as a legitimate re-run rather than a duplicate. A strict +"outranks every tag" rule contradicts that, because such a candidate necessarily +*equals* its own tag. The gate therefore carries the same condition rather than +silently removing a deliberate affordance — this preserves the existing behaviour; +it does not extend it. Real publishes are unaffected: `dry_run != true` means the +flag is never granted, and `release.yml:314-317` still refuses outright. + +Reading tags from stdin rather than shelling out from inside the script keeps the +module free of process spawning and matches how `release.yml:256-259` already pipes +`git tag --list` into `scripts/release-notes.ts`. Precedent, not invention. + +**Placement matters.** It must come after `release.yml:303`'s fetch — before it, the +runner's tag set is whatever the checkout brought and the gate would be checking +stale data, which is the same class of bug as the CI-green-before-preview sequence +it exists to catch. + +This gate subsumes the `020` §4.4 global-floor assertion for stable releases: that +one runs at resolution time on a maintainer's machine, this one at publication time +on the audited SHA. Keep both — they answer the same question at different moments, +and only the second is on the path a manual dispatch takes. + +## 7. Why the gate is safe in the publish job + +It reads `origin/dev` and compares two strings. It grants no permission, mutates +nothing, and fails closed. Placed with the other pre-publish gates +(`release.yml:188-283`), before the preflight metadata step. + +It asserts a version relationship and nothing more. It does **not** assert or imply +any ancestry between the release commit and `dev` — under this ordering the release +commit is created on `main` after promotion, so it is a descendant of the promoted +state and never an ancestor of it (`001_design.md` §0). + +## 8. Honest limitation of the ref guard + +The §3 dispatch check runs *inside* the already-selected body, so a malicious branch +could delete it. Tier E2 (workflow-internal), executing surface: the job itself, +known bypass: edit the step out on the dispatched branch, residual: accepted because +pushing such a branch requires repository write and the release branches are +protected. It is an **early warning against maintainer error**, not enforcement. + +## 9. IN / OUT + +IN: the workflow trigger and input normalization, the freeness assertion, the +readiness gate, the `version-line.ts` CLI, matching tests. + +OUT: `scripts/release.ts`; the publish/pack path; the equality check at +`release.yml:175-184`, which stays exactly as it is; any deletion of +`bump-dev-version.ts` or its test; the `020` resolvers. + +## 10. Accept criteria + +1. `bun test tests/ci-workflows.test.ts` green with: the dispatch ref guard present; + the `bump-dev-version` job absent from `release.yml`; the readiness step present + in `publish`; and **the routing assertion** — the decision step, the freeness + step and the PR body all read `steps.target.outputs.version`, and no step reads + `inputs.intended-version` directly except the resolver. +2. `bun test tests/bump-dev-version.test.ts` green with the intended-version cases. +3. `bun run typecheck`. +4. A dispatched pre-move against a real intended version opens a PR whose only + changed file is `package.json` and whose title names that version — the existing + branch-content check (`dev-version-bump.yml:139-143`) is unchanged and still + applies. +5. A dispatched pre-move whose target already has a tag fails at §4's assertion. +6. **The bypass sequence is covered.** `tests/version-line.test.ts` drives + `assert-releasable` through the §5a scenario as data — tag set + `[v2.42.0, v2.43.0-preview.1]` with candidate `2.42.1` must be refused, while the + same candidate against `[v2.42.0]` alone is allowed. That is the + CI-green-before-preview / dispatch-after-preview case reduced to the two inputs + that actually decide it. +7. `tests/ci-workflows.test.ts` asserts the §5a step exists **and sits after** the + preflight `git fetch --force --tags origin` (`release.yml:303`). Position is the + whole point: before the fetch it would read a stale tag set. Asserted by index + comparison, the same technique the file already uses for step ordering + (`tests/ci-workflows.test.ts:788-795`). +8. **The CLI is tested, not only the pure function.** `tests/version-line.test.ts` + spawns `bun scripts/version-line.ts assert-releasable ` with a tag list + on stdin and asserts exit 0 / non-zero, plus the same for `assert-ahead`, plus + that an **unknown subcommand exits non-zero with the usage line**. A pure-function + test cannot catch a missing CLI branch — that omission is exactly what round 5 + found, where §5a invoked a subcommand §6 never implemented and every release would + have been blocked. +9. **The dry-run exception survives.** `assertReleasable` with + `allowExistingTagAtHead: true` accepts a candidate equal to an existing tag, and + rejects it without the flag. Pinning both directions keeps a future simplification + from quietly breaking post-release dry runs. + +Criteria 4 and 5 need a real dispatch. 5 is cheap and safe: dispatch with an +already-released version such as `2.42.0` and confirm the refusal. + +Criteria 6 and 7 are the ones that make `020` §4.0 a policy rather than a +suggestion, and neither needs a dispatch: one is a pure-function test, the other a +workflow-text assertion. + +Criterion 1's routing assertion is the specific guard against this phase's failure +mode — an input declared in `on:` that nothing downstream reads. + +## 11. Activation grounding + +| Path | Trigger | Observable | +|---|---|---| +| input normalization, default | dispatch with `intended-version`, no mode | `steps.target.outputs.version` equals it; `mode=pre-move` | +| input normalization, repair | dispatch with `mode: repair` | same output; `mode=repair`; freeness check skipped | +| version missing | malformed invocation | `intended-version was not supplied` | +| dispatch ref guard | dispatch from a non-default branch | `may only be dispatched from the default branch` | +| tag-exists refusal | dispatch `intended-version=2.42.0` | `v2.42.0 already exists; this is a catch-up` | +| npm-exists refusal | same | `already on npm` | +| readiness gate fails | release dispatched while `dev` trails | `origin/dev carries X, which does not outrank Y` | +| readiness gate passes | release after a merged pre-move | step succeeds, publish proceeds | +| ordering gate refuses | candidate `2.42.1` with `v2.43.0-preview.1` in the tag set | non-zero exit naming the outranking tag | +| ordering gate passes | same candidate, no higher-core preview | step succeeds | +| dry-run re-run allowed | dry run, tag exists at this SHA | flag granted, step succeeds | +| same state, real publish | `dry-run=false`, tag exists at this SHA | flag withheld; `release.yml:314-317` refuses | +| unknown subcommand | `bun scripts/version-line.ts nonsense` | non-zero exit, usage line | +| no-op pre-move | dispatch when `dev` already outranks | `changed=false`, no PR opened | + +Row 7 must be shown firing: it converts the pre-move from a habit into a gate. +Exercising it means dispatching a release before the pre-move merges — safe under +`dry-run: true`, the workflow's default (`release.yml:22-26`). diff --git a/devlog/_plan/260904_release_version_line/040_phase4_invariant_and_docs.md b/devlog/_plan/260904_release_version_line/040_phase4_invariant_and_docs.md new file mode 100644 index 0000000000..64a2c32725 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/040_phase4_invariant_and_docs.md @@ -0,0 +1,132 @@ +# 040 — Phase 4: documentation, and one retained invariant + +The smallest phase, and documentation-only in effect. It corrects the release policy +that currently instructs maintainers to do the chore in the wrong order, syncs the +architecture SoT, and records why the invariant's equality exception is retained +rather than removed. + +Depends on: `020` **and** `030`, both landed, with `030` exercised by one release. +`020` is required because §4a documents the patch-refusal policy that `020` +implements; documenting a rule the code does not yet enforce would be worse than +documenting nothing. + +## 1. File change map + +| Path | Action | +|---|---| +| `tests/release-version-line.test.ts` | MODIFY — header comment only; **no assertion changes** | +| `MAINTAINERS.md:76-90` | MODIFY — ordering correction | +| `structure/06_docs-and-release.md` | MODIFY — SoT sync | + +**No deletions, and no ancestry test.** An earlier draft proposed asserting that +every release tag is an ancestor of `dev`. That is withdrawn: it is false on today's +repository (10 of 226 tags are not ancestors, all previews), it cannot hold under the +pre-move ordering, and nothing depends on it. `001_design.md` §0 states the contract. + +## 2. The invariant keeps its exception + +`tests/release-version-line.test.ts` is correct as written. Its three outcomes — +ahead, equal-on-the-tagged-commit, behind — remain right, and `tagPointsAtHead` +(lines 68-81) is retained: the release commit still equals its own tag. + +No new comparator case is added. An earlier draft proposed asserting +`compareReleaseTags("v2.42.0", "v2.42.0") === 0`, which is tautological: it exercises +the comparator, not `tagPointsAtHead`, and would pass against a build that had +deleted the exception entirely. + +What actually exercises the exception is acceptance criterion 1 (§6): running the +invariant on a checkout of the newest tag, where `ordering === 0` and the test passes +**only** because `tagPointsAtHead` returns true. That path already exists and needs +no new code. + +The change here is therefore documentation-only: the file header (lines 8-29) gains +one sentence recording that the repair moved from after the release to before it, so +a future reader does not reconstruct the catch-up as the intended design. The +assertions are untouched. + +## 3. `MAINTAINERS.md` + +Lines 76-90 currently open "**Closing out a release includes moving `dev`'s version +line forward.**" That instruction is the cause of the recurrence: done at closing +time, it is always too late. + +```diff +-- **Closing out a release includes moving `dev`'s version line forward.** A published +- release leaves `dev` carrying a version at or behind it ... ++- **Opening a release starts by moving `dev`'s version line forward.** Before cutting ++ a release, `dev` must already outrank the version being released; `release.yml` ++ asserts this and refuses to publish otherwise. Dispatch ++ `.github/workflows/dev-version-bump.yml` with the intended version, merge the pull ++ request it opens, then promote and release. When `dev` already outranks the target ++ — a preview cut, or a stable hotfix below `dev`'s line — no move is needed and the ++ workflow reports `changed=false`. ++ ++ Done AFTER the publish, as this repository did for ten releases (`32529c2b2`, ++ `e4a85d134`, `076ad3036`, `befcac3e1`, then #3045, #3076, #3127, #3265, #3354, ++ #3434), it leaves `dev` and every open pull request carrying a failure ++ contributors cannot fix from their own diff. The pull request itself does not go ++ away — `Protect dev` requires a reviewed merge. Design: ++ `devlog/_plan/260904_release_version_line/`. +``` + +Note what this does **not** claim: nothing about ancestry, and not "one PR per +release". The conditional phrasing matches `decideDevVersion`'s actual no-op +behaviour (`scripts/bump-dev-version.ts:120-126`). + +## 4. `structure/06_docs-and-release.md` + +Lines 181, 240 and 253 describe the release path. They get the same ordering +correction and a pointer to this unit. Per `AGENTS.md`, the unit moves to +`devlog/_fin/` when the work closes — it is a design record of shipped work at that +point and contains no security material. + +## 4a. The patch-line consequence must be documented + +`020` §4.0 chose global cross-channel ordering, which means **publishing a preview +for a higher core closes the older stable patch line**: once `v2.43.0-preview.1` +exists, `2.42.1` ranks below the highest tag and cannot be released. + +That is operator-visible policy, not an implementation detail, and it is surprising +enough that discovering it from a refusal message would be a bad experience. Both +`MAINTAINERS.md` and `structure/06_docs-and-release.md` state it plainly: + +> Opening a preview for the next core ends the current patch line. After +> `vX.Y.0-preview.*` is tagged, a fix ships as part of `X.Y.0`, not as +> `X.(Y-1).(Z+1)`. The release helper refuses such a bump rather than producing a +> version the repository would reject. + +`020` documents the same consequence in `docs-site` for contributors; this phase +covers the maintainer-facing files. + +## 5. IN / OUT + +IN: the test file's header comment, and the two documentation files. + +OUT: any code change; any assertion change; any deletion; any ancestry assertion; +the workflow (`030`). + +## 6. Accept criteria + +1. `bun test tests/release-version-line.test.ts` green, including on a checkout of + the newest tag — via the retained exception. +2. `bun run typecheck`. +3. `rg -n 'Closing out a release includes moving' MAINTAINERS.md` returns nothing. +4. `rg -n 'ends the current patch line' MAINTAINERS.md structure/06_docs-and-release.md` + finds the §4a wording in both files. + +The repository-wide suite is not warranted: this phase deletes nothing and imports +nothing new. `AGENTS.md` still requires it before the PR is marked review-ready, +which is a separate gate from this phase's acceptance. + +Criterion 1 is the phase's real gate and the only thing that exercises +`tagPointsAtHead`: on a tagged checkout `ordering === 0`, and the test passes only +because the exception returns true. + +## 7. Activation grounding + +| Path | Trigger | Observable | +|---|---|---| +| equality on the tagged commit | checkout `v2.42.0`, run the invariant | passes via `tagPointsAtHead` | +| equality off the tagged commit | `dev` at a published version | fails with the existing message | + +No conditional code is added, so there is nothing further to activate. diff --git a/devlog/_plan/260904_release_version_line/050_migration.md b/devlog/_plan/260904_release_version_line/050_migration.md new file mode 100644 index 0000000000..c2b5a4ff20 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/050_migration.md @@ -0,0 +1,108 @@ +# 050 — Migration from today's real state + +Not a phase; a record of exactly what the first release under the new ordering does, +from the state verified on 2026-09-04. + +## 1. Starting state + +``` +dev 2.43.0 25 commits ahead of main; main IS an ancestor +main 2.42.0 tag v2.42.0 -> 48f818664 +preview 2.43.0-preview.20260904 no v2.43.0-preview.* tag exists +npm latest=2.42.0 preview=2.40.0-preview.20260902 +``` + +`dev` at `2.43.0` outranks every tag, so the repository is currently green and needs +no preparatory commit. + +## 2. The ordering + +The pre-move must put `dev` **ahead of the version being released**, which means it +targets `N(X)`, not `X`. Releasing `2.43.0`: + +``` +1. decide X 2.43.0 +2. pre-move dev to N(X) 2.43.0 -> 2.44.0 [the one PR] +3. promote dev -> main main receives 2.44.0 +4. release X from main release.ts sets main's package.json to 2.43.0 +5. tag v2.43.0 published dev already at 2.44.0; never red +``` + +Step 4 lowers `package.json` on `main` from `2.44.0` to `2.43.0`. That is unusual +enough to have been flagged as a risk in an earlier draft; it is now **verified +safe**: + +- `npm version 2.43.0 --no-git-tag-version` against a tree at `2.44.0` exits 0 and + writes `2.43.0`. Probed directly on a scratch `package.json`. The + `scripts/release.ts:559-573` bump therefore needs no change and no + `--allow-same-version`-style flag. +- `assertChannelVersionMovesForward` (`:342-370`) compares `X` against the npm + channel tip, not the tree: `2.43.0 > 2.42.0` passes. +- `assertUnusedReleaseVersion` (`:372-391`) checks npm/tag/release for `X`. +- `release.yml:175-184` compares the tree to `X` **after** the bump. +- The invariant on the release commit: `2.43.0` equals the new highest tag on the + commit that tag names — legal via `tagPointsAtHead`. +- On `main` between step 3 and step 4 the tree says `2.44.0` with `v2.42.0` highest + — strictly ahead, legal. + +Every existing gate tolerates the sequence. + +## 3. Releases that need no pre-move + +The pre-move is required only when the release would otherwise leave `dev` at or +behind the new tag. After the above, `dev` carries `2.44.0` and: + +| Release | `dev` outranks it? | Pre-move needed | +|---|---|---| +| `2.43.1` hotfix | `2.44.0 > 2.43.1` ✓ | no | +| `2.44.0-preview.20260910` | `2.44.0 > 2.44.0-preview.*` ✓ | no | +| `2.44.0` stable | `2.44.0 == 2.44.0` ✗ | **yes** -> `2.45.0` | + +`decideDevVersion` already returns `changed: false` for the first two +(`scripts/bump-dev-version.ts:120-126`), so a dispatched pre-move in those cases is a +harmless no-op that opens no pull request. + +## 4. The preview channel + +Preview cuts continue exactly as today: `preview` carries the prerelease it is +publishing, and `release.yml:204-209` enforces the shape. The pre-move is normally +unnecessary for a preview (§3), because `dev`'s stable-shaped version outranks any +prerelease of the same core. + +What `020` changes for previews is only how the *candidate* is computed: from the +stable line plus the preview tag set, never from the stale `preview` dist-tag alone. +Today that tag is `2.40.0-preview.20260902` while stable has reached `v2.42.0`, so a +channel-only computation could propose a `2.41.*` candidate behind a shipped stable. + +## 5. The npm preview gap + +npm `preview` is `2.40.0-preview.20260902`; the branch is at +`2.43.0-preview.20260904`; no `v2.41.0-preview.*` or `v2.42.0-preview.*` tags exist. +Either the last two preview cuts were abandoned mid-train, or previews stopped being +published. I could not determine which from the repository. + +Neither reading breaks this design — §4 holds under both — but a maintainer should +decide it, because it determines whether the preview resolver in `020` is exercised +at all. + +## 6. Rollout order + +``` +PR 1: 010 -> dev behaviour-neutral, safe alone +PR 2: 020 -> dev --bump only; no workflow coupling +PR 3: 030 -> dev pre-move + readiness gate; independent of 020 + promote to main, release under the new ordering +PR 4: 040 -> dev invariant case + docs, after one clean release +``` + +No two phases must land together. The atomicity constraint an earlier draft carried +existed only because of a dispatch input that no longer exists. `040` is the one +phase with two prerequisites: it documents the patch-line policy `020` implements +and the ordering gate `030` enforces, so it lands after both. + +## 7. First release under the gate + +`030`'s readiness gate requires `dev` to strictly outrank `X`. At step 4 above, +`dev` is `2.44.0` and `X` is `2.43.0`, so it passes. If the pre-move has **not** +merged, `dev` is `2.43.0`, the gate refuses, and the remedy is the pre-move itself — +which is the intended behaviour, not a migration obstacle. diff --git a/devlog/_plan/260904_release_version_line/060_rollback_and_failure_modes.md b/devlog/_plan/260904_release_version_line/060_rollback_and_failure_modes.md new file mode 100644 index 0000000000..33264cfb82 --- /dev/null +++ b/devlog/_plan/260904_release_version_line/060_rollback_and_failure_modes.md @@ -0,0 +1,119 @@ +# 060 — Rollback and failure modes + +## 1. Rollback + +The design changes when an existing pull request is opened. It adds no publish-time +mutation, no dispatch input, and deletes no gate. + +| Landed through | To revert | Blast radius | +|---|---|---| +| `010` | revert the PR | none; behaviour-neutral | +| `020` | revert the PR | none; `--bump` is additive, the typed form still works | +| `030` | revert the PR | the workflow returns to post-publish catch-up; the red window returns | +| `040` | revert the PR | one test case and two documents | + +**No phase is irreversible and none strands a published artifact.** The release +commit still carries the published version and the tarball is still packed from the +tree, so a rollback at any point leaves every release, tag and attestation exactly as +it would otherwise have been. + +One asymmetry: reverting `030` after `040` leaves `MAINTAINERS.md` describing a +pre-move that no longer runs. Revert both, or fix the document — a documentation +inconsistency, not a broken release path. + +## 2. Failure modes + +**F1 — The pre-move can be forgotten.** The scheme is an ordering convention. +*Guard:* the readiness gate (`030` §5) refuses to publish when `dev` does not +outrank the release version, converting a forgotten step from silent inherited red +into a blocked release. *Residual:* the gate can be removed, or `dev` moved by hand +— but `dev` is protected, so the manual path is itself a reviewed PR, which is the +pre-move. + +**F2 — The readiness gate can block a release.** See §3; this is the one that will +actually be felt. + +**F3 — Two version-line PRs could race.** The pre-move opens a PR into `dev` while +development continues. *Guard:* existing idempotency (`dev-version-bump.yml:114-157`) +checks for an open PR and validates branch content before reuse; +`concurrency: dev-version-bump` (lines 49-51) serialises runs. *Residual:* low; the +repository releases serially. + +**F4 — The dispatch ref guard is bypassable.** `030` §3's check runs inside the +already-selected workflow body, so a branch could delete it. Tier E2, executing +surface the job itself, known bypass "edit the step out on the dispatched branch", +residual accepted because pushing such a branch needs repository write. Called an +early warning, not enforcement. + +**F5 — The service-lifecycle gate depends on the release commit touching +`package.json`.** `release.yml:268` includes `package.json` in its trigger regex and +the release commit still edits it. Unchanged by this design, recorded because the +dependency is implicit. + +## 3. The readiness gate's real cost + +An earlier draft claimed this gate would block ordinary hotfixes. **That was wrong** +and the correction matters, because it changes whether the gate is acceptable. + +After a compliant release, `dev` carries `2.44.0`. A `2.43.1` hotfix satisfies +`2.44.0 > 2.43.1`, so the gate at `030` §5 **passes without any pre-move**. The same +holds for preview cuts (`050` §3). The gate blocks only when `dev` is *already* in +the state the invariant forbids — i.e. when publishing would create inherited red. + +So the friction is narrower than described: it appears when `dev` has drifted behind, +which is precisely the condition this design exists to prevent. + +**On an override input.** If an override is ever added, it must be understood for +what it is: used when `dev <= X`, it **explicitly reopens the red state** — `dev` and +every open pull request go red the moment the tag lands, exactly as they do today. It +is not a convenience flag. If added, it should log loudly and name the consequence. +I do not recommend adding one until a real release is actually blocked by the gate. + +A silent patch-release exemption is rejected outright: it would skip the check for +the releases most likely to be cut in a hurry. + +## 4. What this design does not introduce + +- no divergence between the tarball and the tagged tree +- no publish-time working-tree mutation +- no new required dispatch input +- no change to compatibility-manifest identity or the GUI bundle version +- no change to what a source checkout of a tag reports +- no ancestry obligation between release tags and `dev` (`001_design.md` §0) + +## 5. Verified facts + +Both were open risks in earlier drafts and are now settled. + +**npm provenance does not bind the tree.** The published attestation for `v2.42.0` +binds the tarball's sha512, the workflow path, and source commit +`48f8186647d9ffb108d226dcfa91a64225aae2a7` as a resolved dependency. It does not +assert tarball/tree byte-equality, and no non-devlog consumer of npm's `gitHead` +exists in `scripts/`, `tests/` or `.github/`. Moot for this design, which creates no +such divergence; recorded because it would have decided the withdrawn stamping +approach. + +**`npm version` accepts a downgrade.** `npm version 2.43.0 --no-git-tag-version` +against a tree at `2.44.0` exits 0 and writes `2.43.0`. Probed directly. This retires +the top implementation risk in `050` §2 — `scripts/release.ts:559-573` needs no +change. + +## 6. What could still make me wrong + +1. **Whether the readiness gate's friction is acceptable in practice** (§3). An + operator judgment, best made after the gate has run for a release or two. +2. **Whether previews are still published at all** (`050` §5). If not, `020`'s + preview resolver is untested-in-anger code solving a problem nobody has. +3. **Same-day preview ordinals** rely on SemVer ordering that the comparator at + `scripts/release.ts:323-335` implements. Unit-tested in `020` §6, never exercised + in a real release, because the repository has never cut two previews in one day. + +## 7. Out of scope + +- Relaxing `Protect dev` (option C). +- Making releases automatic; `release.yml` stays dry-run by default + (`release.yml:22-26`). +- Changing the dist-tag model, the branch layout, or `expected-sha` binding. +- ima2-gen's `assertPreviewProof` (stable tag as a certificate that a preview build + proved the same SHA). A good idea, orthogonal to this unit, and worth its own unit + — folding it in here would make the diff impossible to review as one idea. From d0539a27f8951d881fc1e2c20591422c55451f0a Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 4 Sep 2026 23:07:05 +0900 Subject: [PATCH 4/6] feat(release): extract one shared version algebra into scripts/version-line.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repository ordered releases in two places that could not agree, and only one of them was reachable from a test. `compareReleaseVersions` lives in `scripts/release.ts`, which parses argv and calls `process.exit` at module scope, so importing it from a test kills the runner — it was exercised only through a subprocess fixture. The new module is pure at module level, which is the whole point: it is importable. The two comparators stay deliberately different, and a test now pins that: - `compareVersions` THROWS on unparseable input, because a release decision must fail closed. `release.ts:305-307` records that `Number()` on a garbage core once yielded NaN and made the forward guard accept any candidate. - `compareTagsLenient` falls back to numeric-aware locale compare, exactly as `release-notes.ts` does today. Collapsing the two would be a live regression: `build-release-changelog.ts` admits any `/^v\\d/` tag, so one malformed historical tag would newly abort release-note generation. Both assertions live in one test so the distinction cannot be optimised away later. `nextDevelopmentVersion` moves here from `bump-dev-version.ts`. Its prerelease row is load-bearing rather than an edge case: the rule was once written as "increment the released minor", and befcac3e1 disproves it — a published `X.Y.Z-preview.*` means the stable core has not shipped, so dev should carry `X.Y.Z`, not `X.(Y+1).0`. `tests/bump-dev-version.test.ts` is unchanged and still green. That is the proof the extraction was faithful, and it was this phase's primary gate. Verification (focused only; the repository-wide suite was deliberately not run): bun test tests/version-line.test.ts 7 pass bun test tests/bump-dev-version.test.ts 10 pass, file unchanged bun test tests/release-notes.test.ts 71 pass bun test tests/release-version-line.test.ts 3 pass bun test tests/release-helper.test.ts 33 pass bun run typecheck exit 0 Both new assertions were proven red before the implementation by mutation: the collapsed comparator threw where the lenient one must not, and the prerelease row returned 2.37.0 instead of 2.36.0. Design: devlog/_plan/260904_release_version_line/010_phase1_version_algebra.md --- scripts/bump-dev-version.ts | 5 +-- scripts/release-notes.ts | 56 +----------------------- scripts/release.ts | 39 +---------------- scripts/version-line.ts | 85 +++++++++++++++++++++++++++++++++++++ tests/version-line.test.ts | 68 +++++++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 94 deletions(-) create mode 100644 scripts/version-line.ts create mode 100644 tests/version-line.test.ts diff --git a/scripts/bump-dev-version.ts b/scripts/bump-dev-version.ts index e57d02e032..552fe83300 100644 --- a/scripts/bump-dev-version.ts +++ b/scripts/bump-dev-version.ts @@ -55,6 +55,7 @@ import { existsSync, renameSync, unlinkSync, writeFileSync } from "node:fs"; import { compareReleaseTags } from "./release-notes"; +import { nextDevelopmentVersion } from "./version-line"; /** * `compareReleaseTags` wants a tag. The workflow supplies `github.event.release.tag_name` @@ -103,9 +104,7 @@ export function decideDevVersion(released: string, current: string): BumpDecisio if (!rel) throw new Error(`released version is not parseable: ${JSON.stringify(released)}`); if (!parseVersion(current)) throw new Error(`current version is not parseable: ${JSON.stringify(current)}`); - const candidate = rel.prerelease === null - ? `${rel.major}.${rel.minor + 1}.0` - : `${rel.major}.${rel.minor}.${rel.patch}`; + const candidate = nextDevelopmentVersion(released); // Nothing to do when dev is already clear of the RELEASED version. That is the real // question — the detector in tests/release-version-line.test.ts compares dev against diff --git a/scripts/release-notes.ts b/scripts/release-notes.ts index 48812539b0..16627f5f93 100644 --- a/scripts/release-notes.ts +++ b/scripts/release-notes.ts @@ -16,66 +16,14 @@ * bun scripts/release-notes.ts polish --in --out [--model ...] [--base-url ...] */ -type ParsedReleaseTag = { - major: number; - minor: number; - patch: number; - /** null = stable release; otherwise the SemVer prerelease identifier string. */ - prerelease: string | null; -}; - -function parseReleaseTag(tag: string): ParsedReleaseTag | null { - const match = /^v?(\d+)\.(\d+)\.(\d+)(?:-(.+))?$/.exec(tag.trim()); - if (!match) return null; - return { - major: Number(match[1]), - minor: Number(match[2]), - patch: Number(match[3]), - prerelease: match[4] ?? null, - }; -} - -/** SemVer identifier compare: numeric parts by number; numeric < non-numeric. */ -function comparePrereleaseIds(a: string, b: string): number { - const aParts = a.split("."); - const bParts = b.split("."); - const len = Math.max(aParts.length, bParts.length); - for (let i = 0; i < len; i += 1) { - const ap = aParts[i]; - const bp = bParts[i]; - if (ap === undefined) return -1; - if (bp === undefined) return 1; - const aNum = /^\d+$/.test(ap); - const bNum = /^\d+$/.test(bp); - if (aNum && bNum) { - const diff = Number(ap) - Number(bp); - if (diff !== 0) return diff; - continue; - } - if (aNum !== bNum) return aNum ? -1 : 1; - const cmp = ap.localeCompare(bp); - if (cmp !== 0) return cmp; - } - return 0; -} +import { compareTagsLenient } from "./version-line"; /** * Ascending SemVer-aware tag compare. Stable ranks after prereleases with the * same core version (`v2.7.42-preview.*` < `v2.7.42`). */ export function compareReleaseTags(a: string, b: string): number { - const pa = parseReleaseTag(a); - const pb = parseReleaseTag(b); - if (!pa || !pb) { - return a.localeCompare(b, undefined, { numeric: true, sensitivity: "base" }); - } - if (pa.major !== pb.major) return pa.major - pb.major; - if (pa.minor !== pb.minor) return pa.minor - pb.minor; - if (pa.patch !== pb.patch) return pa.patch - pb.patch; - if (pa.prerelease === null && pb.prerelease === null) return 0; - if (pa.prerelease === null) return 1; - if (pb.prerelease === null) return -1; - return comparePrereleaseIds(pa.prerelease, pb.prerelease); + return compareTagsLenient(a, b); } function sortVersionTagsAscending(tags: string[]): string[] { diff --git a/scripts/release.ts b/scripts/release.ts index c9939e09d4..ec595e7c0e 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -24,6 +24,7 @@ * behaves exactly as before. */ import { commandInvocation } from "../src/lib/win-exec"; +import { compareVersions as compareReleaseVersions } from "./version-line"; const args = process.argv.slice(2); interface GhRun { @@ -298,43 +299,7 @@ async function githubReleaseExists(tagName: string): Promise { process.exit(1); } -/** Order two semver strings per the semver.org rules (numeric identifiers numerically, - * numeric < alphanumeric prerelease, prerelease < release). Returns negative/0/positive. */ -export function compareReleaseVersions(left: string, right: string): number { - // SemVer 2.0.0: build metadata (+...) is valid and ignored for precedence, but - // anything else unparseable must fail CLOSED. Number() on a garbage core used to - // yield NaN, and NaN comparisons made the forward guard pass any candidate. - const SEMVER = /^(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; - const parse = (value: string) => { - const match = SEMVER.exec(value.trim()); - if (!match) throw new Error(`unparseable release version: ${JSON.stringify(value)}`); - const nums = [Number(match[1]), Number(match[2]), Number(match[3])]; - return { nums, pre: match[4] ? match[4].split(".") : null }; - }; - const a = parse(left); - const b = parse(right); - for (let i = 0; i < 3; i += 1) { - const delta = (a.nums[i] ?? 0) - (b.nums[i] ?? 0); - if (delta !== 0) return delta; - } - if (a.pre === null && b.pre === null) return 0; - if (a.pre === null) return 1; - if (b.pre === null) return -1; - const len = Math.max(a.pre.length, b.pre.length); - for (let i = 0; i < len; i += 1) { - const x = a.pre[i]; - const y = b.pre[i]; - if (x === undefined) return -1; - if (y === undefined) return 1; - const xn = /^\d+$/.test(x) ? Number(x) : null; - const yn = /^\d+$/.test(y) ? Number(y) : null; - if (xn !== null && yn !== null && xn !== yn) return xn - yn; - if (xn !== null && yn === null) return -1; - if (xn === null && yn !== null) return 1; - if (xn === null && yn === null && x !== y) return x < y ? -1 : 1; - } - return 0; -} +export { compareVersions as compareReleaseVersions } from "./version-line"; /** The proposed version must move its npm channel FORWARD: an unused-but-obsolete * target (e.g. cut from a dev branch whose version line trails main) would otherwise diff --git a/scripts/version-line.ts b/scripts/version-line.ts new file mode 100644 index 0000000000..5cd36434ff --- /dev/null +++ b/scripts/version-line.ts @@ -0,0 +1,85 @@ +export interface ParsedVersion { + major: number; + minor: number; + patch: number; + prerelease: readonly string[] | null; +} + +const VERSION_PATTERN = /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; + +/** Optional leading v, optional prerelease, optional (ignored) build metadata. */ +export function parseVersion(raw: string): ParsedVersion | null { + const match = VERSION_PATTERN.exec(raw.trim()); + if (!match) return null; + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: Number(match[3]), + prerelease: match[4] ? match[4].split(".") : null, + }; +} + +function compareParsedVersions( + left: ParsedVersion, + right: ParsedVersion, + compareText: (a: string, b: string) => number, +): number { + if (left.major !== right.major) return left.major - right.major; + if (left.minor !== right.minor) return left.minor - right.minor; + if (left.patch !== right.patch) return left.patch - right.patch; + if (left.prerelease === null && right.prerelease === null) return 0; + if (left.prerelease === null) return 1; + if (right.prerelease === null) return -1; + + const length = Math.max(left.prerelease.length, right.prerelease.length); + for (let i = 0; i < length; i += 1) { + const a = left.prerelease[i]; + const b = right.prerelease[i]; + if (a === undefined) return -1; + if (b === undefined) return 1; + const aIsNumeric = /^\d+$/.test(a); + const bIsNumeric = /^\d+$/.test(b); + if (aIsNumeric && bIsNumeric) { + const difference = Number(a) - Number(b); + if (difference !== 0) return difference; + continue; + } + if (aIsNumeric !== bIsNumeric) return aIsNumeric ? -1 : 1; + const difference = compareText(a, b); + if (difference !== 0) return difference; + } + return 0; +} + +/** Strict ordering for release decisions. */ +export function compareVersions(left: string, right: string): number { + const a = parseVersion(left); + if (!a) throw new Error(`unparseable release version: ${JSON.stringify(left)}`); + const b = parseVersion(right); + if (!b) throw new Error(`unparseable release version: ${JSON.stringify(right)}`); + return compareParsedVersions(a, b, (x, y) => x < y ? -1 : x > y ? 1 : 0); +} + +/** Lenient ordering for historical tag sets. */ +export function compareTagsLenient(left: string, right: string): number { + const a = parseVersion(left); + const b = parseVersion(right); + if (!a || !b) { + return left.localeCompare(right, undefined, { numeric: true, sensitivity: "base" }); + } + return compareParsedVersions(a, b, (x, y) => x.localeCompare(y)); +} + +/** + * The version a development line carries once `released` exists. + * + * X.Y.Z-preview.* -> X.Y.Z + * X.Y.Z (stable) -> X.(Y+1).0 + */ +export function nextDevelopmentVersion(released: string): string { + const parsed = parseVersion(released); + if (!parsed) throw new Error(`released version is not parseable: ${JSON.stringify(released)}`); + return parsed.prerelease === null + ? `${parsed.major}.${parsed.minor + 1}.0` + : `${parsed.major}.${parsed.minor}.${parsed.patch}`; +} diff --git a/tests/version-line.test.ts b/tests/version-line.test.ts new file mode 100644 index 0000000000..0b704e58ba --- /dev/null +++ b/tests/version-line.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, test } from "bun:test"; +import { + compareTagsLenient, + compareVersions, + nextDevelopmentVersion, + parseVersion, +} from "../scripts/version-line"; + +describe("version line algebra", () => { + test("parses optional v, prerelease identifiers, and ignored build metadata", () => { + expect(parseVersion(" v2.36.0-preview.20260829+build.1 ")).toEqual({ + major: 2, + minor: 36, + patch: 0, + prerelease: ["preview", "20260829"], + }); + expect(parseVersion("2.36.0+build.1")).toEqual({ + major: 2, + minor: 36, + patch: 0, + prerelease: null, + }); + expect(parseVersion("not-a-version")).toBeNull(); + expect(parseVersion("2.36")).toBeNull(); + expect(parseVersion("garbage")).toBeNull(); + }); + + test("orders SemVer cores and prerelease identifiers", () => { + expect(compareVersions("2.36.0-preview.2", "2.36.0-preview.10")).toBeLessThan(0); + expect(compareVersions("2.36.0-preview.10", "2.36.0-preview.beta")).toBeLessThan(0); + expect(compareVersions("2.36.0-preview.1", "2.36.0")).toBeLessThan(0); + expect(compareVersions("2.37.0-preview.1", "2.36.0")).toBeGreaterThan(0); + expect(compareVersions("v2.36.0", "2.36.0")).toBe(0); + }); + + test("ignores build metadata for strict release precedence", () => { + expect(compareVersions("2.19.4", "2.19.3+build.1")).toBeGreaterThan(0); + expect(compareVersions("2.19.3", "2.19.3+build.1")).toBe(0); + expect(() => compareVersions("2.19.4", "not-a-version")).toThrow(/unparseable/); + }); + + test("keeps historical tag sorting lenient while release decisions fail closed", () => { + const fallback = "vNOTAVERSION".localeCompare("v2.42.0", undefined, { + numeric: true, + sensitivity: "base", + }); + expect(compareTagsLenient("vNOTAVERSION", "v2.42.0")).toBe(fallback); + expect(() => compareVersions("vNOTAVERSION", "v2.42.0")).toThrow(/unparseable/); + }); + + test("a stable release is succeeded by the next minor", () => { + expect(nextDevelopmentVersion("2.36.0")).toBe("2.37.0"); + expect(nextDevelopmentVersion("2.33.0")).toBe("2.34.0"); + expect(nextDevelopmentVersion("v2.36.0")).toBe("2.37.0"); + }); + + test("a prerelease is succeeded by its own stable core", () => { + expect(nextDevelopmentVersion("2.36.0-preview.20260829")).toBe("2.36.0"); + expect(nextDevelopmentVersion("2.36.0-preview.20260829")).not.toBe("2.37.0"); + expect(nextDevelopmentVersion("v2.36.0-preview.20260829")).toBe("2.36.0"); + }); + + test("refuses malformed released versions instead of guessing", () => { + expect(() => nextDevelopmentVersion("not-a-version")).toThrow(/not parseable/); + expect(() => nextDevelopmentVersion("2.36")).toThrow(/not parseable/); + expect(() => nextDevelopmentVersion("garbage")).toThrow(/not parseable/); + }); +}); From b181dd7911224279d90e450b0a3b5046b42cfc8b Mon Sep 17 00:00:00 2001 From: jun Date: Fri, 4 Sep 2026 23:51:15 +0900 Subject: [PATCH 5/6] feat(release): add --bump and move the dev version PR before the release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phases 020 and 030 of devlog/_plan/260904_release_version_line/, implemented in parallel and committed together because they share scripts/version-line.ts. 020 — `--bump patch|minor|major` The maintainer no longer hand-passes a version string. Two resolvers keep the channels apart, which the audit required: a single global floor would let a future v2.43.0-preview.1 turn `--bump minor` into 2.44.0 and skip the intended 2.43.0. - nextStableRelease derives from the stable channel and tags only. A future same-core preview may validate the target core but never raises the base, and a patch bump is REFUSED outright when a preview tag sits above the base — publishing a preview for a higher core closes the older stable patch line. - nextPreviewRelease picks a core outranking the latest stable, then a prerelease outranking existing preview tags. Succession comes from the incumbent, so an equal stamp increments its ordinal (.3 becomes .4) and an older stamp is an explicit clock-regression error rather than a silently behind candidate. 030 — the dev version PR opens BEFORE the release dev-version-bump.yml stops being a repairer and becomes an opener. The count of reviewed commits into dev is unchanged — that is structural, since Protect dev requires review — but the window in which dev and every open PR carry a red they cannot fix disappears. - workflow_call is deleted together with its only caller, the bump-dev-version job in release.yml. A repository-wide search found no second caller. - One normalized target version is resolved before the decision step, so no downstream consumer reads a raw event input. - The chosen-version freeness check is RETAINED and the target-availability check is added alongside it. Replacing it would have dropped candidate-collision protection. - release.yml gains a readiness gate and an ordering gate. The ordering gate runs after the fresh tag fetch — before it, the stale tag set would defeat the point — and --allow-existing-tag-at-head is granted only for a dry run whose tag names the exact SHA, preserving the deliberate exception that already lived there. Verification, per phase, focused files only: 020: version-line 20 pass, release-helper 39 pass, release-version-line 3 pass, typecheck exit 0, privacy:scan passed, docs-site build 425 pages 030: ci-workflows 136 pass, bump-dev-version 14 pass, version-line 20 pass, typecheck exit 0 Red-before proofs: 020's resolver suite failed on the higher-core patch refusal and the equal-stamp succession before implementation; 030's ordering assertion fails when the gate is moved ahead of the tag fetch and passes when restored. MAINTAINERS.md still describes the old post-release flow. That correction belongs to phase 040 and is deliberately not in this commit. --- .github/workflows/dev-version-bump.yml | 142 +++++++---- .github/workflows/release.yml | 65 ++--- docs-site/src/content/docs/contributing.md | 5 + docs-site/src/content/docs/fr/contributing.md | 5 + docs-site/src/content/docs/ja/contributing.md | 5 + docs-site/src/content/docs/ko/contributing.md | 5 + docs-site/src/content/docs/ru/contributing.md | 5 + docs-site/src/content/docs/tr/contributing.md | 6 +- .../src/content/docs/zh-cn/contributing.md | 4 + .../src/content/docs/zh-tw/contributing.md | 4 + scripts/release.ts | 100 ++++++-- scripts/version-line.ts | 234 ++++++++++++++++++ tests/bump-dev-version.test.ts | 96 ++++++- tests/ci-workflows.test.ts | 127 ++++++++++ tests/release-helper.test.ts | 85 ++++++- tests/version-line.test.ts | 139 +++++++++++ 16 files changed, 914 insertions(+), 113 deletions(-) diff --git a/.github/workflows/dev-version-bump.yml b/.github/workflows/dev-version-bump.yml index b884b04ace..fca538ff8d 100644 --- a/.github/workflows/dev-version-bump.yml +++ b/.github/workflows/dev-version-bump.yml @@ -1,48 +1,41 @@ name: Dev version bump -# When a release publishes, open a pull request that moves `dev` past the published -# version. Without this, `dev` keeps carrying a version that is at or behind a released -# one, and `tests/release-version-line.test.ts` fails on `dev` and on every pull request -# opened against it - inherited red a contributor cannot fix from their own diff. +# Before a release publishes, open a pull request that moves `dev` past the intended +# version. Merge that pull request before promoting and publishing so `dev` and pull +# requests based on it never inherit a version-line failure from the new tag. # # That has been repaired by hand four times: 32529c2b2, e4a85d134, 076ad3036, befcac3e1. -# The second of those ADDED the detector and two more repairs followed it, so more -# visibility was never the missing piece; a prepared change was. +# The workflow now prepares the move before publication. Explicit repair mode retains +# the old catch-up capability if a release somehow publishes without the pre-move. # # WHAT THIS DOES NOT DO. It does not push to `dev`. It opens a pull request and a human # merges it, because ruleset `Protect dev` requires an approving review and code-owner -# sign-off that a bot cannot supply. Until that merge the red persists. This converts a -# forgotten chore into a queued, reviewable change - not into an automatic repair. +# sign-off that a bot cannot supply. `release.yml` independently refuses publication +# until `dev` already outranks the intended version. # -# WHY THIS IS CALLED, NOT TRIGGERED. It used to listen for `release: published`, and in -# that form it ran ZERO times across v2.37.0, v2.38.0 and v2.39.0 - every one of those -# bumps was still opened by hand (#3045, #3076, #3127). The workflow was not broken; the -# event never existed. `release.yml` creates the GitHub release with -# `GH_TOKEN: ${{ github.token }}`, and GitHub does not start workflow runs from events -# raised by the default `GITHUB_TOKEN`. A `release: published` listener therefore cannot -# observe a release this repository publishes itself, no matter which branch it sits on. +# WHY THIS IS DISPATCHED. The intended version is known before publication, and this +# workflow's purpose is to queue the reviewed `dev` move first. It is not called by the +# release workflow after an irreversible publish, and it does not react to release events. # -# The fix keeps the credential surface unchanged: no PAT, no app token, no -# `contents: write` on the release job. `release.yml` CALLS this workflow directly after -# a successful publish, so the run is a child of the release run instead of a reaction to -# an event that is never delivered. -# -# A `workflow_call` body resolves from the CALLER's ref, and `release.yml` only ever runs -# on `main` or `preview` (its own branch gate). So this file must be on `main` to take -# effect - the same promotion requirement the old comment described, now for a different -# reason. -# -# There is deliberately no `workflow_dispatch`: a branch-selected manual run executes -# THAT branch body with `contents: write`. Re-drive a missed run by running -# `bun scripts/bump-dev-version.ts package.json` locally and opening the pull -# request normally. +# A branch-selected dispatch executes that branch's workflow body with write permission. +# The in-job guard therefore rejects accidental non-default-ref dispatches. It is an early +# warning, not a security boundary: a writer could remove it on their branch. Protected +# release branches and the required review on `dev` remain the enforcement boundaries. on: - workflow_call: + workflow_dispatch: inputs: - released-version: - description: "The tag that just published, e.g. v2.39.0" + intended-version: + description: "Version about to be released (pre-move), or one already published (repair)" required: true type: string + mode: + description: "pre-move (default) or repair — repair allows an already-published version" + required: false + default: pre-move + type: choice + options: + - pre-move + - repair permissions: {} @@ -83,17 +76,59 @@ jobs: - name: Install dependencies run: bun install --frozen-lockfile + - name: Refuse a dispatch from a non-default ref + run: | + test "$GITHUB_REF" = "refs/heads/${{ github.event.repository.default_branch }}" || { + echo "::error::this workflow may only be dispatched from the default branch" + exit 1 + } + + - name: Resolve the target version + id: target + env: + INTENDED: ${{ inputs.intended-version }} + MODE: ${{ inputs.mode }} + run: | + set -euo pipefail + target="${INTENDED:-}" + if [ -z "$target" ]; then + echo "::error::intended-version was not supplied" + exit 1 + fi + echo "version=${target}" >> "$GITHUB_OUTPUT" + if [ "${MODE:-pre-move}" = "repair" ]; then + echo "mode=repair" >> "$GITHUB_OUTPUT" + else + echo "mode=pre-move" >> "$GITHUB_OUTPUT" + fi + - name: Decide the version dev should carry id: decide env: - RELEASED_VERSION: ${{ inputs.released-version }} + RELEASED_VERSION: ${{ steps.target.outputs.version }} run: | set -euo pipefail bun scripts/bump-dev-version.ts "${RELEASED_VERSION}" package.json + - name: Prove the intended version is not already released + if: ${{ steps.target.outputs.mode == 'pre-move' }} + env: + INTENDED: ${{ steps.target.outputs.version }} + run: | + set -euo pipefail + git fetch --force --tags origin + if git rev-parse -q --verify "refs/tags/v${INTENDED#v}" >/dev/null; then + echo "::error::v${INTENDED#v} already exists; this is a catch-up, not a pre-move" + exit 1 + fi + if npm view "@bitkyc08/opencodex@${INTENDED#v}" version >/dev/null 2>&1; then + echo "::error::${INTENDED#v} is already on npm" + exit 1 + fi + - name: Prove the chosen version is unused if: ${{ steps.decide.outputs.changed == 'true' }} - # The script decides the candidate from the released version SHAPE, which is all + # The script decides the candidate from the target version SHAPE, which is all # a pure function can see. Whether that candidate is actually FREE is a property # of the tag set, so it is settled here by the detector that already owns the # question. If this fails, no pull request is opened and the job goes red asking @@ -104,21 +139,31 @@ jobs: if: ${{ steps.decide.outputs.changed == 'true' }} env: GH_TOKEN: ${{ github.token }} + MODE: ${{ steps.target.outputs.mode }} NEXT_VERSION: ${{ steps.decide.outputs.version }} - RELEASED_VERSION: ${{ inputs.released-version }} + TARGET_VERSION: ${{ steps.target.outputs.version }} run: | set -euo pipefail branch="codex/dev-version-${NEXT_VERSION}" + if [ "${MODE}" = "repair" ]; then + subject="fix(release): move dev to ${NEXT_VERSION} after ${TARGET_VERSION}" + reason="\`${TARGET_VERSION}\` has published, so \`dev\` is carrying a version at or behind a released one and \`tests/release-version-line.test.ts\` fails on \`dev\` and on every pull request opened against it. This is the post-publish repair." + freeness="\`bun test tests/release-version-line.test.ts\` proved the chosen development version is unused." + else + subject="chore(release): open dev at ${NEXT_VERSION} before releasing ${TARGET_VERSION}" + reason="\`${TARGET_VERSION}\` is about to be released. Merging this first means \`dev\` already outranks the new tag when it lands, so neither \`dev\` nor any open pull request ever inherits the version-line failure. \`release.yml\` refuses to publish until this has merged." + freeness="The workflow proved \`${TARGET_VERSION}\` has neither a Git tag nor an npm publication, and \`bun test tests/release-version-line.test.ts\` proved the chosen development version is unused." + fi - # Idempotent: a second publish, a re-run, or a manual repair must not turn a - # successful release into a red job. + # Idempotent: a repeated dispatch, a re-run, or a manual repair must not turn + # an already-queued version move into a red job. # # Check the PULL REQUEST as well as the branch, not just the branch. A security # review caught that: an open bump pull request whose head branch was deleted # leaves the branch check passing, so the job would recreate the branch and then - # fail on `gh pr create` with "already exists" — turning a successful release red - # for a repair that was already queued. + # fail on `gh pr create` with "already exists" — turning a successful run red + # for a move that was already queued. open_prs="$(gh pr list --base dev --head "${branch}" --state open --json number --jq 'length')" if [ "${open_prs}" != "0" ]; then echo "::notice::a bump pull request for ${branch} is already open; nothing to do" @@ -126,7 +171,7 @@ jobs: fi # An existing branch is NOT terminal. If a previous run pushed the branch and then - # failed at `gh pr create`, exiting here would leave the repair permanently unqueued + # failed at `gh pr create`, exiting here would leave the move permanently unqueued # while every rerun reports success - the exact failure mode a reviewer caught. So # reuse the branch and fall through to pull-request creation instead. if git ls-remote --exit-code --heads origin "${branch}" >/dev/null 2>&1; then @@ -152,31 +197,28 @@ jobs: git config user.email "41898282+github-actions[bot]@users.noreply.github.com" git checkout -b "${branch}" git add package.json - git commit -m "fix(release): move dev to ${NEXT_VERSION} after ${RELEASED_VERSION}" + git commit -m "${subject}" git push origin "${branch}" fi gh pr create \ --base dev \ --head "${branch}" \ - --title "fix(release): move dev to ${NEXT_VERSION} after ${RELEASED_VERSION}" \ + --title "${subject}" \ --body "$(cat < # commits/pushes the bump; publish workflow is dry-run by default +bun run release --bump minor # derive the next patch, minor, or major version from tags and npm channels bun run release --publish # publish after the CI-gated dry run is understood bun run release:watch # watch the newest Release workflow run ``` +`--bump patch|minor|major` is an alternative to an explicit version. Once a preview tag opens a +higher version core, `--bump patch` refuses to continue the older stable patch line; ship that fix +in the open preview core instead. + ## Branches - `dev` — the only integration target. Open your pull request here. diff --git a/docs-site/src/content/docs/fr/contributing.md b/docs-site/src/content/docs/fr/contributing.md index facce7337b..75b1d6ae4d 100644 --- a/docs-site/src/content/docs/fr/contributing.md +++ b/docs-site/src/content/docs/fr/contributing.md @@ -94,10 +94,15 @@ Utilisez l'assistant pour les versions : ```bash bun run release # commits/pushes the bump; publish workflow is dry-run by default +bun run release --bump minor # calcule la prochaine version patch, minor ou major depuis les tags et canaux npm bun run release --publish # publish after the CI-gated dry run is understood bun run release:watch # watch the newest Release workflow run ``` +`--bump patch|minor|major` remplace une version explicite. Dès qu’un tag de préversion ouvre un +core supérieur, `--bump patch` refuse de prolonger l’ancienne ligne stable ; publiez plutôt le +correctif dans le core de préversion ouvert. + ## Branches - `dev` — l’unique branche d’intégration. Ciblez-la avec votre pull request. diff --git a/docs-site/src/content/docs/ja/contributing.md b/docs-site/src/content/docs/ja/contributing.md index 26a1de8c42..c8b3724bde 100644 --- a/docs-site/src/content/docs/ja/contributing.md +++ b/docs-site/src/content/docs/ja/contributing.md @@ -76,10 +76,15 @@ GitHub Actions は必要な作業のみを行います。 ```bash bun run release # バージョン bump を commit/push、publish ワークフローはデフォルト dry-run +bun run release --bump minor # tag と npm channel から次の patch、minor、major バージョンを導出 bun run release --publish # CI-gated dry-run を確認した後、実際の publish bun run release:watch # 直近の Release ワークフロー run を監視 ``` +明示的なバージョンの代わりに `--bump patch|minor|major` を指定できます。上位 core の preview tag が +作られた後は、`--bump patch` は古い stable patch ラインの継続を拒否します。その修正は開いている +preview core に含めてください。 + ## ブランチ - `dev` — 唯一の統合先。すべての PR をここに出します。 diff --git a/docs-site/src/content/docs/ko/contributing.md b/docs-site/src/content/docs/ko/contributing.md index 21892e69cf..c0326b656a 100644 --- a/docs-site/src/content/docs/ko/contributing.md +++ b/docs-site/src/content/docs/ko/contributing.md @@ -76,10 +76,15 @@ GitHub Actions는 필요한 작업만 수행합니다. ```bash bun run release # 버전 bump를 commit/push, publish workflow는 기본 dry-run +bun run release --bump minor # tag와 npm channel에서 다음 patch, minor, major 버전을 계산 bun run release --publish # CI-gated dry-run을 확인한 뒤 실제 publish bun run release:watch # 가장 최근 Release workflow run 감시 ``` +명시적 버전 대신 `--bump patch|minor|major`를 사용할 수 있습니다. 더 높은 core의 preview tag가 +열린 뒤에는 `--bump patch`가 이전 stable patch 라인의 계속을 거부합니다. 해당 수정은 열린 preview +core에 포함해 릴리즈하세요. + ## 브랜치 - `dev` — 유일한 통합 대상. 모든 PR을 여기로 올립니다. diff --git a/docs-site/src/content/docs/ru/contributing.md b/docs-site/src/content/docs/ru/contributing.md index 7ac718ae2d..473830fd9a 100644 --- a/docs-site/src/content/docs/ru/contributing.md +++ b/docs-site/src/content/docs/ru/contributing.md @@ -76,10 +76,15 @@ GitHub Actions намеренно остаются компактными: ```bash bun run release # коммитит/пушит bump версии; publish workflow по умолчанию dry-run +bun run release --bump minor # вычисляет следующую patch, minor или major версию по тегам и каналам npm bun run release --publish # publish после осознанного CI-gated dry-run bun run release:watch # наблюдение за последним запуском Release workflow ``` +`--bump patch|minor|major` можно использовать вместо явной версии. После появления preview-тега +для более высокого core команда `--bump patch` откажется продолжать старую stable patch-линию; +включите исправление в уже открытую preview-версию. + ## Ветки - `dev` — единственная цель интеграции. Открывайте все PR сюда. diff --git a/docs-site/src/content/docs/tr/contributing.md b/docs-site/src/content/docs/tr/contributing.md index 9fd8c71a20..024a02d9ad 100644 --- a/docs-site/src/content/docs/tr/contributing.md +++ b/docs-site/src/content/docs/tr/contributing.md @@ -108,10 +108,15 @@ Sürümler için yardımcıyı kullanın: ```bash bun run release # sürüm artışını commit/push eder; yayınlama iş akışı varsayılan olarak kuru çalıştırmadır (dry-run) +bun run release --bump minor # tag'ler ve npm kanallarından sonraki patch, minor veya major sürümü türetir bun run release --publish # CI onaylı kuru çalıştırma anlaşıldıktan sonra yayınlayın bun run release:watch # en yeni Sürüm iş akışı çalıştırmasını izleyin ``` +Açık bir sürüm yerine `--bump patch|minor|major` kullanılabilir. Daha yüksek bir core için preview +tag'i açıldıktan sonra `--bump patch`, eski stable patch hattını sürdürmeyi reddeder; düzeltmeyi açık +preview core içinde yayınlayın. + ## Dallar - `dev` — tek entegrasyon hedefi. Çekme isteğinizi burada açın. @@ -255,4 +260,3 @@ typecheck`, davranış için odaklanmış bir `bun test tests/.test.ts` veya çalışma zamanı probu, ardından etkilenen yüzeye uygun daha geniş kapılar. opencodex büyük partiler yerine küçük, doğrulanabilir commit'leri tercih eder. - diff --git a/docs-site/src/content/docs/zh-cn/contributing.md b/docs-site/src/content/docs/zh-cn/contributing.md index 5f102c587f..0a417807ec 100644 --- a/docs-site/src/content/docs/zh-cn/contributing.md +++ b/docs-site/src/content/docs/zh-cn/contributing.md @@ -72,10 +72,14 @@ GitHub Actions 有意只保留必要步骤： ```bash bun run release # commit/push 版本 bump；publish workflow 默认 dry-run +bun run release --bump minor # 根据 tag 与 npm channel 推导下一个 patch、minor 或 major 版本 bun run release --publish # 确认 CI-gated dry-run 后真正 publish bun run release:watch # 观察最新的 Release workflow run ``` +可用 `--bump patch|minor|major` 代替显式版本。较高 core 的 preview tag 建立后，`--bump patch` +会拒绝继续旧的 stable patch 版本线；请将修复包含在已开启的 preview core 中发布。 + ## 分支 - `dev` — 唯一的集成目标。请把所有 PR 提到这里。 diff --git a/docs-site/src/content/docs/zh-tw/contributing.md b/docs-site/src/content/docs/zh-tw/contributing.md index e39c0d1f66..6a70ac79e1 100644 --- a/docs-site/src/content/docs/zh-tw/contributing.md +++ b/docs-site/src/content/docs/zh-tw/contributing.md @@ -72,10 +72,14 @@ GitHub Actions 有意只保留必要步驟： ```bash bun run release # commit/push 版本 bump；publish workflow 預設 dry-run +bun run release --bump minor # 依 tag 與 npm channel 推導下一個 patch、minor 或 major 版本 bun run release --publish # 確認 CI-gated dry-run 後真正 publish bun run release:watch # 觀察最新的 Release workflow run ``` +可用 `--bump patch|minor|major` 取代明確版本。較高 core 的 preview tag 建立後，`--bump patch` +會拒絕延續舊的 stable patch 版本線；請把修正納入已開啟的 preview core 中釋出。 + ## 分支 - `dev` — 唯一的整合目標。請在此開啟 pull request。 diff --git a/scripts/release.ts b/scripts/release.ts index ec595e7c0e..3f222a2445 100644 --- a/scripts/release.ts +++ b/scripts/release.ts @@ -4,6 +4,7 @@ * * Usage: * bun scripts/release.ts [--tag latest|preview] [--publish] + * bun scripts/release.ts --bump patch|minor|major [--tag latest|preview] [--publish] * Preflight (clean tree + dependency audit + typecheck + tests + privacy scan) → bump package.json → commit → push → * wait for Cross-platform CI → dispatch the Release workflow → watch it. * The version bump commit/push is real; the Release workflow publish step is dry-run by default. @@ -13,6 +14,7 @@ * * Example: bun scripts/release.ts 0.1.0 # commit/push bump, workflow dry-run publish * bun scripts/release.ts 0.1.0 --publish # actually publish 0.1.0 + * bun scripts/release.ts --bump minor # resolve the next version from tags + npm channels * * Requires: gh CLI (authed). Publishing is tokenless via Trusted Publishing (OIDC) — no NPM_TOKEN. * @@ -24,7 +26,13 @@ * behaves exactly as before. */ import { commandInvocation } from "../src/lib/win-exec"; -import { compareVersions as compareReleaseVersions } from "./version-line"; +import { + compareVersions as compareReleaseVersions, + nextPreviewRelease, + nextStableRelease, + parseVersion, + type ReleaseBumpKind, +} from "./version-line"; const args = process.argv.slice(2); interface GhRun { @@ -301,23 +309,25 @@ async function githubReleaseExists(tagName: string): Promise { export { compareVersions as compareReleaseVersions } from "./version-line"; -/** The proposed version must move its npm channel FORWARD: an unused-but-obsolete - * target (e.g. cut from a dev branch whose version line trails main) would otherwise - * pass the unused-version check and publish a regression over the channel tip. */ -async function assertChannelVersionMovesForward(packageName: string, version: string, channel: string): Promise { +async function readNpmDistTags(packageName: string): Promise> { const result = await runQuiet(["npm", "view", packageName, "dist-tags", "--json"]); if (result.exitCode !== 0) { console.error(`✗ failed to read npm dist-tags for ${packageName}`); if (result.stderr) console.error(result.stderr); process.exit(1); } - let distTags: Record; try { - distTags = JSON.parse(result.stdout) as Record; + return JSON.parse(result.stdout) as Record; } catch { console.error(`✗ npm dist-tags response for ${packageName} was not JSON`); process.exit(1); } +} + +/** The proposed version must move its npm channel FORWARD: an unused-but-obsolete + * target (e.g. cut from a dev branch whose version line trails main) would otherwise + * pass the unused-version check and publish a regression over the channel tip. */ +function assertChannelVersionMovesForward(version: string, channel: string, distTags: Record): void { const current = distTags[channel]; if (!current) return; // channel not published yet — nothing to regress let forward: number; @@ -449,11 +459,38 @@ if (args[0] === "watch") { process.exit(0); } -const version = args[0]; -if (!version || !/^\d+\.\d+\.\d+(-[\w.]+)?$/.test(version)) { - console.error("Usage: bun scripts/release.ts [--tag latest|preview] [--publish]\n bun scripts/release.ts watch"); +const usage = "Usage: bun scripts/release.ts [--tag latest|preview] [--publish]\n" + + " bun scripts/release.ts --bump patch|minor|major [--tag latest|preview] [--publish]\n" + + " bun scripts/release.ts watch"; +const explicitVersion = args[0] && !args[0].startsWith("--") ? args[0] : null; +const bumpIndexes = args.flatMap((arg, index) => arg === "--bump" ? [index] : []); +if (bumpIndexes.length > 1) { + console.error(`--bump may be supplied only once.\n${usage}`); + process.exit(1); +} +const bumpIndex = bumpIndexes[0]; +const rawBumpKind = bumpIndex === undefined ? null : args[bumpIndex + 1] ?? null; +if (rawBumpKind !== null && !["patch", "minor", "major"].includes(rawBumpKind)) { + console.error(`--bump must be one of patch|minor|major (got ${JSON.stringify(rawBumpKind)}).`); + process.exit(1); +} +if (bumpIndex !== undefined && rawBumpKind === null) { + console.error("--bump requires one of patch|minor|major."); + process.exit(1); +} +if (explicitVersion !== null && bumpIndex !== undefined) { + console.error(`An explicit version and --bump are mutually exclusive; supply exactly one.\n${usage}`); + process.exit(1); +} +if (explicitVersion === null && bumpIndex === undefined) { + console.error(`Exactly one of an explicit version or --bump is required.\n${usage}`); + process.exit(1); +} +if (explicitVersion !== null && !/^\d+\.\d+\.\d+(-[\w.]+)?$/.test(explicitVersion)) { + console.error(usage); process.exit(1); } +const bumpKind = rawBumpKind as ReleaseBumpKind | null; const dryRun = !args.includes("--publish"); // 1. Preflight — must be on main or preview, and local verification must pass. @@ -465,6 +502,44 @@ if (tag !== expectedTag) { console.error(`Release tag mismatch: ${branch} releases must use npm dist-tag '${expectedTag}' (got '${tag}').`); process.exit(1); } +if (!allowedBranches.includes(branch)) { console.error(`✗ must be on ${allowedBranches.join(" or ")} (currently ${branch}).`); process.exit(1); } +if ((await capture(["git", "status", "--porcelain"])).trim()) { console.error("✗ working tree not clean — commit or stash first."); process.exit(1); } +const packageName = await readPackageName(); +const distTags = await readNpmDistTags(packageName); +let version = explicitVersion; +if (version === null) { + const tags = (await capture(["git", "tag", "--list", "v*"])) + .split(/\r?\n/) + .map(value => value.trim()) + .filter(Boolean); + const stableTags: string[] = []; + const previewTags: string[] = []; + for (const candidate of tags) { + const parsed = parseVersion(candidate); + if (!parsed) continue; + (parsed.prerelease === null ? stableTags : previewTags).push(candidate); + } + try { + version = tag === "preview" + ? nextPreviewRelease({ + kind: bumpKind!, + stableTip: distTags.latest ?? null, + stableTags, + previewTip: distTags.preview ?? null, + previewTags, + stamp: new Date().toISOString().slice(0, 10).replaceAll("-", ""), + }) + : nextStableRelease({ + kind: bumpKind!, + stableTip: distTags.latest ?? null, + stableTags, + previewTags, + }); + } catch (error) { + console.error(`✗ ${error instanceof Error ? error.message : String(error)}`); + process.exit(1); + } +} if (branch === "preview" && !version.includes("-preview.")) { console.error(`Preview releases must use a preview prerelease version (got ${version}).`); process.exit(1); @@ -473,12 +548,9 @@ if (branch === "main" && version.includes("-")) { console.error(`Main releases must use a stable semver version (got ${version}).`); process.exit(1); } -if (!allowedBranches.includes(branch)) { console.error(`✗ must be on ${allowedBranches.join(" or ")} (currently ${branch}).`); process.exit(1); } -if ((await capture(["git", "status", "--porcelain"])).trim()) { console.error("✗ working tree not clean — commit or stash first."); process.exit(1); } -const packageName = await readPackageName(); console.log(`→ release metadata preflight (${packageName}@${version})`); await assertUnusedReleaseVersion(packageName, version); -await assertChannelVersionMovesForward(packageName, version, tag); +assertChannelVersionMovesForward(version, tag, distTags); console.log("→ dependency audit"); await runLoud(["bun", "run", "audit:high"]); console.log("→ typecheck"); diff --git a/scripts/version-line.ts b/scripts/version-line.ts index 5cd36434ff..b33eba2524 100644 --- a/scripts/version-line.ts +++ b/scripts/version-line.ts @@ -5,6 +5,8 @@ export interface ParsedVersion { prerelease: readonly string[] | null; } +export type ReleaseBumpKind = "patch" | "minor" | "major"; + const VERSION_PATTERN = /^v?(\d+)\.(\d+)\.(\d+)(?:-([0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*))?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; /** Optional leading v, optional prerelease, optional (ignored) build metadata. */ @@ -83,3 +85,235 @@ export function nextDevelopmentVersion(released: string): string { ? `${parsed.major}.${parsed.minor + 1}.0` : `${parsed.major}.${parsed.minor}.${parsed.patch}`; } + +function newestVersion(versions: readonly string[]): string | null { + return versions.reduce((newest, version) => { + if (!parseVersion(version)) { + throw new Error(`unparseable release version: ${JSON.stringify(version)}`); + } + return newest === null || compareVersions(version, newest) > 0 ? version : newest; + }, null); +} + +function stableBase(stableTip: string | null, stableTags: readonly string[]): string { + const candidates = stableTip === null ? stableTags : [stableTip, ...stableTags]; + for (const candidate of candidates) { + const parsed = parseVersion(candidate); + if (!parsed || parsed.prerelease !== null) { + throw new Error(`stable release version is not parseable as stable SemVer: ${JSON.stringify(candidate)}`); + } + } + const base = newestVersion(candidates); + if (base === null) throw new Error("cannot resolve a release bump without a stable channel tip or stable tag"); + return base; +} + +function versionCore(version: string): string { + const parsed = parseVersion(version); + if (!parsed) throw new Error(`unparseable release version: ${JSON.stringify(version)}`); + return `${parsed.major}.${parsed.minor}.${parsed.patch}`; +} + +function bumpCore(base: string, kind: ReleaseBumpKind): string { + const parsed = parseVersion(base); + if (!parsed || parsed.prerelease !== null) { + throw new Error(`release bump base is not a stable version: ${JSON.stringify(base)}`); + } + if (kind === "major") return `${parsed.major + 1}.0.0`; + if (kind === "minor") return `${parsed.major}.${parsed.minor + 1}.0`; + return `${parsed.major}.${parsed.minor}.${parsed.patch + 1}`; +} + +function higherCorePreview(base: string, previews: readonly string[]): string | null { + const blockers = previews.filter(preview => { + const parsed = parseVersion(preview); + if (!parsed || parsed.prerelease === null) { + throw new Error(`preview release version is not parseable as prerelease SemVer: ${JSON.stringify(preview)}`); + } + return compareVersions(versionCore(preview), versionCore(base)) > 0; + }); + return newestVersion(blockers); +} + +function assertAboveGlobalFloor(candidate: string, published: readonly (string | null)[]): void { + const floor = newestVersion(published.filter((version): version is string => version !== null)); + if (floor !== null && compareVersions(candidate, floor) <= 0) { + throw new Error(`resolved release ${candidate} does not outrank the global published floor ${floor}`); + } +} + +/** + * Resolve the next stable release from the stable channel only. Preview tags are + * consulted only for the higher-core patch refusal and the final global assertion. + */ +export function nextStableRelease(input: { + kind: ReleaseBumpKind; + stableTip: string | null; + stableTags: string[]; + previewTags: string[]; +}): string { + const base = stableBase(input.stableTip, input.stableTags); + if (input.kind === "patch") { + const blocker = higherCorePreview(base, input.previewTags); + if (blocker !== null) { + throw new Error( + `cannot bump stable patch from ${versionCore(base)} while higher-core preview ${blocker} is open; ship the fix in ${versionCore(blocker)}`, + ); + } + } + + const candidate = bumpCore(base, input.kind); + assertAboveGlobalFloor(candidate, [input.stableTip, ...input.stableTags, ...input.previewTags]); + return candidate; +} + +interface PreviewIdentity { + ordinal: number; + stamp: string; +} + +function previewIdentity(version: string): PreviewIdentity { + const parsed = parseVersion(version); + const prerelease = parsed?.prerelease; + if ( + !prerelease + || prerelease[0] !== "preview" + || !/^\d{8}$/.test(prerelease[1] ?? "") + || prerelease.length > 3 + || (prerelease[2] !== undefined && !/^\d+$/.test(prerelease[2])) + ) { + throw new Error(`preview incumbent has an unsupported prerelease shape: ${JSON.stringify(version)}`); + } + return { + stamp: prerelease[1]!, + ordinal: prerelease[2] === undefined ? 1 : Number(prerelease[2]), + }; +} + +/** Resolve a preview core from the stable line, then succeed its same-core incumbent. */ +export function nextPreviewRelease(input: { + kind: ReleaseBumpKind; + stableTip: string | null; + stableTags: string[]; + previewTip: string | null; + previewTags: string[]; + stamp: string; +}): string { + if (!/^\d{8}$/.test(input.stamp)) { + throw new Error(`preview stamp must be YYYYMMDD: ${JSON.stringify(input.stamp)}`); + } + + const base = stableBase(input.stableTip, input.stableTags); + const allPreviews = input.previewTip === null + ? input.previewTags + : [input.previewTip, ...input.previewTags]; + if (input.kind === "patch") { + const blocker = higherCorePreview(base, allPreviews); + if (blocker !== null) { + throw new Error( + `cannot bump preview patch from ${versionCore(base)} while higher-core preview ${blocker} is open`, + ); + } + } + + const core = bumpCore(base, input.kind); + const sameCorePreviews = allPreviews.filter(preview => { + const parsed = parseVersion(preview); + if (!parsed || parsed.prerelease === null) { + throw new Error(`preview release version is not parseable as prerelease SemVer: ${JSON.stringify(preview)}`); + } + return versionCore(preview) === core; + }); + const incumbent = newestVersion(sameCorePreviews); + let candidate = `${core}-preview.${input.stamp}`; + + if (incumbent !== null) { + const identity = previewIdentity(incumbent); + if (input.stamp < identity.stamp) { + throw new Error( + `preview clock regression: supplied stamp ${input.stamp} is older than incumbent stamp ${identity.stamp}`, + ); + } + if (input.stamp === identity.stamp) { + candidate = `${candidate}.${identity.ordinal + 1}`; + } + if (compareVersions(candidate, incumbent) <= 0) { + throw new Error(`resolved preview ${candidate} does not succeed incumbent ${incumbent}`); + } + } + + assertAboveGlobalFloor(candidate, [ + input.stableTip, + ...input.stableTags, + input.previewTip, + ...input.previewTags, + ]); + return candidate; +} + +/** + * The publication-boundary ordering policy: a candidate must strictly outrank + * every release tag. The equality exception is granted only by release.yml for + * a dry run whose existing tag already names the commit under test. + */ +export function assertReleasable(input: { + candidate: string; + tags: readonly string[]; + allowExistingTagAtHead?: boolean; +}): { ok: true } | { ok: false; blockedBy: string } { + for (const tag of input.tags) { + const order = compareVersions(input.candidate, tag); + if (order < 0 || (order === 0 && !input.allowExistingTagAtHead)) { + return { ok: false, blockedBy: tag }; + } + } + return { ok: true }; +} + +const VERSION_LINE_USAGE = "usage: bun scripts/version-line.ts assert-ahead | assert-releasable [--allow-existing-tag-at-head]"; + +if (import.meta.main) { + const [command, ...rest] = process.argv.slice(2); + + if (command === "assert-ahead") { + const [left, right] = rest; + if (!left || !right) { + console.error(VERSION_LINE_USAGE); + process.exit(1); + } + if (compareVersions(left, right) <= 0) { + console.error( + `::error::origin/dev carries ${left}, which does not outrank ${right}. Run the dev pre-move before releasing.`, + ); + process.exit(1); + } + process.exit(0); + } + + if (command === "assert-releasable") { + const [candidate, ...flags] = rest; + if (!candidate) { + console.error(VERSION_LINE_USAGE); + process.exit(1); + } + const tags = (await Bun.stdin.text()) + .split("\n") + .map(line => line.trim()) + .filter(Boolean); + const verdict = assertReleasable({ + candidate, + tags, + allowExistingTagAtHead: flags.includes("--allow-existing-tag-at-head"), + }); + if (!verdict.ok) { + console.error( + `::error::${candidate} does not outrank the current tag set (blocked by ${verdict.blockedBy}). Opening a preview for a higher core closes older stable patch lines — see devlog/_plan/260904_release_version_line/020 §4.0.`, + ); + process.exit(1); + } + process.exit(0); + } + + console.error(VERSION_LINE_USAGE); + process.exit(1); +} diff --git a/tests/bump-dev-version.test.ts b/tests/bump-dev-version.test.ts index 79d5388496..b01d0bb504 100644 --- a/tests/bump-dev-version.test.ts +++ b/tests/bump-dev-version.test.ts @@ -4,9 +4,10 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { decideDevVersion } from "../scripts/bump-dev-version"; +import { assertReleasable } from "../scripts/version-line"; /** - * The bump rule that keeps dev off an already-published version. + * The bump rule that moves dev ahead of an intended or already-published version. * * Every case here is a real repair this repository performed by hand. The rule was got * wrong once during design - "increment the released minor" - and befcac3e1 is the @@ -17,12 +18,30 @@ import { decideDevVersion } from "../scripts/bump-dev-version"; // which bun cannot open, so every CLI case exited 1 before reaching the code under test — // and the malformed-input case read that same load failure as a correct rejection. const CLI = fileURLToPath(new URL("../scripts/bump-dev-version.ts", import.meta.url)); +const VERSION_LINE_CLI = fileURLToPath(new URL("../scripts/version-line.ts", import.meta.url)); function runCli(...args: string[]) { const proc = Bun.spawnSync([process.execPath, CLI, ...args]); return { ...proc, stderrText: new TextDecoder().decode(proc.stderr) }; } +async function runVersionLineCli(args: string[], stdin = "") { + const proc = Bun.spawn([process.execPath, VERSION_LINE_CLI, ...args], { + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + const stdoutPromise = new Response(proc.stdout).text(); + const stderrPromise = new Response(proc.stderr).text(); + proc.stdin.write(stdin); + proc.stdin.end(); + return { + exitCode: await proc.exited, + stdoutText: await stdoutPromise, + stderrText: await stderrPromise, + }; +} + function tempPackageJson(version: string): string { const dir = mkdtempSync(join(tmpdir(), "ocx-bump-")); const path = join(dir, "package.json"); @@ -41,6 +60,17 @@ function tempPackageJson(version: string): string { } describe("dev version bump rule", () => { + test("an intended release uses the same shape rule before publication", () => { + expect(decideDevVersion("2.42.0", "2.42.0")).toMatchObject({ + changed: true, + version: "2.43.0", + }); + expect(decideDevVersion("2.43.0-preview.20260904", "2.42.0")).toMatchObject({ + changed: true, + version: "2.43.0", + }); + }); + test("a stable release moves dev to the next minor", () => { // e4a85d134 (2.33.0 -> 2.34.0) and 076ad3036 (2.34.0 -> 2.35.0). expect(decideDevVersion("2.36.0", "2.36.0")).toMatchObject({ changed: true, version: "2.37.0" }); @@ -70,7 +100,7 @@ describe("dev version bump rule", () => { }); test("a v-prefixed release tag is accepted, not double-prefixed", () => { - // The workflow passes github.event.release.tag_name, which is "v2.36.0", while + // The workflow accepts an intended version with an optional leading v, while // package.json holds a bare "2.36.0". Prefixing blindly built "vv2.36.0" and the // comparison silently misordered, so the script rejected a correct candidate with // "candidate 2.37.0 does not rank ahead of released v2.36.0". Both forms must agree. @@ -95,6 +125,68 @@ describe("dev version bump rule", () => { expect(() => decideDevVersion("2.36.0", "garbage")).toThrow(/not parseable/); }); + test("release ordering refuses a patch after a higher-core preview opens", () => { + expect(assertReleasable({ + candidate: "2.42.1", + tags: ["v2.42.0"], + })).toEqual({ ok: true }); + expect(assertReleasable({ + candidate: "2.42.1", + tags: ["v2.42.0", "v2.43.0-preview.1"], + })).toEqual({ ok: false, blockedBy: "v2.43.0-preview.1" }); + }); + + test("release ordering preserves only the explicit equal-tag dry-run exception", () => { + expect(assertReleasable({ + candidate: "2.42.0", + tags: ["v2.42.0"], + })).toEqual({ ok: false, blockedBy: "v2.42.0" }); + expect(assertReleasable({ + candidate: "2.42.0", + tags: ["v2.42.0"], + allowExistingTagAtHead: true, + })).toEqual({ ok: true }); + expect(assertReleasable({ + candidate: "2.42.0", + tags: ["v2.42.0", "v2.43.0-preview.1"], + allowExistingTagAtHead: true, + })).toEqual({ ok: false, blockedBy: "v2.43.0-preview.1" }); + }); + + test("the version-line CLI wires both gates, stdin tags, and usage failures", async () => { + const ahead = await runVersionLineCli(["assert-ahead", "2.43.0", "2.42.0"]); + expect(ahead.exitCode, ahead.stderrText).toBe(0); + + const behind = await runVersionLineCli(["assert-ahead", "2.42.0", "2.42.0"]); + expect(behind.exitCode).not.toBe(0); + expect(behind.stderrText).toContain("does not outrank 2.42.0"); + + const releasable = await runVersionLineCli( + ["assert-releasable", "2.42.1"], + "v2.42.0\n", + ); + expect(releasable.exitCode, releasable.stderrText).toBe(0); + + const blocked = await runVersionLineCli( + ["assert-releasable", "2.42.1"], + "v2.42.0\nv2.43.0-preview.1\n", + ); + expect(blocked.exitCode).not.toBe(0); + expect(blocked.stderrText).toContain("blocked by v2.43.0-preview.1"); + + const allowedEqual = await runVersionLineCli( + ["assert-releasable", "2.42.0", "--allow-existing-tag-at-head"], + "v2.42.0\n", + ); + expect(allowedEqual.exitCode, allowedEqual.stderrText).toBe(0); + + const unknown = await runVersionLineCli(["nonsense"]); + expect(unknown.exitCode).not.toBe(0); + expect(unknown.stderrText).toContain( + "usage: bun scripts/version-line.ts assert-ahead | assert-releasable [--allow-existing-tag-at-head]", + ); + }); + test("the CLI rewrites only the version line", () => { const path = tempPackageJson("2.36.0"); const before = readFileSync(path, "utf8"); diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index d282349d19..03beb2ba99 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -633,6 +633,93 @@ describe("GitHub Actions hardening", () => { expect(workflow).not.toMatch(/uses:\s+\S+@(?:v\d+|main|master)\b/); }); + test("dev version bump is a default-ref pre-move opener with one normalized target", async () => { + const text = await readText(".github/workflows/dev-version-bump.yml"); + const workflow = Bun.YAML.parse(text) as { + on?: { + workflow_dispatch?: { + inputs?: Record; + }; + workflow_call?: unknown; + }; + jobs?: { + "open-bump-pr"?: { + steps?: Array<{ + name?: string; + id?: string; + if?: string; + env?: Record; + run?: string; + }>; + }; + }; + }; + + expect(workflow.on?.workflow_call).toBeUndefined(); + expect(Object.keys(workflow.on ?? {})).toEqual(["workflow_dispatch"]); + const inputs = workflow.on?.workflow_dispatch?.inputs ?? {}; + expect(inputs["intended-version"]).toMatchObject({ required: true, type: "string" }); + expect(inputs.mode).toMatchObject({ + required: false, + default: "pre-move", + type: "choice", + options: ["pre-move", "repair"], + }); + + const steps = workflow.jobs?.["open-bump-pr"]?.steps ?? []; + const refGuard = steps.find(step => step.name === "Refuse a dispatch from a non-default ref"); + expect(refGuard?.run).toContain( + 'test "$GITHUB_REF" = "refs/heads/${{ github.event.repository.default_branch }}"', + ); + + const target = steps.find(step => step.name === "Resolve the target version"); + const decision = steps.find(step => step.name === "Decide the version dev should carry"); + const targetFreeness = steps.find( + step => step.name === "Prove the intended version is not already released", + ); + const chosenFreeness = steps.find(step => step.name === "Prove the chosen version is unused"); + const openPr = steps.find(step => step.name === "Open the bump pull request"); + + expect(target?.id).toBe("target"); + expect(target?.env).toEqual({ + INTENDED: "${{ inputs.intended-version }}", + MODE: "${{ inputs.mode }}", + }); + expect(target?.run).toContain('echo "version=${target}" >> "$GITHUB_OUTPUT"'); + expect(target?.run).toContain('echo "mode=repair" >> "$GITHUB_OUTPUT"'); + expect(target?.run).toContain('echo "mode=pre-move" >> "$GITHUB_OUTPUT"'); + expect(text.indexOf("- name: Resolve the target version")).toBeLessThan( + text.indexOf("- name: Decide the version dev should carry"), + ); + + expect(decision?.env?.RELEASED_VERSION).toBe("${{ steps.target.outputs.version }}"); + expect(targetFreeness?.if).toBe("${{ steps.target.outputs.mode == 'pre-move' }}"); + expect(targetFreeness?.env?.INTENDED).toBe("${{ steps.target.outputs.version }}"); + expect(targetFreeness?.run).toContain("git fetch --force --tags origin"); + expect(targetFreeness?.run).toContain('npm view "@bitkyc08/opencodex@${INTENDED#v}" version'); + expect(chosenFreeness?.run).toBe("bun test tests/release-version-line.test.ts"); + expect(openPr?.env).toMatchObject({ + MODE: "${{ steps.target.outputs.mode }}", + TARGET_VERSION: "${{ steps.target.outputs.version }}", + }); + expect(openPr?.run).toContain( + 'chore(release): open dev at ${NEXT_VERSION} before releasing ${TARGET_VERSION}', + ); + expect(openPr?.run).toContain( + 'fix(release): move dev to ${NEXT_VERSION} after ${TARGET_VERSION}', + ); + + // The resolver is the sole raw-input boundary. Every consumer after it reads the + // normalized output, so a future input rename cannot split the decision from its PR. + expect(count(text, "${{ inputs.intended-version }}")).toBe(1); + expect(count(text, "${{ inputs.mode }}")).toBe(1); + }); + test("release workflow gates the exact SHA, channel, and service surface without injection", async () => { const workflow = await readText(".github/workflows/release.yml"); const release = Bun.YAML.parse(workflow) as { @@ -667,6 +754,8 @@ describe("GitHub Actions hardening", () => { "pull-requests": "read", "id-token": "write", }); + expect(workflow).not.toContain("bump-dev-version:"); + expect(workflow).not.toContain("uses: ./.github/workflows/dev-version-bump.yml"); expect(workflow).toContain("actions: read"); expect(workflow).toContain("pull-requests: read"); expect(workflow).toContain("id-token: write"); @@ -781,6 +870,44 @@ describe("GitHub Actions hardening", () => { expect(workflow).toContain("main releases must use a stable semver version"); expect(workflow).toContain("preview releases must use a preview prerelease version"); + const readinessStep = workflow + .split("- name: Require dev to be ready for this release")[1] + ?.split(/\n {6}- name:/)[0]; + expect(readinessStep).toBeDefined(); + expect(readinessStep).toContain( + "git fetch --force --tags origin +refs/heads/dev:refs/remotes/origin/dev", + ); + expect(readinessStep).toContain("git show origin/dev:package.json"); + expect(readinessStep).toContain( + 'bun scripts/version-line.ts assert-ahead "$dev_version" "$RELEASE_VERSION"', + ); + + const orderingStep = workflow + .split("- name: Refuse a release the current tag set already outranks")[1] + ?.split(/\n {6}- name:/)[0]; + expect(orderingStep).toBeDefined(); + expect(orderingStep).toContain( + 'git tag --list \'v*\' | bun scripts/version-line.ts assert-releasable "$RELEASE_VERSION" $allow', + ); + expect(orderingStep).toContain('existing_tag_sha="$(git rev-parse'); + expect(orderingStep).toContain('[ "$DRY_RUN" = "true" ]'); + expect(orderingStep).toContain('[ "$existing_tag_sha" = "$GITHUB_SHA" ]'); + + // This is an ordering gate, not an existence pin. It must consume the freshly + // fetched tag set and must run before either dry-run packing or publication. + const preflightIndex = workflow.indexOf("- name: Preflight release metadata"); + const preflightFetchIndex = workflow.indexOf( + "git fetch --force --tags origin", + preflightIndex, + ); + const orderingGateIndex = workflow.indexOf( + "- name: Refuse a release the current tag set already outranks", + ); + const publishBoundaryIndex = workflow.indexOf("- name: Publish (or dry-run)"); + expect(preflightFetchIndex).toBeGreaterThan(preflightIndex); + expect(orderingGateIndex).toBeGreaterThan(preflightFetchIndex); + expect(publishBoundaryIndex).toBeGreaterThan(orderingGateIndex); + // Release notes are built and coverage-validated before npm publish. The // builder owns Git-history/PR coverage; the workflow only wires the validated // artifact into the release. Stable/preview range semantics are unit-tested in diff --git a/tests/release-helper.test.ts b/tests/release-helper.test.ts index c692a80f58..d441e0cd49 100644 --- a/tests/release-helper.test.ts +++ b/tests/release-helper.test.ts @@ -24,6 +24,7 @@ const sshTarget = `${"git"}@${"github.com"}:lidge-jun/opencodex.git`; interface ReleaseScenario { branch?: string; + gitTags?: string[]; npmLatest?: string; npmPreview?: string; headSha?: string; @@ -129,6 +130,11 @@ if (args[0] === "status" && args[1] === "--porcelain") { process.exit(0); } +if (args[0] === "tag" && args[1] === "--list" && args[2] === "v*") { + stdout((process.env.FAKE_GIT_TAGS ?? "") + "\\n"); + process.exit(0); +} + if (args[0] === "ls-remote") { if (args.some(a => typeof a === "string" && a.startsWith("refs/heads/"))) { const branchRef = args.find(a => typeof a === "string" && a.startsWith("refs/heads/")); @@ -248,7 +254,7 @@ function findCallIndex(calls: LoggedCall[], name: string, matcher: (call: Logged return calls.findIndex(call => call.name === name && matcher(call)); } -async function runRelease(version: string, scenario: ReleaseScenario = {}) { +async function runRelease(releaseArgs: string | string[], scenario: ReleaseScenario = {}) { const shimDir = mkdtempSync(join(tmpdir(), "ocx-release-helper-")); const logPath = join(shimDir, "release-log.jsonl"); writeFileSync(logPath, "", "utf8"); @@ -279,6 +285,7 @@ async function runRelease(version: string, scenario: ReleaseScenario = {}) { [pathKey]: pathValue, FAKE_RELEASE_LOG: logPath, FAKE_GIT_BRANCH: scenario.branch ?? "main", + FAKE_GIT_TAGS: (scenario.gitTags ?? []).join("\n"), FAKE_GIT_HEAD_SHA: scenario.headSha ?? "abc123def456", ...(scenario.remoteHeadSha ? { FAKE_GIT_REMOTE_HEAD_SHA: scenario.remoteHeadSha } : {}), FAKE_BUN_TSC_EXIT_CODE: String(scenario.typecheckExitCode ?? 0), @@ -292,10 +299,14 @@ async function runRelease(version: string, scenario: ReleaseScenario = {}) { ...(scenario.originUrl ? { FAKE_GIT_ORIGIN_URL: scenario.originUrl } : {}), }; try { - const result = await runCaptured(process.execPath, [releaseScriptPath, version], { + const result = await runCaptured( + process.execPath, + [releaseScriptPath, ...(typeof releaseArgs === "string" ? [releaseArgs] : releaseArgs)], + { cwd: repoRoot, env, - }); + }, + ); return { calls: readLoggedCalls(logPath), result }; } finally { removeTreeWithRetry(shimDir); @@ -351,6 +362,74 @@ process.exit(0); } describe("release helper", () => { + test("--bump minor resolves from latest and dispatches the resolved version", async () => { + const { calls, result } = await runRelease(["--bump", "minor"], { npmLatest: "9.9.9" }); + + expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); + expect(findCallIndex(calls, "npm", call => + call.args.join(" ") === "version 9.10.0 --no-git-tag-version", + )).toBeGreaterThanOrEqual(0); + expect(findCallIndex(calls, "gh", call => + call.args[0] === "workflow" + && call.args[1] === "run" + && call.args.includes("version=9.10.0"), + )).toBeGreaterThanOrEqual(0); + }); + + test("--bump and an explicit version are rejected before any command runs", async () => { + const { calls, result } = await runRelease(["9.9.9", "--bump", "minor"]); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toMatch(/mutually exclusive|exactly one/i); + expect(calls).toEqual([]); + }); + + test("an invalid --bump kind is rejected before any command runs", async () => { + const { calls, result } = await runRelease(["--bump", "banana"]); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toContain("patch|minor|major"); + expect(calls).toEqual([]); + }); + + test("--bump consults stable tags as well as the latest channel", async () => { + const { calls, result } = await runRelease(["--bump", "patch"], { + npmLatest: "9.9.0", + gitTags: ["v9.9.5"], + }); + + expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); + expect(findCallIndex(calls, "npm", call => + call.args.join(" ") === "version 9.9.6 --no-git-tag-version", + )).toBeGreaterThanOrEqual(0); + }); + + test("--bump on preview emits a dated preview version", async () => { + const { calls, result } = await runRelease(["--bump", "minor"], { + branch: "preview", + npmLatest: "9.9.9", + npmPreview: "9.9.9-preview.20260903", + }); + + expect(`${result.status}\n${result.stderr ?? ""}`.trim()).toBe("0"); + const versionCall = calls.find(call => call.name === "npm" && call.args[0] === "version"); + expect(versionCall?.args[1]).toMatch(/^\d+\.\d+\.\d+-preview\.\d{8}(?:\.\d+)?$/); + }); + + test("a higher-core preview refusal reaches the operator before bump or commit", async () => { + const blockingPreview = "v9.10.0-preview.1"; + const { calls, result } = await runRelease(["--bump", "patch"], { + npmLatest: "9.9.0", + gitTags: [blockingPreview], + }); + + expect(result.status).not.toBe(0); + expect(result.stderr + result.stdout).toContain("cannot bump stable patch"); + expect(result.stderr + result.stdout).toContain(blockingPreview); + expect(findCallIndex(calls, "npm", call => call.args[0] === "version")).toBe(-1); + expect(findCallIndex(calls, "git", call => call.args[0] === "commit")).toBe(-1); + }); + test("preflight runs the shared audit, typecheck, test suite, and privacy scan before version bump", async () => { const { calls, result } = await runRelease("9.9.9"); diff --git a/tests/version-line.test.ts b/tests/version-line.test.ts index 0b704e58ba..000c9fec75 100644 --- a/tests/version-line.test.ts +++ b/tests/version-line.test.ts @@ -3,6 +3,8 @@ import { compareTagsLenient, compareVersions, nextDevelopmentVersion, + nextPreviewRelease, + nextStableRelease, parseVersion, } from "../scripts/version-line"; @@ -65,4 +67,141 @@ describe("version line algebra", () => { expect(() => nextDevelopmentVersion("2.36")).toThrow(/not parseable/); expect(() => nextDevelopmentVersion("garbage")).toThrow(/not parseable/); }); + + test("a future same-core preview does not raise the stable bump base", () => { + expect(nextStableRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTags: ["v2.43.0-preview.1"], + })).toBe("2.43.0"); + }); + + test("refuses a stable patch below an open higher-core preview", () => { + expect(() => nextStableRelease({ + kind: "patch", + stableTip: "2.42.0", + stableTags: [], + previewTags: ["v2.43.0-preview.1"], + })).toThrow(/cannot bump stable patch.*v2\.43\.0-preview\.1/); + }); + + test("allows a stable patch when no higher-core preview is open", () => { + expect(nextStableRelease({ + kind: "patch", + stableTip: "2.42.0", + stableTags: [], + previewTags: ["v2.42.0-preview.9"], + })).toBe("2.42.1"); + }); + + test("starts the next preview core above the latest stable", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: null, + previewTags: [], + stamp: "20260904", + })).toBe("2.43.0-preview.20260904"); + }); + + test("adds an ordinal when the same-core preview stamp already exists", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: null, + previewTags: ["v2.43.0-preview.20260904"], + stamp: "20260904", + })).toBe("2.43.0-preview.20260904.2"); + }); + + test("honours the preview bump kind when resolving its core", () => { + expect(nextPreviewRelease({ + kind: "major", + stableTip: "2.42.0", + stableTags: [], + previewTip: null, + previewTags: [], + stamp: "20260904", + })).toBe("3.0.0-preview.20260904"); + }); + + test("continues the ordinal from the incumbent", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: null, + previewTags: ["v2.43.0-preview.20260904.3"], + stamp: "20260904", + })).toBe("2.43.0-preview.20260904.4"); + }); + + test("uses an equal-stamp npm preview tip as the incumbent", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: "2.43.0-preview.20260910", + previewTags: [], + stamp: "20260910", + })).toBe("2.43.0-preview.20260910.2"); + }); + + test("uses an equal-stamp preview tag as the incumbent when the npm tip is behind", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: "2.40.0-preview.20260902", + previewTags: ["v2.43.0-preview.20260910"], + stamp: "20260910", + })).toBe("2.43.0-preview.20260910.2"); + }); + + test("refuses a preview stamp older than the incumbent stamp", () => { + expect(() => nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: "2.43.0-preview.20260910", + previewTags: [], + stamp: "20260904", + })).toThrow(/20260904.*20260910/); + }); + + test("uses stable tags rather than the preview channel to resolve the preview core", () => { + expect(nextPreviewRelease({ + kind: "minor", + stableTip: "2.40.0", + stableTags: ["v2.42.0"], + previewTip: "2.40.0-preview.20260902", + previewTags: [], + stamp: "20260904", + })).toBe("2.43.0-preview.20260904"); + }); + + test("promotes a same-core preview to the intended stable version", () => { + expect(nextStableRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTags: ["v2.43.0-preview.20260904"], + })).toBe("2.43.0"); + }); + + test("a preview successor strictly outranks its incumbent", () => { + const incumbent = "v2.43.0-preview.20260904.3"; + const result = nextPreviewRelease({ + kind: "minor", + stableTip: "2.42.0", + stableTags: [], + previewTip: null, + previewTags: [incumbent], + stamp: "20260904", + }); + expect(compareVersions(result, incumbent)).toBeGreaterThan(0); + }); }); From 029ae1ee64ed46d9853c73204e304c6a6a8958ff Mon Sep 17 00:00:00 2001 From: jun Date: Sat, 5 Sep 2026 00:02:00 +0900 Subject: [PATCH 6/6] docs(release): correct the release order and record the closed-patch-line policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 040 of devlog/_plan/260904_release_version_line/. Documentation only; no assertion, script or workflow changes. MAINTAINERS.md told maintainers to move dev's version line while CLOSING a release. Done at closing time it is always too late, and that instruction is the cause of the recurrence it warns about — four hand repairs, and a detector that did not stop two more. It now says the opposite: opening a release STARTS by moving dev forward, dev must already outrank the version being released, and release.yml refuses to publish otherwise. The historical repair record stays, because it is why the rule exists. The SoT gains the policy the code now enforces: publishing a preview for a higher core ends the current stable patch line, and nextStableRelease refuses such a patch bump. This is a deliberate restriction, not the preservation of an unused capability — history contains real counterexamples (v2.6.24-preview.20260705 then v2.6.23, v2.7.39-preview.20260724 then v2.7.37), and 103 of 143 stable tags carry patch > 0. Recording it as policy is what keeps a future reader from re-deriving that as a bug. tests/release-version-line.test.ts gains two comment lines and nothing else. Its assertions are byte-identical and tagPointsAtHead is retained: the release commit still equals its own tag. An earlier draft proposed asserting compareReleaseTags("v2.42.0", "v2.42.0") === 0 — that is tautological, exercises the comparator rather than the exception, and would pass against a build that deleted the exception entirely. It was rejected in audit and is not here. Verified by diff inspection rather than execution: local test and typecheck runs are prohibited for this work, and the change set is being verified on CI instead. --- MAINTAINERS.md | 37 ++++++++++++++++------------ structure/06_docs-and-release.md | 39 ++++++++++++++++++++++++------ tests/release-version-line.test.ts | 2 ++ 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/MAINTAINERS.md b/MAINTAINERS.md index f7183db6ef..5787d36931 100644 --- a/MAINTAINERS.md +++ b/MAINTAINERS.md @@ -73,21 +73,28 @@ when a maintainer steps down. - Direct pushes are reserved for maintainer-owned integration work, urgent repairs, or incident recovery. The same CI and documentation requirements still apply. - Promotion from `dev` to `main` and npm releases is maintainer-controlled. -- **Closing out a release includes moving `dev`'s version line forward.** A published - release leaves `dev` carrying a version at or behind it, and - `tests/release-version-line.test.ts` then fails on `dev` and on every pull request - opened against it — red that contributors inherit and cannot fix from their own diff. - This was repaired by hand four times (`32529c2b2`, `e4a85d134`, `076ad3036`, - `befcac3e1`) before it was automated. - - `.github/workflows/dev-version-bump.yml` now opens that bump as a pull request when a - release publishes. Merging it is part of closing the release; a bot cannot, because - `Protect dev` requires an approving review and code-owner sign-off. Two caveats worth - knowing: the workflow runs from the DEFAULT branch, so it only fires once it has been - promoted to `main`; and a pull request opened with `GITHUB_TOKEN` does not start - `pull_request` workflows, so the bump pull request arrives without CI. To re-drive a - missed run by hand: `bun scripts/bump-dev-version.ts package.json`, - then open the pull request normally. +- **Opening a release starts by moving `dev`'s version line forward.** Before cutting + a release, `dev` must already outrank the version being released; `release.yml` + asserts this and refuses to publish otherwise. Dispatch + `.github/workflows/dev-version-bump.yml` with the intended version, merge the pull + request it opens, then promote and release. When `dev` already outranks the target + — a preview cut, or a stable hotfix below `dev`'s line — no move is needed and the + workflow reports `changed=false`. + + Opening a preview for the next core ends the current patch line. After + `vX.Y.0-preview.*` is tagged, a fix ships as part of `X.Y.0`, not as + `X.(Y-1).(Z+1)`. The release helper refuses such a bump rather than producing a + version the repository would reject. This is a deliberate policy restriction, not + a claim that lower stable patches were historically unused. + + Done after the publish, as this repository did for ten releases (`32529c2b2`, + `e4a85d134`, `076ad3036`, `befcac3e1`, then #3045, #3076, #3127, #3265, #3354, + #3434), it leaves `dev` and every open pull request carrying a failure contributors + cannot fix from their own diff. The pull request itself does not go away — `Protect + dev` requires a reviewed merge. If the pre-move is missed and publication somehow + succeeds, dispatch `dev-version-bump.yml` from the default branch with the released + version and `mode=repair`, then merge the repair pull request. Design: + `devlog/_plan/260904_release_version_line/`. ## The retired `dev2-go` line diff --git a/structure/06_docs-and-release.md b/structure/06_docs-and-release.md index 6305785c38..1c76ef7a8e 100644 --- a/structure/06_docs-and-release.md +++ b/structure/06_docs-and-release.md @@ -69,7 +69,8 @@ authenticated catalog access, and a real routed response themselves. | Workflow | Trigger | Purpose | | --- | --- | --- | | `.github/workflows/ci.yml` | `pull_request` to `main`/`dev`, `push` to `main`/`preview`/`dev`, or manual dispatch when runtime/package paths change | Cross-platform runtime/package quality gate. Linux runs the suite as four parallel shards (`test 1/4`–`4/4`) plus a consolidated `gates` job; macOS runs the full suite. Windows runs the full suite only on a `push` to `main`/`preview` or a manual dispatch — it is the shipping boundary, not the pull-request lane, because it was last to finish in every sampled run at roughly three times the Linux median. The aggregate `ci` job asserts `platform-windows` actually succeeded on those boundary events rather than accepting a skip. `npm-global-smoke` always remains GitHub-hosted because it mutates the global package prefix. | -| `.github/workflows/release.yml` | Manual dispatch only | npm publish/dry-run workflow. It requires the exact `GITHUB_SHA` to have a successful Cross-platform CI run before publish or dry-run. | +| `.github/workflows/dev-version-bump.yml` | Manual dispatch with an intended version and `pre-move` or `repair` mode | Opens the reviewed pull request that moves `dev` past a release target. The default `pre-move` mode runs before promotion and publication; explicit `repair` mode retains the post-publish catch-up path. It is neither called by `release.yml` nor triggered by publication. | +| `.github/workflows/release.yml` | Manual dispatch only | npm publish/dry-run workflow. It requires successful Cross-platform CI for the exact `GITHUB_SHA`, requires `dev` to outrank the target, then checks the target against the freshly fetched global tag set before publish or dry-run. | | `.github/workflows/deploy-docs.yml` | `push` to `main` touching `docs-site/**` or the workflow, or manual dispatch | Build and publish the Astro/Starlight docs site to GitHub Pages. | | `.github/workflows/service-lifecycle.yml` | `pull_request` to `main`/`dev` and `push`, both filtered on the service path set (`src/service.ts`, `src/cli.ts`, `src/cli/index.ts`, `src/lib/bun-runtime.ts`, `package.json`, `bun.lock`, the workflow), or manual dispatch | Service-lifecycle smoke on three platforms: Linux systemd, macOS launchd, and Windows Scheduled Tasks. Each installs, verifies, stops via `ocx stop`, and uninstalls. The path list is kept in sync with the `release.yml` service-gate regex. | | `.github/workflows/enforce-pr-target.yml` | `pull_request_target` (opened, reopened, edited, labeled, unlabeled, ready_for_review, synchronize) plus default-branch `status` events filtered to successful `CodeRabbit` statuses | The `enforce-target` gate: rejects pull requests whose head ancestry sits on the `main` tip while far behind `dev`, rejects empty or malformed descriptions, requires a GUI screenshot when the title/body mentions `gui` (immediately waivable with the maintainer-controlled `gui-screenshot-waived` label; legacy maintainer comments remain compatibility evidence on later PR events), keeps contributor PRs in draft until a four-box readiness checklist is complete, verifies the CI / latest-dev / Codex+CodeRabbit-findings claims (review threads plus current-head CodeRabbit review-body findings outside the diff range), and adds a `review-ready` status label at the ready moment. CodeRabbit status SHAs must resolve to exactly one open current-head PR before writes. Stacked child PRs targeting another open PR's head skip the wrong-base gate. | @@ -178,10 +179,27 @@ Invariants: ## Release workflow Package release is npm-focused. `package.json` exposes `opencodex` and `ocx`, `prepublishOnly` runs -typecheck and GUI build, and `scripts/release.ts` now runs local typecheck, `bun test --isolate tests`, and +typecheck and GUI build. `scripts/release.ts` accepts either an explicit version or +`--bump patch|minor|major`; the stable and preview channels use separate resolvers in +`scripts/version-line.ts`. It runs local typecheck, `bun test --isolate tests`, and `bun run privacy:scan` before the version bump, commit/push, Cross-platform CI wait, and GitHub Release workflow dispatch. Docs publishing is separate from npm release publishing. +Opening a release starts with the `dev` pre-move. Dispatch +`.github/workflows/dev-version-bump.yml` with the intended version, merge the pull request it opens, +then promote and release. A no-op is valid when `dev` already outranks the target. `release.yml` +independently enforces that readiness condition and refuses publication if the pre-move is missing. +The design and repair history live in `devlog/_plan/260904_release_version_line/`. + +Opening a preview for the next core ends the current patch line. After +`vX.Y.0-preview.*` is tagged, a fix ships as part of `X.Y.0`, not as +`X.(Y-1).(Z+1)`. `nextStableRelease` refuses such a patch bump, and the release workflow's global +ordering gate prevents an explicit lower version from bypassing the resolver. This is a deliberate +policy restriction, not preservation of an unused capability: at the design audit, 103 of 143 stable +tags had `patch > 0`, and history includes `v2.6.24-preview.20260705` followed by `v2.6.23` and +`v2.7.39-preview.20260724` followed by `v2.7.37`. Reopening parallel patch lines would require a +separate channel-aware invariant and release-note baseline design. + ### Release notes Release notes are rendered OpenAI-Codex-style by `scripts/release-notes.ts render` inside @@ -223,6 +241,11 @@ The release must fail before `npm publish` if npm, the Git tag, or the GitHub Re requested version. This prevents partial releases where npm is published but GitHub Release creation fails afterward. +Two ordering checks run before publication. The version on `origin/dev` must strictly outrank the +release target, proving the pre-move has landed. After a fresh tag fetch, the release target must also +outrank the global release-tag set. The only equality exception is a dry run whose existing tag points +at the exact `GITHUB_SHA`; a real publish never receives that exception. + Do not force-move public version tags by default. If release metadata is already inconsistent, treat the version as consumed and publish the next unused patch version instead. Only rewrite a public tag after an explicit human decision that the public history rewrite is acceptable. @@ -236,8 +259,9 @@ gh release view v ``` If any of these commands reports an existing artifact for the requested version, stop before -publishing. For a non-destructive recovery, choose the next unused patch version and release that -version through `scripts/release.ts`. +publishing. For a non-destructive recovery, choose the next unused version that also outranks the +global tag set and release it through `scripts/release.ts`. A patch is not available once a higher-core +preview has closed that stable patch line. ## Cross-platform CI @@ -269,9 +293,10 @@ The CI intentionally does not build docs, run coverage, or perform remote Ubuntu Those stay outside the default gate until a concrete regression justifies the extra runtime. The Release workflow remains manual and publish-focused. Before any dry-run or publish step, it -checks that the exact release commit (`GITHUB_SHA`) already has a successful Cross-platform CI run. -This keeps release runs short and makes release a deployment of a verified commit rather than a -second CI pipeline. +checks that the exact release commit (`GITHUB_SHA`) already has a successful Cross-platform CI run, +that `dev` already outranks the target, and that the target passes the fresh global tag-ordering gate. +This keeps release runs short and makes release a deployment of a verified commit after the required +`dev` pre-move rather than a second CI pipeline. ## Remote Hub locale and release gate diff --git a/tests/release-version-line.test.ts b/tests/release-version-line.test.ts index 92cd0bc9ba..9ef257dfd6 100644 --- a/tests/release-version-line.test.ts +++ b/tests/release-version-line.test.ts @@ -23,6 +23,8 @@ const repoRoot = fileURLToPath(new URL("../", import.meta.url)); * Commit 32529c2b2 repaired precisely this by hand once, and nothing has enforced it * since. The assertion reads the local tag set rather than the npm registry, so it needs * no network and no edit at each release. + * The durable repair now moves `dev` forward before the release instead of catching it + * up afterward. * * compareReleaseTags comes from scripts/release-notes and not from scripts/release: the * latter parses process.argv and calls process.exit at module scope, so importing it from