-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: complete Cline registration and stabilize integration fixtures #4390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
6942594
6cda25f
657e068
d8335f7
c43eebf
f0c880f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # Campaign integration fixture repairs | ||
|
|
||
| Hosted CI exposed incomplete Cline registration follow-through and restore fixtures that no longer exercise the documented atomic refusal contract. This change repairs those contracts without changing credential or restore behavior. | ||
|
|
||
| - Trigger/evidence: Cross-platform CI run 34676570087, head 954b1da7804110e440acc9d244e70f32f2aa9aae. Linux, macOS and dashboard gate failures are retained as the failing baseline evidence; no local reproduction is claimed. | ||
| - Cline: correct the lightweight CLI count to fifteen, preserve exact registry equality, recognize only the Cline product-name keys as intentional English, document reuse of its existing mark, and align client/writer test seeds with their committed domain. | ||
| - Restore: assert unsuccessful all-skipped results and unchanged artifacts after refusal; retain exact pre-operation config/profile/journal snapshots when damaged defaults prevent restoration. Canonicalize temporary homes and target the production profile path so macOS fault injection and manifest lookup actually reach the intended boundary. Assert a matching injected read and default manifest visibility. | ||
| - Non-goals: no runtime restore/auth changes, test skips, weaker error/preservation assertions, new dependencies, local tests/build/typecheck/install, release or deployment. | ||
| - Verification: git diff --check for text; independent source review of Cline and restore slices; final-head hosted CI must execute the unchanged failure paths and pass before completion. Local product execution remains NOT RUN. | ||
| - Stop: the original named failures pass at the published final head and no new blocking finding remains. An unrelated CI failure is investigated separately, not waived here. | ||
|
|
||
| The shared baseline also includes the independently reviewed Combo reactivation correction from #4385. It explicitly runs the actual activation callback and preserves the cached quota evidence, dirty draft and Save-state assertions. This known scheduling defect must not remain in the baseline supplied to other campaign PRs. The #4385 source commit is preserved by merge; close that duplicate delivery only after this combined baseline lands. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Deterministic Combo reactivation expiry verification | ||
|
|
||
| Hosted run 34674763850, job 103503977506 failed the active reactivation case because Save remained disabled. The test did not explicitly execute the activation effect's zero-delay callback. | ||
|
|
||
| The fixture now captures cancellable immediate timers only during active and commit-boundary scenarios, commits inactive/active state synchronously to preserve the cached quota snapshot, requires one new callback and executes it inside act. The old expiry timer must be cancelled. Dirty alias preservation, initial disabled state, final enabled state, timer/visibility scenarios and cleanup assertions remain. Subsequent fetches stay unresolved so a new server response cannot satisfy the assertion. | ||
|
|
||
| Independent source review found no blocker and traced the callback to the active-dependent effect in Combos.tsx. This covers reactivation while the resource cache survives; it does not claim coverage after cache eviction. | ||
|
|
||
| Local tests, build, typecheck and installation: NOT RUN by maintainer instruction. git diff --check passes. Hosted CI at the published final head remains required before merge. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| # Release regression train | ||
|
|
||
| dev has drifted 79 commits past its last verified state and nothing has published since | ||
| 2.51.0. This unit takes dev back to a proved-green head, sweeps every commit in the | ||
| unverified window for regressions, and then promotes preview and main with real | ||
| push-event CI evidence behind each publish. | ||
|
|
||
| ## Baseline | ||
|
|
||
| - `main` `c155cc7923`, version 2.51.0. | ||
| - `preview` `69207f1b08`, version 2.52.0-preview.20260911. | ||
| - `dev` `c27a4831a9`, package.json 2.52.0, no CI run recorded for this exact head. | ||
| - Last `dev` run with `conclusion=success`: `e432cf565a` (2026-09-12 03:51 UTC). | ||
| - Unverified window `e432cf565a..dev`: 79 commits, 57 non-merge, 58 `src/` files, | ||
| +3616/-229 inside `src/` alone. | ||
| - Eleven dev push runs after that green ended `cancelled`, because `ci.yml` sets | ||
| `concurrency: cross-platform-ci-${{ github.ref }}` with `cancel-in-progress: true` | ||
| and the merges arrived faster than a run could finish. The twelfth, `f9815da21f` at | ||
| 08:15 UTC, was allowed to finish and concluded `failure`. dev is red, not merely | ||
| unverified, and the current head `c27a4831a9` has no run at all. | ||
| - `release.yml` refuses to publish a SHA without a successful push-event `ci.yml` | ||
| run on the release branch itself, so a red or unrun dev cannot reach a release. | ||
|
|
||
| ## Why the feature backlog stays out | ||
|
|
||
| Twenty-seven open 60plus PRs are implemented and reviewed but unlanded. Adding them now | ||
| would enlarge an already unverified window and make any regression bisect useless. | ||
| They ship in the next cycle. Only two classes of change enter dev here: the shared | ||
| CI repair in #4390, and fixes for regressions this sweep actually finds. | ||
|
|
||
| ## Cycles | ||
|
|
||
| | Work phase | Document | Outcome | | ||
| |---|---|---| | ||
| | wp1 | this file | roadmap locked, later units pre-written | | ||
| | wp2 | 010_ci_repair.md | #4390 green, merged, dev push CI success | | ||
| | wp3 | 020_regression_sweep.md | every commit in the window reviewed | | ||
| | wp4 | 030_defect_landing.md | blocking findings fixed and re-greened | | ||
| | wp5 | 040_preview_release.md | preview promoted and published | | ||
| | wp6 | 050_main_release.md | main promoted and published | | ||
|
|
||
| ## Constraints carried from the campaign | ||
|
|
||
| Local product tests, builds, typecheck, and installs are not run. Verification is | ||
| remote CI only and local checks are reported as NOT RUN. Pushes use `--no-verify`. | ||
| Subagents are read-only verifiers dispatched on `xai/grok-4.6` at the user's explicit | ||
| instruction; the stored subagent role configuration is left untouched. No account, | ||
| credential, or model settings change. Already merged PRs are never recreated or | ||
| reverted. | ||
|
|
||
| ## Stop conditions | ||
|
|
||
| Stop and escalate if the sweep finds a defect in authentication, credential handling, | ||
| or release automation. Stop if a policy gate outside maintainer control blocks the | ||
| same step three times. Hitting a stated resource bound is BUDGET_EXHAUSTED, not DONE. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # wp2 — CI repair and dev re-green | ||
|
|
||
| PR #4390 (`codex/260912-finish-integration-fixtures`, head `d8335f75b4`) carries the | ||
| shared repair that makes the Cline registration and Codex restore fixtures pass again. | ||
| Twenty-one checks including `gates` pass on that head, which is what proves the Cline | ||
| failure class is resolved. Four report failure: `test 4/4`, `macos 2/2`, | ||
| `enforce-target`, and the `ci` aggregate that reflects the first two. | ||
|
|
||
| ## Failure 1 — `test 4/4` | ||
|
|
||
| `tests/codex-integration/codex-inject-integration.test.ts` receives `matchedReads: 0` | ||
| on Linux and on macOS alike, which rules out a path-string difference: the same file | ||
| fails as `test 4/4` and `macos 2/2` with identical output. | ||
|
|
||
| The interception never worked. `src/codex/inject-coordination.ts:8` binds | ||
| `readFileSync` as an ESM named import, and `readOrNull`/`captureCodexPreImages` read | ||
| `CODEX_PROFILE_PATH` through that binding, so `spyOn(fs, "readFileSync")` on the child's | ||
| `require("node:fs")` handle reaches nothing. Every assertion in that test therefore ran | ||
| against an undenied filesystem. The `matchedReads` counter added on this branch did not | ||
| create the defect; it made an already-false test visible. | ||
|
|
||
| That matters beyond this PR: the test arrived in #4342, which merged into dev with its | ||
| own CI red, so dev has carried the failure since. | ||
|
|
||
| Repair: drop the mock and make the profile genuinely unreadable with `chmod 0`, proving | ||
| the precondition through a new `unreadable` field before any later assertion depends on | ||
| it. The EACCES, outcomes, compensation, and preservation expectations are unchanged. | ||
| Windows chmod only toggles the read-only bit and root ignores mode 0, so both are | ||
| skipped. | ||
|
|
||
| ## Failure 2 — `enforce-target` | ||
|
|
||
| The description mentions `gui` and carries no UI screenshot, which the gate rejects. | ||
| Resolve it by supplying the screenshot when the change really touches the UI, or by | ||
| removing the incidental `gui` mention when it does not. Disguising the mention to slip | ||
| past a path-based gate is not an option. | ||
|
|
||
| ## Exit | ||
|
|
||
| Every required check green at the final head, merged to dev with maintainer-integration | ||
| recorded, ancestry and tree verified against fresh `origin/dev`, and one push-event | ||
| `ci.yml` run allowed to finish on that dev head with no competing merge. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # wp3 — regression sweep | ||
|
|
||
| The sweep covers `e432cf565a..dev`, the window that never had a finished CI run. It | ||
| also revisits the higher-risk subsystems across the full `main..dev` range, because | ||
| that whole delta is what a 2.51.0 user receives. | ||
|
|
||
| ## Method | ||
|
|
||
| Parallel read-only lanes on `xai/grok-4.6`. Each lane owns a disjoint read scope and | ||
| returns findings as exact `file:line` or SHA citations, separating confirmed facts | ||
| from suspicion. Lanes never write, never run local product commands, and never touch | ||
| git state. The main session synthesizes, accepts or rebuts each finding, and decides. | ||
|
|
||
| ## Risk ranking | ||
|
|
||
| Account pool and quota routing sit directly on the request path. Codex config injection | ||
| and restore write the user's own `config.toml`. Remote workspace and remote control add | ||
| new surface but default off behind `OCX_REMOTE_WORKSPACE_ENABLED`. Provider catalog, | ||
| integrations and clients change what the CLI exposes. GUI, docs and i18n are visible but | ||
| rarely release-blocking. | ||
|
|
||
| ## Exit | ||
|
|
||
| Every commit in the window assigned to at least one lane with a recorded verdict, and a | ||
| defect register that classifies each finding as release-blocking, user-visible minor, or | ||
| harmless. | ||
|
|
||
| ## Window composition | ||
|
|
||
| Of the 57 non-merge commits, 20 touch `src/`, 13 of those reach the user request path, and | ||
| the remaining 37 are GUI, tests, docs, or devlog. `src/router.ts` and | ||
| `src/server/lifecycle.ts` are untouched; the hub is `src/server/responses/core.ts`, which | ||
| six commits share. That file belongs to one lane alone so the read scopes stay disjoint. | ||
|
|
||
| ## Lanes | ||
|
|
||
| | Lane | Read scope | Commits | | ||
| |---|---|---| | ||
| | 1 | `src/codex/routing.ts`, `src/combos/resolve.ts`, `src/providers/quota*.ts`, `src/server/management/provider-routes.ts` | d42a1363dc (routing), f18541b8f5, 7418ef8eb2, 0bcb43e266 | | ||
| | 2 | `src/server/responses/core.ts`, `chat-completions.ts`, `claude-messages.ts`, `adapter-resolve.ts`, `src/responses/custom-tool-compat.ts`, `src/bridge.ts`, `src/vision/plan.ts`, `src/web-search/index.ts` | d42a1363dc (core), de2042628d, d608d7fb0a, 17b3d3fe99, d6723f7f3b, d27db6dd56, b09ef15c6f (partial), 843486a3b8, 8949fd073f | | ||
| | 3 | `src/claude/inbound*.ts`, `src/types/config.ts` | e114bc97b5, 954c1f28fc | | ||
| | 4 | `src/codex/inject.ts`, `inject-coordination.ts`, `history-provider.ts` | 7f76d736c2, 1338e96c10, be203dd4a4 | | ||
| | 5 | `src/oauth/**`, `src/adapters/**`, `src/providers/registry.ts`, devin-cli migration | fa4226a9ba, b09ef15c6f (partial), d6fb87197a, 96041e7833 | | ||
| | 6 | `src/remote-control/**`, `src/update/job.ts` | 71857fac92, 726ddc7fc0, f378947111, e090ad65cd | | ||
| | 7 | `src/integrations/**`, `src/clients/config-export*`, `src/cli/**`, management config/integration routes | 90975e9fea, 1feec1bdc0, 75a8ec8f78 | | ||
| | 8 | `gui/src/**`, `gui/tests/**` | catalog stack, Combo clock and draft, Cline dialog | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Add a lane for The sweep range contains docs/devlog commits, including 🤖 Prompt for AI Agents |
||
|
|
||
| Lane 2 runs first because everything else can then ignore the hub file. | ||
|
|
||
| ## Already on the register | ||
|
|
||
| Lane 4's own fixture was the first finding, and it came from CI rather than the sweep: | ||
| `tests/codex-integration/codex-inject-integration.test.ts` asserts that an unreadable | ||
| pre-image aborts capture, but it created that condition with | ||
| `spyOn(fs, "readFileSync")` on the child's `require("node:fs")` handle. Production binds | ||
| `readFileSync` as an ESM named import, so the mock intercepted nothing and the test ran | ||
| entirely undenied. It fails identically on Linux and macOS, which means #4342 landed red | ||
| and dev has been red ever since. The repair replaces the mock with a real `chmod`. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # wp4 — defect landing | ||
|
|
||
| Findings the sweep confirms as release-blocking or user-visible get repair branches off | ||
| fresh `dev`, one branch per defect class so each stays independently reviewable. Every | ||
| repair goes through its own PR with exact-head CI, and dev is allowed to finish one | ||
| push-event run afterwards. | ||
|
|
||
| The known entry already on the register is the top-level help text in | ||
| `src/cli/help.ts`, which still advertises `(14 clients)` after Cline made it fifteen. | ||
| That one is carried by #4390 rather than a separate branch. | ||
|
|
||
| A finding classified as non-blocking is recorded with the reason it does not block, not | ||
| silently dropped. A finding in authentication, credential handling, or release | ||
| automation stops the train instead of being landed quickly. | ||
|
|
||
| ## Exit | ||
|
|
||
| No open release-blocking finding, and dev green at a head that already contains every | ||
| repair. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||
| # wp5 — preview promotion and prerelease | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Preview goes first because 248 commits is too large a step to take straight onto | ||||||||||||||||||||||||
| `main`. The prerelease is the only place the changed config-injection path meets a real | ||||||||||||||||||||||||
| user config before the stable channel does. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Sequence | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Decide the prerelease version and bump the dev version line so `release.yml`'s | ||||||||||||||||||||||||
| `assert-ahead` check passes, promote dev into `preview` through a pull request because | ||||||||||||||||||||||||
| the branch ruleset requires one, wait for the `preview` push-event `ci.yml` run to | ||||||||||||||||||||||||
| report success, then dispatch the Release workflow with the preview dist-tag and the | ||||||||||||||||||||||||
| exact release SHA pinned in `expected-sha`. | ||||||||||||||||||||||||
|
Comment on lines
+9
to
+13
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Make the dev-version move conditional.
-Decide the prerelease version and bump the dev version line so `release.yml`'s
-`assert-ahead` check passes, promote dev into `preview` through a pull request because
+Decide the prerelease version and verify that the dev version satisfies
+`release.yml`'s `assert-ahead` check. Bump dev only if that check would otherwise fail.
+Promote dev into `preview` through a pull request because📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| The workflow also demands a successful `service-lifecycle` run when service-related | ||||||||||||||||||||||||
| files changed since the previous merged tag. That condition has to be checked against | ||||||||||||||||||||||||
| the actual diff, not assumed. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Exit | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| `npm dist-tag` `preview` points at the new prerelease and the publishing run is green. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Resolved numbers | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| The prerelease is `2.52.0-preview.20260912`. `nextPreviewRelease` refuses a patch bump | ||||||||||||||||||||||||
| while the higher-core preview `2.52.0-preview.20260911` is open, so the bump kind is | ||||||||||||||||||||||||
| minor, and the UTC stamp is newer than the incumbent's so no ordinal suffix is added. | ||||||||||||||||||||||||
| dev already carries 2.52.0, which outranks that prerelease, so `assert-ahead` passes with | ||||||||||||||||||||||||
| no dev pre-move. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Promotion is a pull request. The `Protect preview` ruleset gives admin a | ||||||||||||||||||||||||
| `pull_request` bypass only, so even an owner cannot push the branch directly; the | ||||||||||||||||||||||||
| `DeployKey` bypass exists for `scripts/release.ts`'s version-bump push and nothing else. | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| The dispatch is | ||||||||||||||||||||||||
| `gh workflow run release.yml --ref preview -f version=2.52.0-preview.20260912 -f tag=preview -f expected-sha=<40-char preview SHA> -f dry-run=false`. | ||||||||||||||||||||||||
| Omitting `--ref` sends it to `main`. | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # wp6 — main promotion and stable release | ||
|
|
||
| Stable publishes only after the prerelease has been exercised. The same gates apply | ||
| with the stable dist-tag, and `release.yml` additionally refuses a prerelease version | ||
| string on `main`. | ||
|
|
||
| ## Sequence | ||
|
|
||
| Confirm the preview channel showed no new defect, promote dev into `main` through a | ||
| pull request, wait for the `main` push-event `ci.yml` run to succeed, then dispatch the | ||
| Release workflow with the stable version, the `latest` dist-tag, and the pinned SHA. | ||
| Verify the published version through `npm view` rather than trusting the run summary. | ||
|
|
||
| Because PRs here target `dev` rather than the default branch, GitHub never auto-closes a | ||
| linked issue. `AGENTS.md` puts that manual close at the point the change reaches `dev`, | ||
| not `main`. | ||
|
|
||
| ## Exit | ||
|
|
||
| `npm dist-tag` `latest` points at the new stable version, the `main` release SHA has a | ||
| successful push-event CI run, and the sweep results are recorded in this unit. | ||
|
|
||
| ## Resolved numbers | ||
|
|
||
| The stable version is `2.52.0`. `assert-ahead` compares `origin/dev`'s package version | ||
| against it and requires a strict win, and 2.52.0 does not outrank 2.52.0, so dev has to | ||
| move to `2.53.0` before this release can publish. That move is its own pull request, | ||
| opened by dispatching `dev-version-bump.yml` from `main`; the workflow never pushes dev | ||
| itself. | ||
|
Comment on lines
+25
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Promote The plan does not define this ordering. If the bump PR merges into State the order explicitly: promote 🤖 Prompt for AI Agents |
||
|
|
||
| Because the release SHA changes `package.json`, the service-lifecycle condition in | ||
| `release.yml` fires, so that SHA also needs a successful `service-lifecycle` run. | ||
|
|
||
| The dispatch is | ||
| `gh workflow run release.yml --ref main -f version=2.52.0 -f tag=latest -f expected-sha=<40-char main SHA> -f dry-run=false`. | ||
| `release.yml` serializes on a `release` concurrency group, so preview and main cannot | ||
| publish at the same time. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clarify whether the
ciaggregate is an independent failure or a consequence of the other failures.The text states "Four report failure" and then lists
test 4/4,macos 2/2,enforce-target, andthe ci aggregate that reflects the first two. The qualifier "that reflects the first two" suggests theciaggregate is a dependent failure—a consequence of the other two failures rather than an independent check—but the text does not explicitly state this. This creates genuine ambiguity about whether there are 3 independent failing checks plus 1 dependent aggregate, or 4 independent failing checks to repair. For clarity, specify whether theciaggregate represents a fourth independent failure or a meta-failure that stems from the first two.🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: The operating system from Apple is written “macOS”.
Context: ...olved. Four report failure:
test 4/4,macos 2/2,enforce-target, and theciag...(MAC_OS)
🤖 Prompt for AI Agents