ci(fork): gate the Release workflow to upstream only - #4
Merged
Conversation
The Release workflow fires on a 3-hourly cron and on version tags, and publishes: the CLI to npm, plus a GitHub Release. Nothing in it was scoped to upstream, so it fired on this fork too, ~8 times a day. It published nothing only because every job requests Blacksmith runners the fork cannot schedule, so the runs hung in the queue forever. That is an accident of the ci-runners customization, not a safeguard — swapping release.yml onto GitHub-hosted runners, the obvious "fix" for the hung runs, would have turned this fork into a nightly publisher. Gate both entry points (check_changes, preflight) on the repository being pingdotgg/t3code. Every other job already requires preflight to have succeeded, so the gate cascades to all 10. The condition is true upstream, so upstream behaviour is unchanged and the hunk stays mergeable back. Deliberately independent of the runner labels: retiring ci-runners must never be able to start a release from the fork. Guard test asserts the property for every job rather than the two literal conditions, so an upstream sync adding a job that skips preflight fails CI instead of quietly reopening the path to publishing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
github-actions
Bot
added
vouch:trusted
NoahHendrickson
added a commit
that referenced
this pull request
Jul 26, 2026
Review findings #4 and #5. Four upstream identities survived the display rename: - macOS protocol handler display name was "T3 Code" — with the schemes deliberately shared, the handler picker showed two identical entries at the one moment a user must tell the apps apart. Now "N3 Code". - Linux executableName installed /usr/bin/t3code over the real app's binary; now n3code. StartupWMClass now matches the runtime class switch ("t3code-fork") instead of upstream's "t3code". - The staged package.json shipped name "t3code" / "T3 Code desktop build". The name is also Electron's fallback app name — the default userData directory before the runtime override lands — so upstream's value was exactly the shared Application Support directory the override avoids. Even the pre-override default is now fork-owned ("n3code"), which shrinks the userData race's blast radius to a fork-owned directory. - The release workflow still titled releases "T3 Code (fork)" and told users to de-quarantine "T3 Code (Alpha).app". Now N3 Code throughout, with release notes covering the empty-first-launch behaviour and v0.1.1 cleanup. resolveDesktopBuildIconAssets' two identical branches collapse to one return with the fence tight around the function; the in-app web UI keeping upstream's T3 branding is now documented as deliberate in customizations.yaml rather than reading as missed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 26, 2026
NoahHendrickson
added a commit
that referenced
this pull request
Jul 28, 2026
…t, named port Review round 1 (PR #32). The two blocking items and the decided ones: Guard rewritten so it can actually fail (review #1, #6a). Source assertions now run against the extracted fork:begin/end hunks and pin the subscription's input expressions and the derived port — the reviewer's mutation table (threadId: null, devServerPort = null, = 5173) now fails the suite, replayed and confirmed. The meta component is asserted on rendered output (attribute presence/absence, sr-only copy and ordering, and the `> svg` direct-child contract the stylesheet depends on — wrapping the icon now fails). CSS is read through cssRules.ts, so a behaviour-identical declaration swap passes where it used to fail. Manifest cost paragraph corrected (review #2): upstream's v1 row already makes this exact per-row subscription unconditionally, so v2 gaining it is parity, not a new class of cost. The per-environment multiplier (M environments → M streams, each host polling) is now disclosed instead of hidden behind "N rows share one stream". The lsof-fallback degradation (probe failure → terminal: null everywhere → pulse permanently off, silently) and the 1MiB truncation flicker are recorded as known limitations next to the external-shell one (review #5). The port is named, "dev server" dropped (review #3, #4): the row tooltip now carries `localhost:<port>` (+n overflow), v1's exact copy, and the accessible text says "Server listening on port N" — what the scanner actually knows — placed after the branch name so identity precedes transient state (review #6c). Slim rows no longer subscribe (Cursor review): the hook input is gated on variant === "card", so settled/snoozed rows pass null and skip the subscription instead of retaining the scanner for rows that cannot pulse. The repaint cost of animating a non-compositing property is now a stated yes in the stylesheet rather than an accident (review #6d). Globe2Icon imports as its own fenced statement because the phosphor guard parses upstream's import braces and a fence inside them reads as a binding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Why
Found while diagnosing #3, whose CI sat QUEUED for two hours. Same root cause, but this one is worse than a hung queue.
The Release workflow fires on a 3-hourly cron (
0 */3 * * *) and onv*.*.*tags, and it publishes — the CLI to npm (publish_cli) and a GitHub Release (release). Nothing in it was scoped to upstream, so it has been firing on this fork ~8 times a day.It has published nothing so far only because every job requests
blacksmith-*runners the fork cannot schedule, so the runs hang in the queue forever. That is an accident of theci-runnerscustomization, not a safeguard — and it makes the obvious fix actively dangerous: swappingrelease.ymlonto GitHub-hosted runners the wayci.ymlwas would have quietly turned this fork into a nightly publisher.What
Gate both entry-point jobs —
check_changesandpreflight— ongithub.repository == 'pingdotgg/t3code'. Every other job already requiresneeds.preflight.result == 'success', so the gate cascades to all 10 jobs; verified by parsing the workflow, 0 jobs remain reachable without it.The condition is true on upstream, so upstream behaviour is unchanged and the hunk stays mergeable back rather than becoming a permanent conflict.
The gate is deliberately independent of the runner labels. If Blacksmith is ever installed here and
ci-runnersis retired wholesale (as itsintentanticipates), that must not be able to start a release from the fork.Guard
releaseUpstreamOnly.test.tsasserts the property for every job rather than the two literal conditions, so an upstream sync that adds a job bypassingpreflightturns CI red instead of silently reopening the path to publishing. Verified non-vacuous: stripping the gate fails 2 of its 3 assertions.Registered in
.fork/customizations.yaml#release-upstream-only(tier 4,watch: .github/workflows/release.yml).Verification
vp test run apps/web/src/__fork_guards__— 33 passed (8 files)pnpm run typecheck(apps/web) — cleanvp fmt --checkon all three touched files — cleanrelease.ymlre-parsed as YAML: 10 jobs, triggers intact,ungated jobs: []Note
This stops the dead runs and keeps the fork from publishing. It does not give the fork its own release pipeline — if you ever want fork nightlies, that is a separate change and needs npm/release credentials this repo doesn't have.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
🤖 Generated with Claude Code