[FIX] Make /connect capture page work + unblock canopy-admin CI/deploy (Node 22, audit) - #3
Merged
Merged
Conversation
…ount guard
The web-hosted connection capture page (renderConnectPage in
connections-routes.js) is server-rendered — every element the widget touches
already exists in the DOM. But the widget's main() bailed early unless it also
found an element with id="connect-root":
const root = document.getElementById('connect-root');
if (!root || !statusEl) return;
No page ever emits connect-root (grep confirms it was referenced only here), so
root was always null, main() returned before adding the form submit handler, and
the Connect button silently did nothing — the whole point of the flow (capture a
key from any browser) never worked. root was otherwise unused; status-message is
the real "are we on the connect page" sentinel and the form controls are
re-checked right before the handler is wired up.
Add a regression test that parses the widget's own getElementById lookups and
asserts renderConnectPage emits each id, so the page and widget can't drift apart
silently again (this exact bug is otherwise invisible until someone loads the
real page). Verified: the test fails against the pre-fix widget with a precise
diagnostic and passes after. Full suite 54/54, build and lint clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AyPaJTiYLvCp5N6GiKZDS
… CI + deploy) Two pre-existing failures were blocking every canopy-admin PR and the Cloud Run deploy, independent of the /connect widget fix on this branch: 1. Node 20 can't run the test suite. CI (security.yml) and the deploy's verify steps (deploy.yml) both pinned `node-version: '20'`, but `connect-widget.test.js` imports `src/connect-widget/main.ts` directly, and Node 20 cannot load `.ts` (ERR_UNKNOWN_FILE_EXTENSION — native type stripping needs Node >=22.6). The repo's own dependencies already require Node 22 (react-router@8.3.0 → >=22.22.0), and canopy standardizes on Node 22, so pinning 20 was the bug. Bump both workflows to '22', `.nvmrc` to 22, and engines to >=22.12.0 to match. 2. `npm audit --audit-level=high` failed on 4 high advisories (multer, nanoid, brace-expansion, browserslist). `npm audit fix` (no --force) clears all 9 within existing semver ranges — package-lock.json only, package.json deps untouched. multer (runtime, multipart uploads) moves to 2.3.0. The Dockerfile build/runtime stay on node:20-alpine intentionally: they don't run the .ts test, and react-router is client-bundled (never executed in node at runtime), with no engine-strict to hard-fail install. Verified on Node 22: audit --audit-level=high exits 0, 54/54 tests, build, lint all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014AyPaJTiYLvCp5N6GiKZDS
ScottieR
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes the web-hosted
/connect/{token}capture flow actually functional and gets canopy-admin CI + the Cloud Run deploy green. Consolidates what were three separate fixes (this PR supersedes #4, now closed).1. The
/connectpage was non-functional (the operational bug)The page is server-rendered by
renderConnectPage, but the widget'smain()bailed unless it found aconnect-rootelement that no page ever emits (grep-confirmed: referenced only on that line). Sorootwas alwaysnull, the submit handler never attached, and the Connect button silently did nothing — the pasted key was never encrypted or sent.src/connect-widget/main.ts— drop the deadconnect-rootguard; key offstatus-message(which the page renders). Form controls are already re-checked before the handler wires up.connections-routes.test.js— regression test that parses the widget's owngetElementById(...)calls and assertsrenderConnectPageemits each id, so the page↔widget contract can't drift silently again. Verified it fails on the pre-fix widget and passes after.2. CI/deploy ran on Node 20, which can't run the test suite
connect-widget.test.jsimportssrc/connect-widget/main.tsdirectly, and Node 20 cannot load.ts(ERR_UNKNOWN_FILE_EXTENSION; native type stripping needs Node ≥22.6). Bothsecurity.ymlanddeploy.yml's verify steps pinnednode-version: '20', sonpm testfailed onmainand on every PR — a pre-existing red. The repo's deps already require Node 22 (react-router@8.3.0→>=22.22.0) and canopy standardizes on 22.node-version: '22',.nvmrc→ 22,engines→>=22.12.0.node:20-alpineintentionally: it doesn't run the.tstest, react-router is client-bundled (never executed in node at runtime), and there's noengine-strictto hard-fail install. Flagging for a future consistency bump, not needed for this deploy.3.
npm audit --audit-level=highwas failing the gate4 high advisories (
multer,nanoid,brace-expansion,browserslist).npm audit fix(no--force) clears all 9 within existing semver ranges —package-lock.jsononly,package.jsondeps untouched.multer(runtime, multipart uploads) → 2.3.0.Verification (Node 22)
npm audit --audit-level=highexits 0 (0 vulnerabilities)npm test— 54/54 pass (previously 1 fail: the.tsload error)npm run build— clean ·npm run lint— cleanPath to production
Merge to
main, then promotemain → productionto trigger the Cloud Run deploy. The companion canopy change (browserslistbump for the deploy's "Verify Canopy" step + v0.4.0 prep) is ScottieR/canopy#119.🤖 Generated with Claude Code
https://claude.ai/code/session_014AyPaJTiYLvCp5N6GiKZDS