fix(coil): stop shipping upstream's update feed, which we never actually disabled - #99
Merged
Merged
Conversation
…lly disabled The check restored in #97 failed on its first real run, on both platforms, and it was right to. `app-update.yml` is in the bundle: .../T3 Coil (Alpha).app/Contents/Resources/app-update.yml .../win-unpacked/resources/app-update.yml It is in the installed build too — 0.0.33-coil.105 on disk carries `owner: radroid / repo: t3code / provider: github / releaseType: release`. So has every build before it. This is not a regression #97 introduced; it is what the old check could not see. WHY IT WAS NEVER DISABLED. The workflow set `GITHUB_REPOSITORY: ""` and its comment claimed that made `resolveGitHubPublishConfig` return undefined. Actions refuses to let a workflow set any `GITHUB_`-prefixed variable, so the line is inert. The build read the runner's real `radroid/t3code`, resolved a publish config, and electron-builder wrote the feed. The shipped file proves the path taken: those owner/repo values cannot come from any other branch of that function. `T3CODE_DESKTOP_UPDATE_REPOSITORY` is the fork's own hook, is read FIRST, and is not reserved. Set to `disabled` it short-circuits `GITHUB_REPOSITORY` before that is read, fails the `owner/repo` split, and returns undefined — the no-publish path. No publish config, no `app-update.yml`, `hasUpdateFeedConfig` false, and upstream's electron-updater switches itself off. Which is what the empty string was always meant to do. WHY THIS WAS URGENT RATHER THAN MERELY WRONG. `releaseType: release` kept the second updater inert by accident: every fork release was a GitHub prerelease, so it could never find anything to install. #96 has just made releases full releases with a real Latest pointer. The next successful publish would have been the first thing that dormant updater could see, in every installed build. Tests pin both directions — that the hook produces no config, and that GITHUB_REPOSITORY alone still produces one, so nobody later reads the hook as redundant. They live in the fork's test file; the function is exported, so this costs no seam row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
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.
The check restored in #97 failed on its first real run, on both platforms — and it was right to.
What it found
app-update.ymlis in the bundle:It is in the installed build too.
0.0.33-coil.105on disk right now contains:So has every build before it. This is not a regression #97 introduced — it is what the old check could not see, because it searched
release/for a file that only ever exists inside the.app.Why it was never disabled
The workflow set
GITHUB_REPOSITORY: "", and its comment claimed that maderesolveGitHubPublishConfigreturn undefined and no feed get written.Actions refuses to let a workflow set any
GITHUB_-prefixed variable, so that line is inert. The build read the runner's realradroid/t3code, resolved a publish config, and electron-builder wrote the feed.The shipped file is the proof of which branch ran —
owner: radroid,repo: t3codecannot be produced by any other path through that function.The fix
T3CODE_DESKTOP_UPDATE_REPOSITORYis the fork's own hook, is consulted first, and is not reserved:Set to
disabledit short-circuitsGITHUB_REPOSITORYbefore that is read, fails theowner/reposplit, and returns undefined — the no-publish path. No publish config, noapp-update.yml,hasUpdateFeedConfigfalse, upstream's electron-updater switches itself off. Which is what the empty string was always meant to achieve.Depending on a parse failure would be fragile alone. It is not alone: the restored check asserts the outcome on every release, so if that parser ever loosens, the release goes red instead of shipping.
Why this was urgent rather than merely wrong
releaseType: releasekept the second updater inert by accident: every fork release was a GitHub prerelease, so electron-updater could never find anything to install.#96 has just made releases full releases with a real Latest pointer. The next successful publish would have been the first thing that dormant updater could see — in every installed build, alongside the fork's own toast. That is issue #47's failure mode, arriving on its own.
Tests
Pin both directions: that the hook produces no config, and that
GITHUB_REPOSITORYalone still produces one — so a later reader cannot conclude the hook is redundant and remove it. They live in the fork's test file;resolveGitHubPublishConfigis exported, so this costs no seam row.Lint, typecheck and the suite are clean locally.
🤖 Generated with Claude Code