|
| 1 | +--- |
| 2 | +description: Audit a v1.4.0-style upstream bridge merge for the regression patterns that bit us last time. Catches brand leaks, behavior patches lost to overwrite, split-brain module pairs, and test fixtures locked to old APIs. Run after the bridge merge tool produces a PR, before merging to main. |
| 3 | +--- |
| 4 | + |
| 5 | +# Bridge Merge Audit |
| 6 | + |
| 7 | +Run this after `script/upstream/bridge-merge.ts` (or any large upstream merge) produces a PR. Catches the regression classes that v1.4.0 shipped before we built the three-layer safety net (commits `3a6b59bdea` + `13ba57f7f1`). |
| 8 | + |
| 9 | +## Input |
| 10 | + |
| 11 | +`$ARGUMENTS` = PR number, or empty to detect from current branch via `gh pr view`. Branch name typically `upstream/merge-vX.Y.Z`. |
| 12 | + |
| 13 | +## Why this skill exists |
| 14 | + |
| 15 | +The v1.4.0 bridge merge shipped 16 user-impacting regressions across 30 files. Root cause: many altimate customizations were never wrapped in `altimate_change` markers, so the bridge merge tool — working as designed — overwrote them with upstream's version. The branding scan was too narrow to catch user-visible bare `opencode` strings; behavior patches (function calls, retry loops, default values) had no marker discipline at all. |
| 16 | + |
| 17 | +This skill walks the auditor through every regression class we've actually seen, with the exact greps + code reads to detect each one. **Treat the patterns as a checklist, not a suggestion** — every item below was a real bug in v1.4.0 that reached the user before we caught it. |
| 18 | + |
| 19 | +## Step 0: Setup |
| 20 | + |
| 21 | +```bash |
| 22 | +git fetch origin |
| 23 | +gh pr view --json number,title,headRefName,baseRefName,url |
| 24 | +git checkout <merge-branch> |
| 25 | +git status --short |
| 26 | +``` |
| 27 | + |
| 28 | +If working tree is dirty, stop. Capture base SHA: `BASE=$(git merge-base HEAD origin/main)`. The audit compares HEAD vs `$BASE`. |
| 29 | + |
| 30 | +## Step 1: Run the three-layer tooling check (mandatory, do this first) |
| 31 | + |
| 32 | +```bash |
| 33 | +bun run script/upstream/analyze.ts --branding --strict |
| 34 | +bun run script/upstream/analyze.ts --require-markers --strict |
| 35 | +bun run script/upstream/analyze.ts --markers --base main --strict |
| 36 | +``` |
| 37 | + |
| 38 | +All three must pass before continuing. If `--branding` flags leaks: those are user-visible strings the broadened scan caught (yargs describe, console output, User-Agent, MCP client name, OIDC audience, etc.). Fix each before moving on. |
| 39 | + |
| 40 | +If `--require-markers` flags missing markers: behavioral patches in known-vulnerable files were lost. Restore them or remove the file from `config.requireMarkers` if it no longer holds patches. |
| 41 | + |
| 42 | +If `--markers` flags unmarked changes: code modifications to upstream-shared files that need wrapping. |
| 43 | + |
| 44 | +## Step 2: Diff scope reconnaissance |
| 45 | + |
| 46 | +```bash |
| 47 | +git diff main --stat -- packages/opencode/src/ | tail -5 |
| 48 | +git diff main --name-only -- packages/opencode/src/ | wc -l |
| 49 | +``` |
| 50 | + |
| 51 | +If >100 files changed, this is a major bridge merge — every step below is mandatory. If <50 files, you can be more selective but still hit the high-risk categories. |
| 52 | + |
| 53 | +Spawn parallel audit subagents to cover the diff in chunks. Each agent gets the regression catalog in Step 3 and reports findings. Three concurrent agents work well: one for `cli/` + `tui/`, one for `session/` + `server/` + `permission/`, one for everything else. |
| 54 | + |
| 55 | +## Step 3: The regression catalog |
| 56 | + |
| 57 | +Each pattern below names a class of bug, where it lives, the exact grep to detect it, and the fix shape. Walk every category — they each bit us in v1.4.0. |
| 58 | + |
| 59 | +### 3.1 Split-brain module pairs |
| 60 | + |
| 61 | +**Symptom:** Two modules export the same API + emit the same bus events, but each owns its own state. HTTP route calls one, runtime calls the other. Replies hit empty maps; tools deadlock forever. |
| 62 | + |
| 63 | +**v1.4.0 instance:** `Permission` (Effect-TS, `src/permission/index.ts`) vs `PermissionNext` (vanilla, `src/permission/next.ts`). Tool calls invoked `PermissionNext.ask`; HTTP `POST /permission/{id}/reply` route invoked `Permission.reply`. Result: every permission round-trip silently no-op'd, tool dispatch hung, "Allow once" looped back to the same prompt forever. |
| 64 | + |
| 65 | +**Detect:** |
| 66 | +```bash |
| 67 | +# Find duplicate BusEvent.define names (the smoking gun) |
| 68 | +grep -rA3 'BusEvent\.define(' packages/opencode/src/ | grep -oE '"[a-z][a-z0-9._]*"' | sort | uniq -d |
| 69 | + |
| 70 | +# Find sibling modules with overlapping APIs |
| 71 | +find packages/opencode/src -name "next.ts" -o -name "*-next.ts" |
| 72 | +# For each, check who imports the original vs the -next variant |
| 73 | +``` |
| 74 | + |
| 75 | +**Fix:** Verify every HTTP route handler calls the same module the runtime ask side calls. The `requireMarkers` allowlist in `script/upstream/utils/config.ts` already includes `server/routes/permission.ts` and `server/routes/session.ts` — so the bridge tool now refuses to overlay them. But check any newly-introduced sibling modules. |
| 76 | + |
| 77 | +### 3.2 Test fixtures locked to old API |
| 78 | + |
| 79 | +**Symptom:** Tests pass type-check but fail at runtime because they use `adaptor.fetch(...)` while production code now uses `adaptor.target()`. Often labeled with `// @ts-nocheck` or "DRAFT" comments. |
| 80 | + |
| 81 | +**v1.4.0 instance:** `test/control-plane/session-proxy-middleware.test.ts` had a TestAdaptor implementing `fetch` only. Production middleware called `(adaptor as any).fetch(...)` with a misleading marker comment claiming `fetch` was a "v1.4.0-only API not in main" — the comment had the upstream direction reversed. |
| 82 | + |
| 83 | +**Detect:** |
| 84 | +```bash |
| 85 | +grep -rn '@ts-nocheck' packages/opencode/test 2>&1 | head -10 |
| 86 | +grep -rn '(adaptor as any)' packages/opencode/src 2>&1 |
| 87 | +grep -rn 'as any).fetch\|as any).target' packages/opencode 2>&1 |
| 88 | +``` |
| 89 | + |
| 90 | +**Fix:** When a marker comment claims an API "is upstream-only" or "not in main," verify by reading the type definition. If the comment is wrong, rewrite to use the new API and update the test fixture. |
| 91 | + |
| 92 | +### 3.3 Brand strings in user-visible contexts |
| 93 | + |
| 94 | +**Symptom:** `describe:` text, `console.log` strings, `UI.println` arguments, MCP `clientInfo.name`, `User-Agent` headers, OIDC audience, GitHub workflow YAML — all reverted to `opencode`. |
| 95 | + |
| 96 | +**v1.4.0 instances:** 11+ files (`cli/cmd/serve.ts`, `web.ts`, `uninstall.ts`, `pr.ts`, `mcp.ts`, `error.ts`, `dialog-status.tsx`, `error-component.tsx`, `ui.ts` non-TTY wordmark, `github.ts` sweeping revert, `plugin/codex.ts` UA strings, `plugin/copilot.ts` UA strings, `network.ts` mDNS). |
| 97 | + |
| 98 | +**Detect:** This is exactly what the broadened branding scan catches now. If Step 1 passed, you're already covered. But also do a manual spot-check: |
| 99 | + |
| 100 | +```bash |
| 101 | +# Anything that says "opencode" in user-visible string contexts |
| 102 | +grep -rE 'describe:\s*["`'\''`][^"`'\''`]*opencode' packages/opencode/src/cli/cmd/ |
| 103 | +grep -rE 'console\.(log|error)\s*\([^)]*opencode' packages/opencode/src/ | grep -v "altimate_change" |
| 104 | +grep -rE 'User-Agent.*opencode/\$\{' packages/opencode/src/ |
| 105 | +``` |
| 106 | + |
| 107 | +**Fix:** Replace `opencode` → `altimate-code` (or `altimate` for some contexts — the User-Agent for chat headers uses `altimate/`, the binary name uses `altimate-code/`). Wrap each fix in `altimate_change start — upstream_fix: ... / altimate_change end`. |
| 108 | + |
| 109 | +### 3.4 Logo / color / theme regressions (visible at TUI startup) |
| 110 | + |
| 111 | +**Symptom:** ASCII logo glyphs replaced; brand colors swapped from `theme.primary` / `theme.accent` to monochrome `theme.textMuted` / `theme.text`; theme palette tokens darkened (Subtext1 → Overlay2). |
| 112 | + |
| 113 | +**v1.4.0 instances:** `cli/logo.ts` (ALTIMATE | CODE → OPEN | CODE), `tui/component/logo.tsx` (primary/accent → textMuted/text), three Catppuccin variants (Subtext1 → Overlay2). |
| 114 | + |
| 115 | +**Detect:** |
| 116 | +```bash |
| 117 | +# Logo glyph diff |
| 118 | +git diff main -- packages/opencode/src/cli/logo.ts |
| 119 | +# Logo render colors diff |
| 120 | +git diff main -- packages/opencode/src/cli/cmd/tui/component/logo.tsx |
| 121 | +# Theme palette tokens (Subtext1 vs Overlay2 etc) |
| 122 | +git diff main -- packages/opencode/src/cli/cmd/tui/context/theme/ |
| 123 | +``` |
| 124 | + |
| 125 | +**Fix:** Restore main's version. The catppuccin JSON files are now in `keepOurs` so this shouldn't recur. Logo files are in `requireMarkers` — markers are required. Logo TSX uses `theme.primary` (warm orange `#fab283`) on the left half + `theme.accent` (purple `#9d7cd8`) on the right. |
| 126 | + |
| 127 | +### 3.5 Behavior patches lost (invisible to string scans) |
| 128 | + |
| 129 | +**Symptom:** A function call disappears, a `setTimeout(..., 0)` wrapper is removed, a retry loop becomes a single attempt, a defensive `.catch(() => undefined)` is dropped. No string change — only a behavioral diff. |
| 130 | + |
| 131 | +**v1.4.0 instances (all silent until users hit them):** |
| 132 | +- `project/bootstrap.ts` — `Truncate.init()` call dropped; tool-output dir grows unboundedly forever. |
| 133 | +- `provider/models.ts` — `setTimeout(..., 0)` deferral removed around the initial `ModelsDev.refresh()`; circular-dep risk on cold start (the bug altimate commit `980efaab64` was originally added to fix). |
| 134 | +- `plugin/codex.ts` — OAuth refresh retry loop (3 attempts, 4xx-vs-5xx aware) removed; transient network blips now hard-fail user sessions. Also the 30s token-expiry skew buffer was removed. |
| 135 | +- `control-plane/workspace.ts` — `.catch(() => undefined)` defensive swallow dropped; transient network blip kills the SSE reconnect loop forever. Local workspaces also `return`ed out of the loop forever. |
| 136 | +- `session/message-v2.ts` — opaque-error augmentation (PR #118/#133) replaced with bare `errorMessage(e)`; bare "Error" / matching name strings leak through to TUI. |
| 137 | +- TUI components (`clipboard.ts`, `dialog-workspace-list.tsx`, `dialog-mcp.tsx`) — `Log.Default.debug/error/info` replaced with `console.log/error`. `console.*` writes directly to terminal mid-render and corrupts the TUI display. |
| 138 | + |
| 139 | +**Detect:** No automated scan can catch these. Read every changed file in `requireMarkers` line-by-line, compare to `git show main:<path>`. Look for: |
| 140 | + |
| 141 | +```bash |
| 142 | +# Functions whose call sites disappeared |
| 143 | +git diff main -- packages/opencode/src/ | grep -E "^- *[A-Z][a-zA-Z]*\.[a-zA-Z]+\(\)" |
| 144 | +# setTimeout / setInterval wrappers removed |
| 145 | +git diff main -- packages/opencode/src/ | grep -B2 -A2 "^-.*setTimeout" |
| 146 | +# .catch handlers dropped |
| 147 | +git diff main -- packages/opencode/src/ | grep "^-.*\.catch(" |
| 148 | +# Log.Default replaced with console |
| 149 | +git diff main -- packages/opencode/src/cli/cmd/tui/ | grep -B1 "console\." |
| 150 | +``` |
| 151 | + |
| 152 | +**Fix:** For each behavior diff, look at the surrounding altimate commit history (`git log --follow main -- <file>`) to see if the patch was intentional. Restore + wrap in `altimate_change start — upstream_fix: ... / end` markers. Add the file to `requireMarkers` if not already there. |
| 153 | + |
| 154 | +### 3.6 New altimate-only files dropped to upstream-deleted-review |
| 155 | + |
| 156 | +**Symptom:** Files that exist in main but not in upstream (e.g., `src/memory/**`, `src/altimate/**`) get classified as `upstreamDeletedReview` by `bridge-merge.ts`. The tool keeps them but flags for human review. If reviewer skips → file might be removed by mistake. |
| 157 | + |
| 158 | +**v1.4.0 instance:** `packages/opencode/src/memory/**` (altimate_memory_* tool family) was in `upstreamDeletedReview` until I added it to `keepOurs`. |
| 159 | + |
| 160 | +**Detect:** Open `.bridge-merge-report.md` (the report file the bridge tool writes) and look at the "Files in main but not in v1.4.0 — REVIEW" section. Anything altimate-owned should already be in `keepOurs`. |
| 161 | + |
| 162 | +**Fix:** Add the file's parent directory to `keepOurs` glob in `script/upstream/utils/config.ts`. |
| 163 | + |
| 164 | +### 3.7 Internal scripts that wrap upstream binary names |
| 165 | + |
| 166 | +**Symptom:** `script/beta.ts`, `script/raw-changelog.ts`, `packages/script/src/index.ts` use literal `opencode` references when they should use `altimate-code` (binary name) or filter against upstream's bot identity. |
| 167 | + |
| 168 | +**Detect:** |
| 169 | +```bash |
| 170 | +grep -n "opencode" script/beta.ts script/raw-changelog.ts packages/script/src/index.ts |
| 171 | +``` |
| 172 | + |
| 173 | +**Fix:** Two cases: |
| 174 | +- Spawning a binary (`$\`opencode run\``) → must be `altimate-code run` (the upstream binary isn't installed). |
| 175 | +- Filtering bot identities for changelog generation (`["actions-user", "opencode", "opencode-agent[bot]"]`) → these are the upstream identities we filter AGAINST. Keep them; wrap the array in `altimate_change start — intentional: filters upstream bot ... / end`. |
| 176 | + |
| 177 | +## Step 4: Verify the safety net itself |
| 178 | + |
| 179 | +The three-layer defense is only useful if the lists stay current. After fixing all findings: |
| 180 | + |
| 181 | +```bash |
| 182 | +# Every file we just touched should be in requireMarkers (if it holds patches) |
| 183 | +# OR keepOurs (if it's pure altimate) |
| 184 | +git diff main --name-only -- packages/opencode/src/ | while read f; do |
| 185 | + in_require=$(grep -c "\"$f\"" script/upstream/utils/config.ts) |
| 186 | + in_keep=$(grep -c "\"$f\"\|$(dirname $f)/\*\*" script/upstream/utils/config.ts) |
| 187 | + has_marker=$(grep -l "altimate_change" "$f" 2>/dev/null) |
| 188 | + if [ -z "$has_marker" ] && [ "$in_require" -eq 0 ] && [ "$in_keep" -eq 0 ]; then |
| 189 | + echo "UNPROTECTED: $f" |
| 190 | + fi |
| 191 | +done |
| 192 | +``` |
| 193 | + |
| 194 | +Files printed by `UNPROTECTED:` are vulnerable to the next bridge merge — either add markers, add to `requireMarkers`, or add the parent directory to `keepOurs`. |
| 195 | + |
| 196 | +## Step 5: Functional verification |
| 197 | + |
| 198 | +Tooling checks aren't enough. Each behavior class needs a runtime check: |
| 199 | + |
| 200 | +```bash |
| 201 | +# Typecheck the whole tree |
| 202 | +bun turbo typecheck |
| 203 | + |
| 204 | +# Focused suites that exercise the regressions |
| 205 | +cd packages/opencode |
| 206 | +bun test test/upstream test/permission test/server test/util/error.test.ts test/control-plane |
| 207 | + |
| 208 | +# Full suite (some flaky failures under heavy parallel load are expected; |
| 209 | +# verify each failure passes in isolation before treating it as a real regression) |
| 210 | +bun test --timeout 30000 |
| 211 | +``` |
| 212 | + |
| 213 | +For UI/TUI changes, build a local binary and visually verify: |
| 214 | + |
| 215 | +```bash |
| 216 | +bun --cwd packages/opencode run build:local |
| 217 | +INSTALL_DIR="$(npm root -g)/@altimateai/altimate-code" |
| 218 | +cp packages/opencode/dist/*/bin/altimate-code "$INSTALL_DIR/bin/altimate-code" |
| 219 | +altimate-code # check: logo colors, brand strings, permission flow |
| 220 | +``` |
| 221 | + |
| 222 | +Critical end-to-end test: trigger a permission prompt and verify "Allow once" / "Allow always" / "Reject" all unblock the tool dispatch (this is the v1.4.0 deadlock scenario — see `test/upstream/v140-permission-deadlock.test.ts` for a runtime regression test). |
| 223 | + |
| 224 | +## Step 6: Sign-off |
| 225 | + |
| 226 | +Before approving the PR, confirm: |
| 227 | + |
| 228 | +- [ ] `bun run script/upstream/analyze.ts --branding --strict` — 0 leaks |
| 229 | +- [ ] `bun run script/upstream/analyze.ts --require-markers --strict` — every file in the list has markers |
| 230 | +- [ ] `bun run script/upstream/analyze.ts --markers --base main --strict` — every change in upstream-shared files has markers |
| 231 | +- [ ] `bun turbo typecheck` — 5/5 packages clean |
| 232 | +- [ ] Walked Step 3 categories 3.1–3.7 explicitly. Don't skip categories — every one was a real bug in v1.4.0. |
| 233 | +- [ ] Each fix is wrapped in `altimate_change start — upstream_fix: ... / altimate_change end` |
| 234 | +- [ ] Any new altimate-only directory is in `keepOurs` glob |
| 235 | +- [ ] Any newly-discovered patched file is in `requireMarkers` |
| 236 | +- [ ] Local TUI smoke-test passes (logo, permission flow, key strokes) |
| 237 | +- [ ] Full test suite passes; any failures are confirmed pre-existing flakes (pass in isolation, present before the merge branch) |
| 238 | + |
| 239 | +## Reference: where everything lives |
| 240 | + |
| 241 | +| Tool / list | Location | Purpose | |
| 242 | +|---|---|---| |
| 243 | +| Branding leak scan | `script/upstream/analyze.ts` `LEAK_PATTERNS` | Catches user-visible bare `opencode` strings | |
| 244 | +| `requireMarkers` allowlist | `script/upstream/utils/config.ts` field | Files that must keep their `altimate_change` markers | |
| 245 | +| `keepOurs` globs | `script/upstream/utils/config.ts` field | Files the bridge merge tool never overlays | |
| 246 | +| `skipFiles` globs | `script/upstream/utils/config.ts` field | Upstream files to discard entirely | |
| 247 | +| Bridge merge tool | `script/upstream/bridge-merge.ts` | Produces the merge plan + applies the overlay | |
| 248 | +| Marker check (CI) | `script/upstream/analyze.ts --markers` | Flags new code in upstream-shared files without markers | |
| 249 | +| v1.4.0 regression tests | `packages/opencode/test/upstream/v140-*.test.ts` | Lock the regression patterns at the test level | |
| 250 | +| Defensive runtime test | `packages/opencode/test/upstream/v140-permission-deadlock.test.ts` | End-to-end ask→reply→resolve regression test for the split-brain class | |
| 251 | + |
| 252 | +## Reference: relevant commits |
| 253 | + |
| 254 | +- `33ffd9c51f` — Permission split-brain fix (route to PermissionNext) |
| 255 | +- `7a91221d05` — Runtime e2e regression test for the deadlock scenario |
| 256 | +- `e203a9ade1` — Logo glyph restore (ALTIMATE | CODE) |
| 257 | +- `72c215d042` — Logo brand color restore (theme.primary + theme.accent) |
| 258 | +- `3a6b59bdea` — 16-regression line-by-line audit fix |
| 259 | +- `13ba57f7f1` — Three-layer regression backstop (broadened scan + requireMarkers + retroactive sweep) |
| 260 | +- `b84264255a`+ — Original v1.4.0 bridge merge (the one that introduced the regressions — read its diff to understand the upstream attack surface) |
| 261 | + |
| 262 | +$ARGUMENTS |
0 commit comments