feat(dev): stop the open PR list lying, in the two directions it does - #872
Open
divshekhar wants to merge 5 commits into
Open
feat(dev): stop the open PR list lying, in the two directions it does#872divshekhar wants to merge 5 commits into
divshekhar wants to merge 5 commits into
Conversation
…s filesystem A field install took 17m45 from the first command to the first Reticle tool call. Four blocks account for 12m45 of it, and this fixes them. THE PROJECT ID HAD TWO SOURCES OF TRUTH. `init` writes it to `.reticle.json`; the Vite plugin never opened that file and re-derived its own from `pkg.name + sha1(absolute root)`. The two agreed only while the plugin's cwd matched the directory `init` ran in, which a containerised dev server breaks: the page announced one project, the daemon expected another, and the bridge refused every connection. `resolveProjectId` now reads the file `init` already wrote, so the halves agree by construction rather than by coincidence. A REFUSED TOKEN NAMED NOTHING. The plugin reads-or-mints the pairing token from `$HOME/.reticle`, which inside a container is the image's throwaway root, so it minted its own and every page was refused with `authentication failed`. The cause was found by grepping the plugin's shipped `dist/` inside the container for an environment variable documented nowhere. The close reason now says the token is wrong rather than that authentication failed, and `reticle status` carries the full diagnosis: the daemon had the evidence the whole time and the branch that renders it was gated on `!initialized`, so it could never fire for a project whose config the daemon was standing in. THE STAMP CRASHED EVERY REACT-THREE-FIBER APP. `sourceMapping` stamped `data-reticle-source` on any lowercase JSX tag. R3F's are three.js objects, and `applyProps` walks a dashed prop as a property path, throws from the commit phase, and unmounts the whole app to a white screen. It fires on an element UPDATE, so the reporter's app ran ~20 minutes and passed ~15 verdicts first, which made an instrumentation bug read as an application bug. The babel plugin now stamps DOM tags only; `init` turns the option off outright when the manifest names a non-DOM reconciler, because `line` and `audio` are both real DOM tags and three.js classes and no build-time rule can separate them. A DRIVE LEFT NO ARTIFACT. 89 act_and_wait plus 15 assert calls, `lastPushAt: null`, an empty dashboard. `.reticle/runs/` had one writer, the flow-replay path, so driving the app produced nothing for the sync daemon to send. Session teardown now folds the journal it already flushed into the same artifact. It refuses to speak for `unknown` and `no-fault`: those are counted in the note, never as passes, and a session that proved nothing writes no run at all, because a run with no checks computes to PASS and a green meaning "not measured" is the one output this product must never produce. Also: `init` now says what a containerised dev server does differently BEFORE it goes wrong (the rebuild, and the token mount), and `<command> --help` answers with usage instead of `unknown argument '--help'` on exit 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
…s own Reading the id of record introduced a walk, and a walk that goes too far adopts somebody else's identity: an app in a monorepo with no config of its own would have taken the id from a `.reticle.json` six directories above it and announced a project it is not. The server's own config walk already caps at six for exactly that reason. Matching it is not a coincidence to be tidied later: the whole point of reading this file is that the plugin and the CLI stop disagreeing about which project this is, and two walkers with two depths is the same defect one level up. The package.json walk keeps its depth. A package name only ever BUILDS an id and never adopts one, so an over-eager walk there costs a less specific name rather than the wrong app's identity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
Session teardown fires on every socket close, and a reconnecting tab keeps its session id and goes on appending to the same journal. Folding that ledger with a fresh random run id each time would publish a row per reload, each one a superset of the last, so a single drive across three reloads would reach the dashboard as three overlapping verifications of the same work. The field session this whole change came from reloaded the page repeatedly. The id is derived from the session instead, so the fold is idempotent: the session rewrites its own artifact, and the cloud diffs by runId, so a re-push supersedes rather than duplicates. A session id is a free string on the wire, so the derived id is checked before it becomes a path segment and anything unsafe falls back to a random one, which loses idempotence for that session rather than refusing to record it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
Every mistake this fixes is one I made today, repeatedly, and none of them was a coding error. `check-stale-issues.mjs` already guards the issue list against a fix that landed. These are its mirror image, about the PR list, which nothing reads. TEN open PRs turned out to be already merged under a different commit — some open two weeks, one merged under a commit literally titled "Merge PR #645" that GitHub still never closed. I found them one at a time, by hand, over hours. Checked against real history, `landedPullRequests` finds all four of the ones I can still name in a single call. NINETEEN of 53 open issues had an open PR nobody could see. GitHub surfaces that nowhere a reader notices, and contributors' titles rarely carry the issue number, so grepping the PR list for `#NNN` finds nothing — which is exactly why my own greps kept coming back empty. Four people duplicated each other in one day because of it, two of them maintainers, and in one case the maintainer's version was the WORSE fix: it would have closed #680 while a webpack 4 app still failed to compile, because it missed that `@reticlehq/core` ships into the same bundle. Both answers are HINTS and the script never gates. A title match is evidence that something carrying that subject landed, not proof this PR did, so the report says to verify with `git merge-base --is-ancestor` rather than telling anyone to close a contributor's work on a substring. A guard that overstates gets switched off, and then catches nothing. `issuesWithOpenPr` deliberately counts a BARE `#5`, the opposite call from `closedRefsIn` next door. There the question is what a commit CLAIMS to close and over-counting cries wolf; here it is whether someone about to start would want to know this exists, and "related to #5" is exactly as informative as "closes #5". `MIN_TITLE_CHARS` exists because a two-word title substring-matches half of any history, and the first false positive is what teaches a reader to ignore the check. Carries a `--self-test` negative control, for the reason its sibling has one. Also written into CLAUDE.md, because a mechanism nobody knows about is not a mechanism: - check the PR list by SYMPTOM before starting an issue; - do not push to `main` while a deep queue is open — every push sends every PR BEHIND and any touching CHANGELOG.md conflict-dirty, which is how ten queued PRs became a treadmill; - a before/after measurement is invalid unless both sides built clean (`tsconfig.tsbuildinfo` survives `rm -rf dist`, so `tsc -b` emits nothing and a "48x smaller" bundle is a broken build, not a result — the honest figure was +2.6%); - never grep source for syntax, parse it (a regex guard reported two false failures on correct output; both hits were inside comments, one quoting the code an earlier fix had removed). Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
divshekhar
force-pushed
the
fix/pr-list-tells-the-truth
branch
from
September 8, 2026 18:17
d3fccb5 to
85a5b13
Compare
divshekhar
enabled auto-merge (squash)
September 8, 2026 18:17
The PR that introduces this check lists eighteen issue→PR pairs in its body — as EVIDENCE that the problem is real. `issuesWithOpenPr` then counted it as a claimant on all eighteen, so every issue came back with two PRs against it and the report became noise on the day it shipped. That is the exact failure the module's own header warns about: "a guard that overstates gets switched off, and then catches nothing at all." I wrote that sentence and then shipped the thing it describes, which is a good argument for running a check against live data before believing it. A PR that FIXES something references one issue, sometimes two or three. A PR referencing many is describing them: a tracking issue, a triage sweep, a release note. Above `MAX_CLAIMED_ISSUES` the PR is skipped. Five, not three, because #429 legitimately closes #428, #430 and #433 — a cap that caught honest multi-issue work would be the same failure pointed the other way. Counted over EVERY reference, before narrowing to open issues. Filtering first would let a long listing of mostly-closed issues slip under the cap and be reported as a claim on the two that happen to still be open. Live re-run is clean: #872 no longer appears against anything, and #429's three real claims survive. Also one yoda in the script itself. Note that `pnpm lint` currently reports three more in `daemon/daemon-resilience.ts` and `daemon/proxy-disconnect-storm.test.ts` — those are another session's uncommitted work in this shared checkout, not this branch's, and are deliberately left alone. Signed-off-by: Divyanshu Shekhar <imdshekhar@gmail.com>
Contributor
Author
|
Pushed a fix for a false positive this guard produced on its own first live run — worth recording, because it is the failure the module's header warns about and I shipped it anyway. This PR's body lists eighteen issue→PR pairs as evidence. A PR that FIXES something references one issue, sometimes two or three. A PR referencing many is describing them — a tracking issue, a triage sweep, a release note. Above Two decisions in that:
Live re-run is clean: this PR no longer appears against anything, #429's three real claims survive. The general lesson, which is the reason I am writing it here rather than just fixing it: a guard is not finished when its unit tests pass — it is finished when it has run against real data and said nothing wrong. Mine had 13 green tests and was still unusable on first contact. |
Contributor
Author
|
Flagging a packaging problem rather than the code, which is fine.
It is currently buried two levels down in a commit titled "the install works where the dev server is not on this filesystem", on a PR titled "stop the open PR list lying". Nobody reviewing this PR will know it is here, and if this branch is reworked the fix goes with it. I have extracted it unchanged onto Suggested resolution: drop the three |
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
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.
Every failure this fixes is one I made today — repeatedly — and none of them was a coding error.
What it catches
check-stale-issues.mjsalready guards the issue list against a fix that landed. These are its mirror image, about the PR list, which nothing reads.1. A PR whose work is already on
main. Ten were closed by hand today, some open two weeks, each already merged under a different commit. #645's own merge commit saysMerge PR #645and GitHub never closed it. Verified the check against real history — it flags all four of the ones I can still name:2. An issue whose PR nobody can see. GitHub surfaces that nowhere a reader notices, and contributors' titles rarely carry the issue number, so grepping the PR list for
#NNNfinds nothing — which is exactly why my own greps kept coming back empty. Live run reproduces the mapping I built by hand:Four people duplicated each other in one day because of this, two of them maintainers — and in one case the maintainer's version was the worse fix, which would have closed #680 while a webpack 4 app still failed to compile.
Design decisions worth arguing with
git merge-base --is-ancestorrather than telling anyone to close a contributor's work on a substring. A guard that overstates gets switched off, and then catches nothing.issuesWithOpenPrcounts a BARE#5— the opposite call fromclosedRefsInnext door. There the question is what a commit claims to close and over-counting cries wolf; here it is whether someone about to start would want to know this exists, and "related to [security] H2: enterprise license gate is trivially bypassable (not baked into source) #5" is exactly as informative as "closes [security] H2: enterprise license gate is trivially bypassable (not baked into source) #5".MIN_TITLE_CHARSbecause a two-word title substring-matches half of any history, and the first false positive is what teaches a reader to ignore the check. There is a test that a shared conventional-commit prefix does not match.--self-testnegative control, for the reason its sibling has one.Also in CLAUDE.md
A mechanism nobody knows about is not a mechanism. Four rules, each with the incident behind it:
mainwhile a deep queue is open — every push sends every PRBEHINDand any touchingCHANGELOG.mdconflict-dirty, which is how ten queued PRs became a treadmill nobody could drain;tsconfig.tsbuildinfosurvivesrm -rf dist, sotsc -bemits nothing and my "48x smaller bundle" was a broken build, not a result (honest figure: +2.6%);Gates
pnpm format:check,pnpm lint17/17,pnpm typecheck22/22,pnpm test:unit(673 server files)pnpm check:open-prs:self-testNot run:
test:e2e— this adds no tool surface and no runtime code path.