fix: unblock the nchat desktop build (nine layers) - #181
Merged
Merged
Conversation
PR Checks Summary
✅ All checks passed! Ready for review. |
acamarata
changed the title
Security Scan Results: PASSED
|
Dependency ReviewThe following issues were found:
|
Security Scan Results: PASSED
|
1 similar comment
Security Scan Results: PASSED
|
acamarata
added a commit
that referenced
this pull request
Sep 14, 2026
* ci: make PR checks actually run on shared-package changes Every job in pr-checks.yml is gated behind a dorny/paths-filter, and none of the filters covered the repo's root packages/ directory. A PR touching only packages/ therefore skipped Lint, Type Check, Unit Tests and Build Check. Measured on PR #181, which changes two tsconfigs under packages/: 12 of its checks reported SKIPPED and nothing compiled the code it changed. - packages: also match packages/**, pnpm-workspace.yaml, pnpm-lock.yaml - web: matched frontend/apps/web/**, which does not exist; the web surface is the root web/ directory The mobile: and desktop: filters matched four paths that are absent from the repo -- frontend/platforms/{capacitor,react-native,electron,tauri}. The real directories are frontend/platforms/mobile and desktop/. Those two filters could never match, so Mobile Build Check and Desktop Build Check have never produced a verdict; had they matched, both would have failed, because each cds into one of the missing directories. Both jobs duplicate a dedicated workflow that already targets the real path: build-capacitor.yml builds frontend/platforms/mobile, and desktop-linux.yml, desktop-macos.yml and desktop-windows.yml build desktop/. Removing the two dead jobs drops no coverage that existed. Still uncovered and deliberately not papered over here: frontend-vite/ matches no filter, but every job in this workflow runs with working-directory: frontend, so adding a filter for it would run the wrong app's build and report a green that means nothing. It needs its own job. * ci: cover root packages/ in ci.yml's filter too ci.yml runs on push to main and gates its jobs with the same paths-filter shape as pr-checks.yml, including the same 'packages:' entry that only matched frontend/. A push touching only the shared packages/ directory skipped lint, type-check, test and build on main as well as on PRs. frontend/pnpm-workspace.yaml does not exist; the workspace manifest is at the repo root.
acamarata
force-pushed
the
fix/ui-dts-ts5101
branch
from
September 14, 2026 22:32
2cb6343 to
4679fca
Compare
acamarata
changed the title
Security Scan Results: PASSED
|
2 similar comments
Security Scan Results: PASSED
|
Security Scan Results: PASSED
|
The "Build workspace packages" step fails, and with it Desktop — Linux,
Desktop — Windows and Desktop — macOS. Those desktop builds have not succeeded
on any run examined back to 2026-08-03. None of them is a required check on main
(required: Gitleaks, CodeQL Analysis), which is why six weeks passed unnoticed.
Two independent TypeScript 6 breakages in the same step.
1. packages/ui — TS5101, and it is NOT our config.
error TS5101: Option 'baseUrl' is deprecated and will stop functioning in
TypeScript 7.0
No tsconfig in this repo sets baseUrl. `tsc --showConfig -p
packages/ui/tsconfig.json` shows none, and plain `tsc --noEmit -p
packages/ui/tsconfig.json` passes clean — the ESM and CJS builds pass too, and
only the declaration build dies.
tsup injects it. tsup 8.5.1, dist/rollup.js:6837:
baseUrl: compilerOptions.baseUrl || ".",
It sets baseUrl unconditionally on the options it hands its dts worker,
defaulting to "." when the project has none. TypeScript 6 makes that an error,
so every tsup declaration build fails regardless of what the project says.
8.5.1 is the newest tsup published, so there is no upgrade that drops the
injection. `ignoreDeprecations: "6.0"` is the suppression TypeScript's own TS5101
message prescribes, it is scoped to that one deprecation, and type checking is
otherwise unchanged. Five packages across nself-org already carry it for the same
reason (plugins-pro's @nself/sentry, and four in the packages repo).
TIME-BOXED: this stops working at TypeScript 7.0, where baseUrl is removed rather
than deprecated. The real fix is upstream in tsup or replacing its dts step.
Re-check when a tsup newer than 8.5.1 ships.
2. packages/state — TS5011.
error TS5011: The common source directory of 'tsconfig.json' is './src'.
The 'rootDir' setting must be explicitly set
TypeScript 6 requires rootDir to be explicit. packages/core and packages/config
already declare `"rootDir": "src"`; packages/state was the only one of the four
missing it. Added, matching its siblings.
Verified by running the exact CI command in a checkout staged the way CI stages
it — WITHOUT the ../packages/@nself/* sibling, which no nchat workflow clones:
pnpm --filter "@nself-chat/ui" --filter "@nself-chat/core" \
--filter "@nself-chat/state" --filter "@nself-chat/config" build
exit 0
Before this change the same command exits 1 on TS5101, and 2 on TS5011 once
TS5101 is cleared.
Both tsconfigs stay strict JSON with no comments, matching every other tsconfig
in this repo.
desktop/ depends on @nself-chat/ui and @nself-chat/state via workspace:*, so its Tauri build compiles whatever those packages emit. The three desktop workflows only triggered on 'desktop/**', which meant a change to the shared packages could break the desktop build without any PR ever running it. That is how this PR's own fix had no evidence: every check on it was SKIPPED. Add packages/** plus the lockfile and workspace manifest to the PR triggers.
With the trigger fixed in the previous commit, the desktop builds ran for the
first time and failed immediately:
[vite]: Rollup failed to resolve import "@nself/observability"
from desktop/src/main.tsx
pnpm-workspace.yaml globs '../packages/@nself/*' -- the nself-org/packages repo
as a SIBLING directory of this one. desktop/package.json depends on five of them
(auth-core, graphql-client, i18n, observability, ui) as workspace:*, and no nchat
workflow has ever checked that repo out. The layout only exists on a developer's
machine, so the desktop build could not have succeeded in CI at any point.
nclaw and ntask already solve this; the variant used here is nsentry's, which
checks out into a subdirectory and MOVEs it to the sibling path rather than
symlinking, because pnpm writes relative node_modules links that resolve against
the physical path and dangle through a symlink.
Also drop a stale 'nchat/' prefix from the Linux and macOS artifact and bundle
paths. Their build steps run with working-directory: desktop, so the bundles land
under desktop/src-tauri/target, not nchat/desktop/src-tauri/target -- Windows
already had this right. Both are downstream of a build that has never succeeded,
so neither had ever been exercised: the macOS bundle-size gate would have exited
1 with "no DMG found" and the Linux upload would have matched no files.
With the sibling repo checked out, the desktop build got one step further and
then failed on the lockfile:
ERR_PNPM_OUTDATED_LOCKFILE Cannot install with "frozen-lockfile" because
pnpm-lock.yaml is not up to date with packages/@nself/errors/package.json
specifiers in the lockfile ({}) don't match specs in package.json
pnpm-lock.yaml had 11 importers and none for '../packages/@nself/*', even though
pnpm-workspace.yaml globs them. It had only ever been generated with the sibling
absent. Regenerated against nself-org/packages@8a5cc53: 16 importers added.
Verified both scopes locally, because 24 other workflows install from this same
lockfile and none of them check the sibling out:
sibling present -> "Scope: all 27 workspace projects", frozen install exit 0
sibling absent -> "Scope: all 11 workspace projects", frozen install exit 0
The glob simply matches nothing when the directory is not there, so the extra
importers are out of scope and the other workflows are unaffected. The only
resolution removed is third-party-web@0.29.2, an optional lighthouse transitive;
everything else in the diff is additive.
Pin the sibling checkout to that same sha rather than tracking main. A floating
sibling and a frozen lockfile contradict each other: the next push to
nself-org/packages would red every desktop build. The cost is that the sha and
the lockfile have to be bumped together, by hand -- Dependabot does not track a
checkout ref.
With the lockfile fixed, the Windows install step passed and the next one failed: packages/ui build: 'tsup' is not recognized as an internal or external command packages/config build: 'tsc' is not recognized as an internal or external command WARN Local package.json exists, but node_modules missing, did you mean to install? The install is scoped on Windows to keep frontend's test-only native dep `canvas` out of the graph (it has no Windows prebuilt and fails node-gyp on windows-2022). But the scope it used, `...@nself-chat/desktop`, does not cover all four packages the very next step builds, so ui/core/state/config never got a node_modules and their build tools were absent. Name the four explicitly instead of widening the filter, so the canvas exclusion keeps working. The invariant is that the install scope must be a superset of the build scope. Linux and macOS install unscoped, so neither is affected.
Third failure in the same class, so stop enumerating and invert the filter.
The Windows install was scoped with positive filters, and each time the build
got further it hit another project that had been left out of the install graph:
packages/{ui,core,state,config} no node_modules -> "'tsc' is not recognized",
"'tsup' is not recognized"
../packages/@nself/i18n package resolved, its dependency i18next did
not -> [vite]: Rollup failed to resolve
import "i18next" from .../i18n/src/provider.tsx
@nself/i18n exports ./src/index.ts, so vite compiles its TypeScript source and
needs its real dependencies installed, not just a link to the package.
The exclusion was always the load-bearing part: shamefully-hoist=true hoists
every in-scope project's deps to the root node_modules, so the positive filter
never stopped pnpm resolving frontend's test-only native dep `canvas` -- dropping
@nself-chat/web from the graph is what skips it. `canvas` has no Windows prebuilt
and fails node-gyp on windows-2022.
So keep only the negative filter. Every workspace project except web is a
superset of anything this build can reach, and the canvas exclusion is unchanged.
With the JS half of the desktop build finally green, the Rust half compiled for
the first time and failed:
error[E0599]: no method named `app_handle` found for reference `&tauri::Window`
--> src/lib.rs:72:32
error[E0599]: no method named `get_webview_window` found for struct `AppHandle<R>`
--> src/lib.rs:86:51
help: trait `Manager` which provides `app_handle` is implemented but not in scope
Both are trait methods on tauri::Manager, not inherent methods, so the trait has
to be in scope. lib.rs imported Emitter, Listener and WindowEvent but not Manager.
Only lib.rs was affected, which is why there were exactly two errors: tray.rs and
menu.rs use the same methods and already import Manager.
This has been latent for as long as the desktop build has been broken. Nothing
ever compiled this crate in CI, so no gate could report it.
The Linux desktop build now compiles, links and bundles. It produced all three
artifacts and then failed on the very last action:
Finished 3 bundles at:
.../bundle/deb/nChat_0.0.0_amd64.deb
.../bundle/rpm/nChat-0.0.0-1.x86_64.rpm
.../bundle/appimage/nChat_0.0.0_amd64.AppImage
failed to decode secret key: incorrect updater private key password:
Missing comment in secret key
tauri.conf.json sets createUpdaterArtifacts: true, so Tauri signs an updater
artifact on every build. There is no TAURI_SIGNING_PRIVATE_KEY in this repo or in
the org, so that step cannot succeed on any branch, and never could.
Disable updater artifacts for anything that is not a tag build. A PR has no
business signing a release artifact. Tag builds are untouched: they still set the
signing env and still produce and sign updater artifacts, so the release path is
unchanged and no gate is weakened.
The override is written to a file rather than passed inline after --config. An
unquoted {"bundle":{...}} argument loses its double quotes to the shell and Tauri
receives invalid JSON.
Two things this does NOT fix, both owner-gated and worth surfacing:
- TAURI_SIGNING_PRIVATE_KEY / _PASSWORD are not set anywhere, so a tag build
will hit this same error until they are provisioned.
- tauri.conf.json has plugins.updater.pubkey: "", so the updater could not
verify a signature even if one were produced.
The Windows build got all the way to the resource compiler and died there:
resource.rc(26) : error RC2175 : resource file
desktop/src-tauri/icons/icon.ico is not in 3.00 format
thread 'main' panicked at tauri-winres-0.3.6/src/lib.rs:543:14:
called `Result::unwrap()` on an `Err` value:
Failed("RC.EXE failed to compile specified resource file")
RC.EXE was right. icon.ico was 70 bytes and not an ICO at all — a 1x1 pixel PNG
with an .ico extension. icon.icns was the same 70-byte 1x1 PNG, so the macOS
bundle carried a placeholder too. The three PNGs were the correct dimensions and
solid black: exactly one distinct colour across every pixel.
Nothing had ever caught this because the desktop build had never reached the
resource compiler on any platform.
Regenerated all five from the brand artwork already committed in this repo,
.github/wiki/brand/icons/icon-1k.png (1024x1024, real artwork). No new asset is
introduced and nothing is designed here — this is a derivation from the project's
own icon.
icon.ico 70 B -> 101 KB, 7 resolutions 16..256
icon.icns 70 B -> 1.4 MB, built with iconutil from a full iconset
32x32.png solid black -> 680 colours
128x128.png solid black -> 4088 colours
128x128@2x.png solid black -> 8565 colours
For reference, nclaw ships a 361 KB multi-resolution ICO and ntask a 33 KB one.
nchat was the outlier.
…hat exists
Two separate macOS failures, neither previously reachable because the build had
never got this far.
1. Code signing. The aarch64 leg built, then died bundling:
failed to bundle project failed codesign application:
failed to run command security import: failed to import keychain certificate
This repo has ZERO APPLE_* secrets (checked by name). `${{ secrets.X }}` for an
unset secret yields a variable that is DEFINED AND EMPTY, and tauri-bundler does
not read that as "no identity" — it runs `security import` on an empty
certificate and fails. The variable has to be genuinely ABSENT. Export each one
only when its secret actually holds a value. nclaw hit the identical trap and
documents it in its own desktop-macos.yml.
A PR now produces an unsigned .app/.dmg, which is the correct outcome. A tag
build with the secrets present signs exactly as before.
2. Runner. The x64 leg asked for macos-13 and was never scheduled — queued for
over 90 minutes on one run and cancelled on another, while the aarch64 leg on
macos-14 ran fine. macos-13 is retired. nclaw builds x86_64-apple-darwin on
macos-15-intel and succeeded there today, so use the same label.
The env vars are renamed to *_SECRET and re-exported under their real names inside
the step, since a workflow cannot conditionally omit an `env:` key.
acamarata
force-pushed
the
fix/ui-dts-ts5101
branch
from
September 15, 2026 00:10
2887628 to
f48aa32
Compare
Security Scan Results: PASSED
|
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.
Correction to this PR's original claim
I opened this saying the TypeScript 6 / tsup
baseUrlinjection was the root cause. That was wrong. The TS6 fix is real and still needed —tsup@8.5.1injectsbaseUrlinto the dts worker's tsconfig and TypeScript 6 rejects it with TS5101 — but it was one layer of nine, and not the important one.Fixing the trigger is what proved it wrong. This build had never completed in CI on any platform, so each defect was invisible until the one before it cleared.
The nine layers
packages/packages/**Rollup failed to resolve import "@nself/observability"nself-org/packagesrepoERR_PNPM_OUTDATED_LOCKFILE … specifiers in the lockfile ({})'tsc' is not recognized, then"i18next"unresolved--filter "!@nself-chat/web"error[E0599]: no method named get_webview_windowuse tauri::Managerfailed to decode secret keyRC2175: icon.ico is not in 3.00 formatfailed to run command security importAPPLE_*only when non-emptymacos-13is retired →macos-15-intelLayer 2 is the actual answer to "why has this been broken for weeks":
pnpm-workspace.yamlglobs'../packages/@nself/*'— thenself-org/packagesrepo as a sibling directory — and no nchat workflow has ever checked it out. That layout exists only on a developer's machine.Layer 7 deserves naming:
icon.icoandicon.icnswere 70-byte 1×1 PNGs, and the three PNGs were solid black — one distinct colour across every pixel. Regenerated from the brand artwork already committed at.github/wiki/brand/icons/icon-1k.png, so nothing is designed here.Proven
Linux went green on
80e806f5— the first successful nchat desktop build, producing.deb,.rpmand.AppImage.To stop paying one CI round per layer, I reproduced CI's steps locally in a sibling layout and got the whole JS half green in one pass:
The Rust half was deliberately not built locally: swap was at 87–89% and this machine has a recorded kernel panic from cargo plus swap exhaustion.
Lockfile blast radius
24 other nchat workflows install from this lockfile and none check out the sibling. Verified both scopes, before the rebase and again after regenerating on top of #182/#183:
Scope: all 27 workspace projects, exit 0Scope: all 11 workspace projects, exit 0The glob matches nothing when the directory is absent, so the extra importers are out of scope.
One deliberate trade-off
The sibling checkout is pinned to
8a5cc53, not trackingmain. A floating sibling and a frozen lockfile contradict each other — the next push tonself-org/packageswould red every desktop build. Cost: the sha and the lockfile must be bumped together by hand.Owner-gated, not fixed here
TAURI_SIGNING_PRIVATE_KEY/_PASSWORDdo not exist, so a tag build still cannot sign an updater artifact.plugins.updater.pubkeyis"", so a client could not verify a signature anyway.APPLE_*secrets, so macOS releases will be unsigned.Logged as OWNER-ACTIONS §68. Nothing is broken for users today because no signed release has ever shipped.