fix(eval): resolve tsx runtime from ancestor node_modules in worktrees - #210
Merged
Conversation
Fresh git worktrees have no node_modules of their own, so the hard-coded resolve(projectRoot, "node_modules", "tsx", "dist", "cli.mjs") path failed with "Could not find tsx runtime", breaking every `npm run eval:*` command in a worktree. Walk up ancestor directories (like Node's own module resolution) so the main checkout's install is found. A plain existsSync walk is used instead of require.resolve because tsx's package "exports" map does not expose dist/cli.mjs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
BigSimmo
enabled auto-merge
July 2, 2026 19:24
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:d7e59747fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:557a45d303
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
15 tasks
BigSimmo added a commit
that referenced
this pull request
Aug 2, 2026
…+8) (#1576) * DS V2 PR-E: answer safety, form foundation, announcements (slices 6+7+8) Build the five specified safety components so a degraded answer, an absent clinical value, an invalid field and a route change all become states the type system and assistive technology can see. Slice 6 - answer safety. VerificationNotice owns eight approved strings (four states x clinician/plain); AnswerCard requires both `state: AnswerState` and `verification`, and a degraded state cannot be constructed without the `onOpenSource` route that turns the caution into an affordance. DoseLine takes a structured dose model, composes Quantity, and marks overdue in three channels (amber rule + "Source review overdue" + StatusMark shape). MissingValue replaces the bare dash with four phrases. DateDisplay takes ISO only and renders <time>. AnswerFooter now names an absent field instead of dropping it - on a provenance strip the absence is the governance signal. answerClipboardText carries the degraded caveat out of the app and routes provenance through the one existing clipboardProvenanceLine() implementation. Slice 7 - FormField/FieldHint/FieldError/ErrorSummary. Hint and error are both in the DOM and both in describedBy when invalid; caller ids merge ahead of them rather than being overwritten; required/optional is label text; ErrorSummary takes focus rather than announcing. Slice 8 - LiveAnnouncer/RouteAnnouncer/announce. One singleton owns every announcement, with a dedupe window and a queue gap; route change moves focus to the new <h1> unless focus sits inside a dialog or a preserve-focus workflow. Step 0 contract pre-check passed with no change to src/lib/rag/** or source-review: ready, stale_evidence and source_only all project from fields the retrieval layer already decides, so no component infers staleness from a date. partial_retrieval has no producer and is recorded as such in SPEC, COMPONENTS, /issues #206 and tests/answer-state-contract.test.ts. All components are built, not registered - registration and product adoption stay in PR 13. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ds-v2): tighten answer-safety states after clinical governance review Eight findings from the PR 6 clinical governance review, all in code this branch introduced, all safety-positive. No product surface imports any of these modules yet, so nothing rendered changes. Counting and identity - answerStateFromRetrieval() counts and keys by document, not chunk. RagAnswer.sources is chunk-level, so the previous projection produced duplicate React keys, repeated a document as several overdue rows, and reported fractions that were wrong in both directions — including the direction that under-warns ("1 of 6" when half the documents are stale). - Where chunks of one document disagree on governance status, the more severe reading wins. Governance status carried, not collapsed - OverdueSource gains `status`, and DoseRow replaces the optional `overdue` boolean with a required `status: DocumentStatus`. An overdue status additionally requires `source`, so a warned row with no route back to the document is unrepresentable. `outdated` (superseded) now renders the slashed mark and the word "superseded" rather than the half-ring "review overdue" vocabulary of a document still in force. Cautions that cannot argue against themselves - stale_evidence with an empty overdue list now throws in development (mirroring the partial_retrieval guard) and, in production, states the caution without a count instead of "0 of 3 sources are past their review date". Same rule in answerClipboardText(). - VerificationNotice falls back to the most cautionary wording for an unrecognised state, and logs once, rather than failing open to `ready`. - The stale clinician wording says "some of which are past their review date" rather than asserting every cited source is overdue. - SourceOnlyBody no longer claims "nothing has been paraphrased": the tier is inferred from the routing mode and the extractive builder composes sections, so the surface cannot stand behind verbatim fidelity. Provenance precision - DateDisplay renders a date-only ISO as a date, even for kind="generated". It previously printed "14/03/2026, 08:00" for "2026-03-14" — a precision that was never recorded, on a provenance strip. Clipboard (exceeds the original SPEC slice-8 scope, deliberately) - answerClipboardText() now carries attribution and "Verify against the linked source documents before clinical use." on every state including `ready`, enumerates the cited documents, and suppresses the single-document provenance line where it would contradict a multi-source stale caveat. A copied answer loses the banner, the notice and the links; unattributed clinical prose in a record reads as clinician-endorsed. It remains narrower than formatAnswerRenderCopyText and is not a replacement for it. Recorded rather than fixed, so PR 13 cannot inherit them as clearance: AnswerState has no channel for an ungrounded answer (#208), the clipboard constraint above (#209), and the Gate 1 contrast pair for --warning at text tier plus the per-process logged-once Sets (#210). All three are also written into SPEC.md and COMPONENTS.md. Co-authored-by: Cursor <cursoragent@cursor.com> * chore(ledger): record PR-E answer-safety review at 8ad91e3 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ds-v2): resolve PR-E review findings for answer safety Filter staleness to cited supporting sources, reject rolled calendar dates, make duplicate production announcers a no-op, re-focus ErrorSummary on repeated submits, clear-then-set identical live announcements, and realign AnswerCard/DoseLine dtsPropsFor declarations. * fix(ds-v2): suppress duplicate LiveAnnouncer without lint cascade Keep production duplicate ownership in an effect with a targeted set-state-in-effect exemption, matching the repo pattern, so the duplicate never registers a listener and unmounts its live regions. * fix(ds-v2): address Copilot findings on banner and form tests Import ComponentProps from react, require exact overdue/sourceCount equality for the totality headline, and derive the partial-retrieval numerator from the rendered missing list. * fix(ds-v2): address CodeRabbit findings on answer-safety surfaces Merge the broken OverdueSource doc comment, omit clipboard provenance when metadata is absent, keep unidentified overdue sources as stale, give ErrorSummary an accessible heading name, fail over LiveAnnouncer ownership when the active instance unmounts, harden MissingValue phrase lookup against prototype keys, and treat sourceCount underflow as totality. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
cursorBot
pushed a commit
that referenced
this pull request
Aug 5, 2026
* fix(gates): catch lint and type errors before push, not in CI Two open PRs burned full CI cycles this week on defects a single local command would have caught: #1606 on a react-hooks/set-state-in-effect lint error, #1618 on a TS2339 for `mode.devOnly` (a union member that lacks the property, where app-modes.ts already exports the correct `"devOnly" in mode` guard). Neither lint nor typecheck was in the pre-push path. Typecheck could not simply be added, because it was already unusable (outstanding-issues #210). tsconfig.json's `include` carries `.next/types/**/*.ts` and `.next/dev/types/**/*.ts` — gitignored build artifacts — so deleting a page leaves the stale generated validator importing a removed module. Reproduced rather than inferred: a planted `.next/dev/types/validator.ts` referencing a removed mockup page yields `error TS2307: Cannot find module .../mockups/deleted-mockup-route/page.js`, base config exit 2, source-only config exit 0. Full source typecheck is clean (71s cold, 8.8s warm). Red locally and green in CI is how the gate got abandoned, which is how the real type error then reached CI. - tsconfig.typecheck.json + `typecheck:source`: identical compiler options, minus the `.next` globs, with a separate tsbuildinfo so the two incremental caches cannot invalidate each other. Route-signature validation is not lost; `next build` still covers it in CI. - guard-push.mjs gains a fourth guard running eslint over the pushed files and this typecheck. Verified to reproduce both defects above with CI-identical messages. Scoped to the lint roots and to pushes that touch TS, skips loudly when node_modules is absent rather than pushing people to GUARD_PUSH_DISABLE=1, and overridable with SKIP_STATIC_GUARD=1. Also corrects a doc claim that made #1580 surprising: "mockups are exempt" was being read as blanket. Mockups are exempt from the wiring and reachability gates and nothing else — they are still typechecked, and their client chunks still count toward check:bundle-budget, which totals every built chunk rather than the initial production bundle. That the budget's scope contradicts ledger #13's "not an initial production bundle" position is a real unmade decision, now recorded as #237 rather than papered over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * docs(issues): capture the phone Category soft-menu fix salvaged from PR #1606#1606 is closed, but it carried the one fix nothing else in the queue provides: MobileResultFilterControl's native <select> paints a harsh system-blue highlight on phones, and #1615 keeps that native select (its change is the iOS 16px anti-zoom rule). So the fix does not survive #1615 landing. Records it as #238 with the two defects the redo must not repeat: the unresolved keyboard trap on disabled options, and the set-state-in-effect lint error that PR #1620's new pre-push guard would now catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy * issues: capture #239 stale Cloud acceptance pin on PR #1617, #240 remote-container browser gate drift * Tighten guard coordinator test Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): shared source-typecheck lease and safer static pre-push Treat typecheck:source:internal as a shared read-only coordinator lease with a distinct per-worktree buildinfo file, drop the pinned in-repo cache path, and harden staticGuard: acquire a short exclusive lease (fail-open when busy), use a private eslint cache, escalate lint on eslint policy changes, fail closed when the push tip is not HEAD, cover eslint-rules, and add Vitest coverage. Align hook/docs wording with the fourth guard and point CLAUDE.md at #252. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): keep tsBuildInfoFile when run-heavy has no npm_execpath Pre-push invokes run-heavy via plain node, so the npm_execpath spawn path was skipped and the fallback dropped effectiveForwarded — undoing the per-worktree buildinfo injection. Also warn when staticGuard passes on a dirty working tree. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: refresh scripts-index for lint:changed:internal Keep docs:check-inventory green after adding the pre-push eslint wrapper script to package.json. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(gates): address Devin findings on static pre-push guard - Treat "Database focused-test capacity is full" as coordinator busy so shared typecheck slot exhaustion fails open instead of faking a type error. - Skip source typecheck when every changed .ts path is excluded by tsconfig.typecheck.json (edge functions, archive, scratch, worktrees). - Restore check-github-shell-access.mjs (and its Role notes) in the scripts index. * chore(ledger): record PR #1620 babysit * fix(gates): emit structured heavy-run admission-busy signal Prefer exit 75 + DATABASE_HEAVY_RUN_ADMISSION_BUSY over prose matching so tsc/eslint output that quotes busy strings cannot false-pass the static guard. * fix(gates): tip-check only when static work runs; isolate typecheck cache Addresses follow-up Devin on PR #1620: - Reorder staticGuard so tip-vs-HEAD fails closed only when lint/typecheck will actually read the working tree; ignore tag refs in the tip check. - Pin a distinct tsBuildInfoFile on tsconfig.typecheck.json so direct tsc does not collide with the base config cache (run-heavy still overrides). * fix(gates): keep lint failures when typecheck admission is busy Addresses Devin on PR #1620 — a prior eslint failure must still block the push if the follow-up source typecheck cannot get a coordinator slot. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
4 tasks
BigSimmo pushed a commit
that referenced
this pull request
Aug 6, 2026
…tion flake sighting) and widen #210
cursorBot
pushed a commit
that referenced
this pull request
Aug 7, 2026
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
cursorBot
pushed a commit
that referenced
this pull request
Aug 7, 2026
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo added a commit
that referenced
this pull request
Aug 7, 2026
* fix: close high-win Grok batch (tokens, a11y, perf, gates) - Drop unused --med-accent-soft (#157) - Document Chip compact=11px / standard=12px (#220) - Reveal phone chrome on source change at top (#176) - Passive wheel listener for modifier-gated PDF zoom (#214) - Point npm run typecheck at source-only tsconfig (#210) - Fail outstanding-issues when queue cites non-open IDs (#201) Also confirms MatchExplanationChips, Favourites empty-state, DocumentFrame role=alert, and refetching dimming claim are already resolved on main (#223/#225/#219/#246). * issues: archive grok-batch wins #157#176#201#210#214#219#220#223#225#246 Prune them from the recommended queue so the new queue-membership gate stays green. * fix: address PR #1651 review (double-zoom, issues:done, tokens) - Keep viewer wheel listener non-passive while wheelZoom is on and preventDefault on Ctrl/⌘+wheel / trackpad pinch so PDF zoom does not also zoom the browser page (Sentry/Devin). - Prune recommended-queue ID citations in resolveIssue so issues:done stays writable under the #201 gate. - Drop restated Chip pixel values from TOKENS.md; amend #214 archive note. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix: recompute issues-queue Order boundary after deletions Devin: pruneResolvedIdFromQueue used a pre-splice openStart for renumbering, so enough solo-queue deletions could rewrite Open items headers/ids. Recompute the section limit after edits and cover it in the writer self-test. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: record Run PR sweep ledger for PR #1651 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 12, 2026
…system rows Archived as verified-delivered or duplicate: #303, #284 (dup of #296), #149, #159, #286, #287, #288, #256, #261, #291. Re-scoped with re-measured evidence: #210 (gate was never missing; the defect is the .next/dev types include), #262 (aliases 228 -> 193), #266 (counts moved, 23 unadopted holds), #275 (premise 'only one implementation' is false — the values leaked to five files). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
BigSimmo pushed a commit
that referenced
this pull request
Aug 12, 2026
#248 — settled without touching live. check:drift does cover the missing-index class: all four 20260705180000 indexes are in schema.sql and in the drift manifest, indexes compare by name on table + def_hash, and the drift allowlist is empty, so a run would have failed with four missing_live findings. The gap is cadence, not coverage — live-drift.yml is weekly plus manual and gates nothing. Also records a genuinely uncovered sub-class: schema_drift_snapshot() reads pg_get_indexdef but never indisvalid/indisready, so an index left invalid by a failed CREATE INDEX CONCURRENTLY compares byte-identical while the planner cannot use it. The live forensic (partial apply vs manual drop vs history repair) remains operator work behind the approved window. #210 — half already fixed, and its prescribed fix refuted. npm run typecheck has used tsconfig.typecheck.json since 450690f, which excludes .next entirely; verified green with .next/dev/types/validator.ts present. Dropping the dev-types glob from tsconfig.json does not hold: Next 16 emits it deliberately and writeConfigurationDefaults pushes it back into an existing include on every next dev/build. What remains is narrower — the Playwright isolated tsconfig inherits the repo-root globs, confirmed by probe, and Next's own dev-types filter does not apply because useTypeScriptCli defaults true. Recorded as not yet proven end-to-end. Also records the session-start marker defect fixed in the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STG6AU5J4gxrFJagP4pRti
BigSimmo pushed a commit
that referenced
this pull request
Aug 13, 2026
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
BigSimmo added a commit
that referenced
this pull request
Aug 13, 2026
Correct the false claim that Next mutates the isolated Playwright tsconfig. Next 16.3 returns early for configs with extends, while the default CLI checker still honours the inherited root includes. Preserve that narrower open hypothesis and add the immutable exact-head review record.
1 task
BigSimmo added a commit
that referenced
this pull request
Aug 13, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 13, 2026
BigSimmo added a commit
that referenced
this pull request
Aug 13, 2026
…r7k9 docs(issues): correct #210 — the prescribed fix would be reverted by Next
This was referenced Aug 14, 2026
BigSimmo pushed a commit
that referenced
this pull request
Aug 14, 2026
…merged #215, #210, and #293 were queued as `done` in this same PR, each citing a PR (#1949, #1953, #1962) that turned out still to be open, not merged — verified by checking whether each PR's actual file change is present on origin/main (none are). Reconciling the original `done` requests would have closed these rows while their fixes exist only on unmerged branches, one of which (#1949) currently has failing required CI. Converts all three to `update` requests documenting the correction and the real current state, so reconciling this PR now cannot close a row before its fix has actually landed. Leaves #98, #189, and #194 as `done` unchanged — those cite PR #1950 and PR #1947, both confirmed merged into main.
BigSimmo pushed a commit
that referenced
this pull request
Aug 14, 2026
scripts/run-playwright.mjs writes an isolated tsconfig.json (extends: "../../tsconfig.json") for each Playwright run root, but left include/ exclude unset. TypeScript resolves an extended config's *inherited* relative include/exclude entries against the base config's own directory (the repo root), so the isolated config still resolved ".next/dev/types/**/*.ts" (and the broad "**/*.ts") against the shared top-level .next/ directory rather than this run's own NEXT_DIST_DIR output under its dist/ folder. Empirically confirmed with a stale .next/dev/types/*.ts fixture file: before this change, `tsc --showConfig` on the generated child config resolved that repo-root file into the isolated run's file list; after this change it does not. The child config now declares its own include (repo source globs plus this run's own dist/types and dist/dev/types) and exclude (mirrors the root tsconfig's exclude list plus the repo-root .next/**). Verified this is not clobbered by Next's own tsconfig auto-config: writeConfigurationDefaults() returns immediately when the parsed config has "extends" (confirmed by reading node_modules/next/dist/lib/typescript/writeConfigurationDefaults.js). Hashed the generated child tsconfig.json immediately after write and again after a full `next build --webpack` + server start + Playwright test run (npm run test:e2e:pr equivalent, single focused spec) — both hashes match byte-for-byte (sha256 1c744da9634c2d712b1fd9ae428a5808fadf82b9d623d0ce30ca923b56a3ce4a). Ledger #210 (re-scoped 2026-08-13): items 1-2 already closed/refuted; this closes item 3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
BigSimmo added a commit
that referenced
this pull request
Aug 14, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
scripts/run-eval-safe.mjs(the process-guard wrapper behind everynpm run eval:*command) hard-coded the tsx CLI path asresolve(projectRoot, "node_modules", "tsx", "dist", "cli.mjs"). Fresh git worktrees have nonode_modulesof their own, so this failed withCould not find tsx runtime at …, breaking all eval commands when run from a worktree.Fix
resolveTsxCli()walks up ancestor directories (like Node's own module resolution) and returns the firstnode_modules/tsx/dist/cli.mjsit finds — the worktree's own install when present, otherwise the main checkout's install (worktrees live inside the repo directory). A plainexistsSyncwalk is used deliberately instead ofrequire.resolve, because tsx's packageexportsmap does not expose thedist/cli.mjssubpath.The error message now points at the real remedy (
npm install/npx tsx) instead of a specific missing path.Verification
node --check scripts/run-eval-safe.mjs— syntax OK.node scripts/run-eval-safe.mjs scripts/check-runtime.tsfrom a worktree with no localnode_modules: resolved tsx from the main checkout and printed[Runtime Check] PASS.🤖 Generated with Claude Code