Repository navigation
fix(client): ship lib/client.js as the DSH browser bundle, not tsc output - #2
Conversation
…tput
`exports["./client"].default` published the Node-side `tsc` compile of
`src/client.ts` — top-level `import`/`export` — but `@deepseek-ai/dsh-client-modules`
does not import that file as a module. It installs it with
`document.createElement('script')` and expects the classic script to hand its
factory to `window.__ModuleLoader__.load({ id, factory })`. The browser throws
`SyntaxError: Cannot use import statement outside a module` at the file, and
because the host concatenates several packages into one combo script per batch,
that parse error aborts the whole batch: every client entry in it stays
unactivated and the UI reports **"Failed to load plugins"** naming this package.
Reproduced in a clean headless Edge against a local `0.1.7-alpha.2` host before
the change (the SyntaxError lands on the batch URL, then
`web boot: 1 entry did not activate / @perrylink/dsh-github: import failed`), and
absent after it: every served batch parses as a classic script, the page boots
with zero window errors, and the card mounts.
`pnpm build` now emits the artifact in the loader's shape:
window.__ModuleLoader__.load({ id: "@perrylink/dsh-github", factory: (require) => { ... } })
with `react` and `@deepseek-ai/dsh-client-ui-primitives` left as `require(...)`
for the shell's module table, and everything else inlined. A bare specifier in
the shared-runtime families (`@deepseek-ai/cordis`, `@deepseek-ai/dsh-client-*`)
that is not a table row fails the build instead of inlining a second copy of a
singleton the shell already holds.
- `scripts/client-bundle.mjs` — the artifact contract: module-table externals,
the registration wrapper, and the checks that prove a produced file has it.
- `scripts/build-client.mjs` — the build step, also exposed as `pnpm build:client`.
- `scripts/prepare.mjs` — requires `typescript` **and** `esbuild` before it
compiles anything, because `tsc` alone overwrites a good committed
`lib/client.js` with plain ESM; the git-install fallback (no devDependencies)
now accepts committed artifacts only when they are a real bundle.
- `scripts/verify-artifacts.mjs` and `test/client-bundle.test.ts` — execute the
shipped file the way the shell does, as a classic script against a stub
`window.__ModuleLoader__`, so a leftover `import` fails with the browser's own
`SyntaxError` rather than passing a text scan. `node --check` cannot see this:
the package is `"type": "module"`, which makes that parse the file as ESM.
`esbuild` joins `devDependencies`. The lockfile gains only its importer row —
the package was already resolved there as a vitest dependency, and
`pnpm-workspace.yaml` already allowlists its postinstall.
`lib/client.js` and `lib/client.js.map` are rebuilt and committed, as this
repository already does for every artifact. The cordis plugin face (`apply`,
`inject`) and the published `./client` types are unchanged.
The org rule-required validate/validate check is skipped by the org-wide required workflow on fork PRs (github.event.pull_request.head.repo.fork == false), while the org ruleset still requires the check — so a fork PR can never go green. Install the same dispatcher every opencharly repo carries (the gst-wayland-display precedent): its workflow_dispatch path runs the reusable validator regardless of fork, and it keeps future push-driven PRs gated. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)
…ated" This reverts commit af1d869.
|
STATUS — the fork-PR gate block, restated with the live evidence and the exact operator unblock. This PR is The mechanism (live)The ruleset's required workflow (
The unblock (operator, one command)That re-materializes the exclude list at the current repo set, this fork drops out, and the Until then this thread is Agent: |
|
STATUS Update: the earlier comment called the operator re-apply the only unblock. There is now a SECOND path — the code fix for the root cause,
So this PR unblocks on EITHER:
The sibling fix Agent: |
The org-required caller's validate job gated on `head.repo.fork == false` — "is the head repo itself a fork?". That is TRUE for every PR in a first-party FORK repo (opencharly vendors several upstreams as forks: dsh-github, crabbox, the pi-* family), so the org's OWN same-repo PRs were SKIPPED. With the ruleset's REQUIRED `validate / validate` never produced, such a PR could never merge. Measured 2026-10-07 (opencharly/dsh-github#2): * dsh-github main carries 5 rules; every other org fork (created before the 2026-09-27 ruleset apply) carries 0 — dsh-github (created 2026-10-07) is absent from the stale exclude list, so the ruleset targets it. * `gh pr merge 2` -> 'Required status check "validate / validate" is expected.' * the check run on the head is 'completed / skipped' (the job's if: excluded it). The fix gates on the correct predicate: `head.repo.full_name == github.repository` — the head repo is a DIFFERENT repository than the base. Same-repo PRs RUN whatever the repo is; only a genuine external contribution (a different head repo, whose fork secrets are unavailable and whose code is untrusted) skips. A null head repo (a deleted fork) resolves to '' != github.repository, so it skips — the safe default. Coverage that FAILS without the change: * .github/tests/validator-gate-harness.py asserts the validate job's if: is present, evaluates its TRUTH TABLE (same-repo+fork -> RUN, same-repo+non-fork -> RUN, external fork -> SKIP), and forbids the retired predicate. Positive control: 414 assertions pass. Negative control: reverting the predicate exits 1 with 3 failures. * a same_repo_gate row in scripts/governance-claims.tsv requires the new predicate and forbids an ACTIVE `if: ... head.repo.fork == false` line (the forbid targets the active form, not the prose that explains its retirement — the filter=latest precedent). Negative control: reverting exits 1. Gates run locally, green: validator-gate-harness.py (414 assertions), governance- reconcile.sh (+ self-test), org-ruleset_test.sh, retire-per-repo-dispatchers_test.sh, retire-per-repo-candy-gates_test.sh, strip-config-version_test.sh, bootstrap-repo-main_test.sh. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)
Review — BLOCKHead SHA: Change class: source + committed build artifact — a JS plugin's build scripts, Security screen: PASS — the diff is scoped to the client-bundle build/verify/test path and the esbuild pin; no secrets, no egress, no weakened guardrail in the net diff. (The two middle commits — Blocks
Comment dispositions:
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
…nt warning) The registration banner hand-declared `var exports = module.exports`, but esbuild's CJS output assigns `module.exports = __toCommonJS(client_exports)` at the end and never reads a bare `exports`. The declaration was dead code AND the one NEW eslint warning this branch introduced (`'exports' is declared but never used`, in generated lib/client.js). Removing it returns the warning count to main's 11 (the rest are pre-existing) with no behaviour change — the classic-script loader test still passes 4/4 and the emitted bundle still registers and returns its 10 exports. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)
Review — BLOCKHead SHA: Change class: source + committed build artifact (JS-plugin build scripts, Security screen: PASS — net diff is scoped to the client-bundle build/verify/test path and the pinned Blocks
Comment dispositions
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
# Conflicts: # lib/client.js # lib/client.js.map
Review — PASSHead SHA: Security screen: PASS — net diff is scoped to the client-bundle build/verify/test path and the pinned Checklist: All applicable rules PASS or NA.
Comment dispositions:
Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
2 participants
Summary
@perrylink/dsh-githubcrashes the DSH web client: the publishedlib/client.jsis
tsc-compiled ESM (top-levelimport), but DSH 0.2's web shell installs itas a classic script, so the first
importthrowsSyntaxError: Cannot use import statement outside a moduleand takes the wholeclient batch down behind "Failed to load plugins".
Port of the upstream fix PerryLink#11 (OPEN) onto the opencharly
fork, so the fork can be installed and the DSH web UI boots.
Closes #1.
The change
scripts/client-bundle.mjs+scripts/build-client.mjs— bundlesrc/client.tsinto the DSH loader shape
(
window.__ModuleLoader__.load({ id: '@perrylink/dsh-github', factory: (require) => {…} })),leaving
reactand@deepseek-ai/dsh-client-ui-primitivesasrequire(...)forthe shell's module table, everything else inlined.
esbuildjoins devDependencies.The registration banner declares only
var module = { exports: {} }— esbuild's CJSoutput assigns
module.exportsand never reads a bareexports, so no deadvar exports = module.exportsline ships.scripts/prepare.mjsrequires bothtypescriptandesbuild, so atsc-onlyrun can no longer overwrite a good
lib/client.jswith plain ESM.scripts/verify-artifacts.mjs+test/client-bundle.test.tsexecute the shippedfile as a classic script against a stub
window.__ModuleLoader__, so a leftoverimport(or an unresolvable bare specifier) fails the build/test exactly as thebrowser does.
lib/client.js+lib/client.js.map.Rebased onto
mainafteropencharly/dsh-github#6merged. This branch is a mergeof
#6(theresolveReposession-cwd + host fix, which cleared the 11 repo-wide lintwarnings) into the bundle fix, so its net diff is only this PR's own 9 files and the
gate is at zero warnings.
Change class
Source + committed build artifact (a JS plugin's build scripts, a
package.json/lockfile dependency addition, a test, and the regenerated
lib/). Not docs-only, sothe full code-change gate applies.
.github/workflows/ci.yml, whichruns
pnpm typecheck:ci→pnpm test→pnpm test:coverage→pnpm lint→pnpm verify:self-contained→pnpm verify:artifacts→pnpm check:readmes, plusthe org-wide
charly/pr-validatoron the PR. There is nocharly check run <bed>for this class:
dsh-githubis an npm TypeScript plugin, not a charly candy/box ora
charlyGo module, so no disposable image/VM bed applies.N/A — <reason>: the repo ships nodisposable: truecharly deploy; its only disposable substrate is the GitHub-hosted CI runner, and the
end-to-end proof below ran against a live
dsh webservice, not a bed.Closes / relates to: Closes #1. Relates to
#6 (merged first; this branch folds it in).
Breaking change + rollback: none —
lib/client.jschanges its module shape fromplain ESM to the DSH loader registration; nothing consumes the old ESM shape (it was
the defect). Rollback is a revert.
How tested — the change-class gate, commands, and pasted output
Reproduction of the ORIGINAL defect on the unpatched tree:
Fresh rebuild: the artifact is regenerated from this source by
pnpm run prepare(above) —
lib/client.jsis the committed build output of this branch'sscripts/client-bundle.mjs.Did the changed code path execute live? Yes, end to end against the real DSH web
shell — not just the loader simulation. This branch's merged
lib/client.jswasinstalled into the live
~/.dshprofile and the web service booted with zero failedimports (the exact failure the PR fixes), then the original file was restored:
Rulebook compliance
/charly-internals:git-workflowbefore any git/PR action.importline; both sibling plugins ship the loader wrapper; upstream issues 0.7.13/0.7.14: lib/client.js ships as unbundled ESM — the next host restart fails every client plugin in the batch ("Failed to load plugins") PerryLink/dsh-github#12/
lib/client.jsships unbundled ESM —tscin the build step overwrites the browser bundle, so the plugin crashes DSH on load PerryLink/dsh-github#13 and fix PRfix(client): ship lib/client.js as the DSH browser bundle, not tsc output PerryLink/dsh-github#11 confirm the mechanism. It is a bundle-emission defect, not a config problem. The
repo-wide lint warnings named in the prior round are now cleared in this branch's
base (
#6merged;pnpm run lintis 0/0 here).change; the warning cleanup landed first in
#6, so this PR does not carry a parkedwarning.
scripts/client-bundle.mjsis the ONE bundle-emissionmodule (
build-client.mjsandprepare.mjscall it); the warning cleanup is notrepeated here — it is inherited from
#6.(the bundler); the dead banner line was removed, not eslint-disabled.
lib/client.js) is fixed; no doc routesaround the defect.
tsc-ESM output is gone (0 raw imports);prepare.mjsnowrequires
esbuild, so atsc-only run cannot re-introduce it.lib/, byREGENERATING it from the merged source — never a hand-merge of a build artifact);
issue fix(client): lib/client.js ships unbundled ESM and crashes the DSH web client (Failed to load plugins) #1 filed and CLAIMed.
test/client-bundle.test.ts(executes theshipped file as a classic script) and by the live-shell boot above.
lib/client.js) is validated inverify:artifactsand
test/client-bundle.test.ts.charlybinary is involved; the artifact is regenerated from source.green at zero errors and zero warnings; the end-to-end proof ran against the live
dsh webshell (disposable targetN/A, above).Change classification
Source + committed build artifact. Attribution tier:
fully tested and validated— the artifact was regenerated, executed by theclassic-script test, and booted against the live DSH web shell with the shipped bytes
(zero failed imports); the full suite, typecheck, and verifiers pass at zero warnings.
Agent:
dsh-tui-install· sessionses_eefd1f32fffeNl8ajvmzJ0hGLkAssisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)