Skip to content

Retire fork compatibility shims and adopt upstream implementations #13

Description

@androidand
Proposal

Why

The fork carries code whose only reason for existing is that upstream moved and we
did not follow. It is not a fork feature. Nobody chose it. It accumulated because a
sync was easier to finish by re-creating the old world locally than by adapting to
the new one.

The 2026-08-11 sync to 0d927ba03f made the cost visible. The merge resolved
package.json and several source files as "ours", which silently reverted upstream's
catalog (opentui 0.4.5 → 0.3.4, effect beta.83 → beta.74) and kept pre-refactor copies
of files upstream had since rewritten. The build broke in four places, the binary
crashed at schema-construction time, and none of it was a fork feature failing — it
was drift presenting as breakage.

The distinction this change enforces:

  • Fork feature — something we added for a reason we can name (loop detection,
    llama-skein/local provider discovery, beads sync, peers tool, fork distribution
    targets, themed loading). These stay, and fork/manifest.json is their register.
  • Drift — a local copy, shim, or stale pattern that exists only because upstream
    changed. These have no defenders and must be deleted in favour of upstream's version.

Drift is worse than dead code. Dead code is inert; drift compiles, runs, and quietly
diverges from upstream until a sync detonates. It also inflates the merge-conflict
surface on every future sync, which is the tax sustainable-upstream-sync is trying
to lower.

What changes

Three concrete surfaces, measured on dev at 0d927ba03f:

1. The legacy logger shim. packages/core/src/util/log.ts (98 lines) re-implements
a logger upstream deleted in anomalyco#31310 ("replace legacy logger with Effect logging"). Its
own header documents the retirement path and admits it is a stopgap. 17 call sites
across 5 fork files (local/sync.ts 9, local/placement.ts 3, beads/sync.ts 3,
provider/provider.ts 2, beads/beads.ts 0). Migrate callers to Effect logging,
delete the file and its fork/manifest.json entry.

2. The legacy defaultLayer DI pattern. Upstream's unify-layer-node-graph work
replaced per-service defaultLayer exports with the LayerNode graph. The fork still
carries 167 references across 21 files (11 fork-only, 10 upstream-existing), which is
the bulk of the ~260 outstanding typecheck errors. Upstream's replacement is
AppNodeBuilder.build(X.node) / LayerNode.compile(...).

3. Divergence in files that should be upstream-identical. Files carrying no fork
feature but differing from upstream — the sync already surfaced project.ts
(inlined duplicates of @opencode-ai/schema/project), app-runtime.ts, layer-node.ts
(a gutted copy, 285 lines short), tool/registry.ts, session/prompt.ts and
installation/index.ts. Those six are fixed; this change sweeps for the rest.

The sweep is the deliverable, not just the three known instances. Every fork-modified
file gets classified as feature or drift, and drift is reverted to upstream.

What does not change

  • No fork feature is removed. bun run fork:verify must report 10/10 owned and
    7/7 patched at every phase boundary.
  • The fork's distribution identity (ForkDistribution) stays; the updater must never
    point at upstream.
  • Deliberate divergence is allowed to remain if it is recorded. The output of the
    sweep is that every remaining difference from upstream is either in
    fork/manifest.json or gone.

Impact

  • Cuts the merge-conflict surface for every future sync — the motivating goal of
    sustainable-upstream-sync.
  • Clears ~260 typecheck errors, restoring bun run typecheck as a usable gate. Today
    it fails on dev, so it catches nothing.
  • Removes an entire class of sync failure: the 2026-08-11 build break was drift, not
    feature breakage, and would not have happened against an upstream-parity tree.

Risks

  • session/prompt.ts and provider/provider.ts are large, carry real fork features,
    and are manifest-patched. They need per-hunk classification, not wholesale restore.
  • Test files are the largest single block of defaultLayer use (~90 refs). Migrating
    them changes what the suite proves; the suite must be run, not just typechecked.

Disposition (2026-09-18)

Shipped. Phase 3 finished 2026-09-18: the 77 defaultLayer exports re-added by af670cf deleted (typecheck + 1310 tests clean), llm.ts upstream-identical, accepted divergence 67→35. 1.x inventory, 6.4, 6.5 dropped as non-goals — fork/manifest.json is the inventory.

Tasks

Phase order matters. Each phase ends with bun run fork:verify (10/10 owned, 7/7
patched) and a build smoke test. Do not start a phase until the previous one is green.

Phase 0: Baseline (done during the 2026-08-11 sync)

  • 0.1 Restore upstream catalog in package.json; keep only the 5 fork scripts
  • 0.2 Restore patches/solid-js@1.9.10.patch; drop 3 orphaned downgrade-era patches
  • 0.3 Revert drift in project.ts, app-runtime.ts, layer-node.ts, tool/registry.ts
  • 0.4 Repoint phantom imports (max-steps.txt → core max-steps, withStatics → statics)
  • 0.5 Re-apply ForkDistribution onto upstream's installation/index.ts (21 sites)
  • 0.6 Fix Schema.Defect → Schema.Defect() (beta.83 made it a function)
  • 0.7 Delete junk file packages/core/src/effect/dfdf
  • 0.8 Confirm green: build + smoke test + fork:verify

Phase 1: Classify the divergence surface

  • 1.1 Enumerate every file differing from upstream/dev under packages/*/src
    and packages/*/test, excluding generated gen/ trees
  • 1.2 Classify each as feature (named fork capability) or drift (local copy,
    shim, or stale pattern). Record the verdict and a one-line reason per file
  • 1.3 Cross-check every feature verdict against fork/manifest.json; anything
    claiming feature status but absent from the manifest is drift until argued otherwise
  • 1.4 Publish the classification table in this change directory as inventory.md

Phase 2: Retire the legacy logger shim

  • 2.1 Migrate packages/opencode/src/local/sync.ts (9 call sites) to Effect logging
  • 2.2 Migrate packages/opencode/src/local/placement.ts (3 call sites)
    • pick() kept plain deliberately: it documents a synchronous slot reservation
      ("before any await"), and an Effect.gen yield boundary through that would be a
      real concurrency bug. It now returns a PickOutcome discriminated union and
      tool/task.ts logs in Effect context.
  • 2.3 Migrate packages/opencode/src/beads/sync.ts (3 call sites)
  • 2.4 Migrate packages/opencode/src/provider/provider.ts (2 call sites) — manifest-patched,
    keep the mergeDiscoveredModel marker intact
    • Plain promise chain, so discoverOpenAICompatibleModels now returns
      { models, warnings } and the Effect caller logs. Same shape as pick below.
  • 2.5 Drop the unused import in packages/opencode/src/beads/beads.ts (0 call sites)
  • 2.6 Delete packages/core/src/util/log.ts and its fork/manifest.json owned entry
  • 2.7 Confirm no importer of @opencode-ai/core/util/log remains
    (note: packages/console/core/src/util/log.ts is a different, upstream file — leave it)

Phase 3: Retire the legacy defaultLayer DI pattern

  • 3.1 Revert the 10 upstream-existing files that use defaultLayer to upstream,
    re-applying fork features per the Phase 1 classification
  • 3.2 Migrate the 11 fork-only files to LayerNode — delete export const defaultLayer,
    keep export const node
  • 3.3 Migrate remaining call sites: Effect.provide(X.defaultLayer) →
    Effect.provide(AppNodeBuilder.build(X.node))
  • 3.4 Migrate the test block (~90 refs, largest single group), incl. fork-only
    test/loop/loop.test.ts and test/loop/queue-mode.test.ts
    • The merge had kept the fork's test files while taking upstream's source, so
      suites referenced defaultLayer on services that no longer export it, and
      provider.test.ts imported @/project/instance-layer, a module that does not
      exist (whole file failed to import).
    • A mechanical X.defaultLayer -> AppNodeBuilder.build(X.node) swap is WRONG:
      build() returns a closed layer, so each call constructs its own Database and
      a session written through one is invisible to the others. The working shape is
      upstream's: one module-level LayerNode.group root, compiled once with
      LayerNode.compile(root, replacements), mocks injected as node replacements.
    • prompt.test.ts restored from upstream (only 2 fork-only tests, re-added).
      Loop suites are fork-only, so they were ported to the upstream harness shape.
  • 3.5 Run the suite — not just typecheck — and confirm no test silently weakened

Phase 4: Regenerate the SDK

  • 4.1 Establish why sdk.gen.ts references symbols absent from types.gen.ts
    (both generated, both fork-modified — determine which is stale)
  • 4.2 Regenerate via script/generate.ts; confirm the fork's llama-skein client
    additions survive regeneration
  • 4.3 Confirm downstream consumers typecheck (packages/plugin, packages/tui)
  • 4.4 Coordinate with the in-flight llama-skein client work before landing
    • Regenerating exposed that the fork's loop.updated and side-question.response
      events had been dropped from the event manifest by the merge, so no client could
      subscribe. Re-registered in packages/opencode's manifest.

Phase 5: Make the gate real

  • 5.1 bun run typecheck passes clean at the root
  • 5.2 Extend script/fork-verify.ts to fail when a file differs from upstream
    without a fork/manifest.json entry — drift becomes a build error, not a discovery
  • 5.3 Document the feature-vs-drift rule in FORK_WORKFLOW.md
  • 5.4 Add the classification step to the sync runbook so the next sync starts here

Phase 6: Test-suite stability (discovered during Phase 3)

Unchecked items above: see Disposition in proposal.md (2026-09-18).

Plan changes

26 done

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions