Skip to content

fix(social): stop the import barrel re-exporting Node-only code - #595

Closed
crs48 wants to merge 1 commit into
mainfrom
fix/social-barrel-browser-safe
Closed

fix(social): stop the import barrel re-exporting Node-only code#595
crs48 wants to merge 1 commit into
mainfrom
fix/social-barrel-browser-safe

Conversation

@crs48

@crs48 crs48 commented Jul 19, 2026

Copy link
Copy Markdown
Owner

What

packages/social/src/import/index.ts did export * from './node'. Since the package root barrel (packages/social/src/index.ts) does export * from './import', the Node-only archive-reader.ts — which imports node:fs, node:zlib, node:stream, node:crypto — was reachable from every browser bundle that touches @xnetjs/social.

One line changed:

-export * from './node'
+export * from './core'

Why

This already broke apps/web once (July 2026), failing Build packages and with it the production deploy-site run:

archive-reader.ts (15:27): "promises" is not exported by "__vite-browser-external"

It stopped reproducing when the editor swap changed what apps/web pulls in — but the hazard was one import away from returning.

The package already declares the split it wants via its exports map: ./import/core (browser-safe), ./import/browser, ./import/node. The barrel simply named the wrong one. This also aligns with the sub-barrel policy in CLAUDE.md.

node.ts is export * from './core' plus the zip helpers, so core is a strict subset — nothing else is lost.

Verification

Probed the package root with a browser-platform bundle:

barrel target node:* builtins reaching the browser graph
'./node' (before) 6crypto, fs, path, stream, stream/promises, zlib
'./core' (after) 0

apps/web's own build isn't a sensitive test any more (it no longer imports that path), which is why the direct probe matters.

Also green, on node v23.11.1:

  • pnpm --filter "xnet-web..." build — passes (baseline established first, then re-run with the change)
  • npx vitest run src/__tests__/archive-reader.test.ts from packages/social — 4/4 pass (not in the root vitest project list)
  • turbo typecheck --filter @xnetjs/social and --filter xnet-desktop — clean

Consumers

Three reach the Node-only symbols (createZipJsonEntryReader, createZipTextEntryReader, readZipArchiveManifest):

  • packages/social/src/__tests__/archive-reader.test.ts — went through the barrel, repointed to ../import/node
  • apps/electron/src/main/social-import-ipc.ts — already on @xnetjs/social/import/node, unchanged
  • scripts/benchmark-social-import-fixtures.ts — already on ../packages/social/src/import/node.ts, unchanged

Notes

@xnetjs/social is private, so no changeset. No user-visible behaviour change, so skip-changelog.

🤖 Generated with Claude Code

`packages/social/src/import/index.ts` re-exported `./node`, which pulls in
`archive-reader.ts` and its `node:fs` / `node:zlib` / `node:stream` /
`node:crypto` imports. Because the package root barrel re-exports `./import`,
those Node builtins were reachable from every browser bundle that touched
`@xnetjs/social`.

This already broke `apps/web` once, failing `Build packages` and with it the
production `deploy-site` run:

    archive-reader.ts (15:27): "promises" is not exported by
    "__vite-browser-external"

It stopped reproducing when the editor swap changed what apps/web pulls in,
but the hazard was one import away from returning.

The package already declares the split via its `exports` map — `./import/core`
(browser-safe), `./import/browser`, `./import/node`. The barrel simply named
the wrong one. `node.ts` is `export * from './core'` plus the zip helpers, so
`core` is a strict subset and nothing is lost.

Verified with a browser-platform bundle of the package root: 6 `node:*`
builtins reached the browser graph before, 0 after.

The only consumer reaching the Node-only symbols through the barrel was
`archive-reader.test.ts`; it now imports from `../import/node`.
`apps/electron/src/main/social-import-ipc.ts` and
`scripts/benchmark-social-import-fixtures.ts` already import from the
`node` entrypoint and are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: xNet Test <test@xnet.dev>
@crs48
crs48 temporarily deployed to pr-595 July 19, 2026 19:57 — with GitHub Actions Inactive
@crs48 crs48 added the skip-changelog Exclude this PR from the changelog label Jul 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🖼️ UI changes in this PR

No visual differences detected in the changed UI.

CI run

github-actions Bot added a commit that referenced this pull request Jul 19, 2026
github-actions Bot added a commit that referenced this pull request Jul 19, 2026
@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Preview removed for PR #595.

github-actions Bot added a commit that referenced this pull request Jul 19, 2026
@crs48

crs48 commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Closing as already landed. The change is byte-identical to main today:

$ git diff origin/main pr595 -- packages/social/src/import/index.ts \
    packages/social/src/__tests__/archive-reader.test.ts
# (empty)

main's barrel is already export * from './core', landed by fbc2965a2 feat(social): add browser-safe import staging. The branch is 864 files behind, so merging would be a no-op. Triage: docs/explorations/0410_[_]_OPEN_PR_TRIAGE_AND_THE_STRANDED_BRANCH_PROBLEM.md.

@crs48 crs48 closed this Jul 30, 2026
github-actions Bot added a commit that referenced this pull request Jul 30, 2026
crs48 added a commit that referenced this pull request Jul 30, 2026
…(0410) (#658)

Implements exploration
`0410_[_]_OPEN_PR_TRIAGE_AND_THE_STRANDED_BRANCH_PROBLEM.md`, written in
the previous turn.

## What this closes out

Eight PRs were open, the oldest from March. Checking each one's **actual
diff against `main`** (rather than its description) found that four
described work already in `main`, re-derived and landed weeks later by
another route.

| PR | Age | Action taken |
| --- | --- | --- |
| #400 | 24d | **Closed** — fix landed on main in a better form; branch
predates 0391 and would have deleted `NodeLineRunner` |
| #9 | 145d | **Closed** — draft;
`apps/electron/src/renderer/workspace/` no longer exists |
| #595 | 11d | **Closed** — byte-identical to main (`fbc2965a2`) |
| #463 | 20d | **Closed** — main has a longer, `[x]` version of 0298 |
| #13 | 141d | **Closed** — both fixes on main verbatim |
| #505 | 17d | **Merged** — exploration 0318 + scale bench |
| #389 | 25d | **Merged** — exploration 0270 |
| #449 | 20d | **Rebuilt here** — see below |

`#400` was the load-bearing one: `git diff origin/main pr400 --
packages/devkit/src/command-runner.ts` showed ~150 lines of *deletion*
covering the whole 0391 streaming agent seam. Only its `CONFLICTING`
state had prevented that.

`#505`'s `build-and-smoke-test` went green on a rebase, confirming it
was branch staleness rather than the diff.

## The #449 rebuild (0290 bugs #2 and #3)

The original patched `apps/web/src/workbench/SyncStatus.tsx`, which 0406
deleted; the component now lives in `packages/workbench`, so the rebuild
lands on **desktop and web at once**.

- `WorkbenchHost` gains a `hub` capability (`configuredUrl` / `connect`)
so the shell can offer a way out of the disconnected state without
importing either app's `hub-url` module. `connect` returns a reason
string on rejection — never a silent no-op.
- `SyncStatus` exports `openSyncStatusPanel()`; the desktop popover and
mobile sheet both listen. The panel shows an inline hub-URL form while
there is no hub.
- `ShareDialog`'s `!ready` branch explains why a hub is needed and
offers **Connect a hub…**; private-hub links now confirm before copying.
- Adds `normalizeHubUrl` to the electron renderer's `hub-url` mirror (+4
unit tests), so a malformed URL is rejected loudly rather than
persisted.

Verified by driving the real app against a local hub — the CTA closes
the dialog and opens the panel; an invalid URL shows an error and
persists nothing; `https://hub.xnet.fyi/` normalises to
`wss://hub.xnet.fyi`; a `localhost` link's first Copy click asks "Copy
local-only link?" and writes nothing, the second copies. No console
errors. 0290 is now `[-]` at 11/20.

## Stopping the recurrence

Time-to-merge for the last 20 merged PRs was **0 hours median, 1 hour
max** — there is no review queue, so a PR that outlives its session is
abandoned, not pending.

- `.github/workflows/stale.yml` — warns at 14d, closes at 21d,
`keep-open` exempts. Named consumer (the maintainer, weekly) and a
decidable pass condition, per AGENTS.md.
- Fixed the `/explore` next-number command, which read only the working
tree and so handed out numbers already claimed on branches. Seven
explorations were found stranded that way.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Exclude this PR from the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant