Skip to content

fix(nix): derive npm workspace source closure from file: deps - #67529

Open
ak2k wants to merge 1 commit into
NousResearch:mainfrom
ak2k:fix/nix-workspace-closure
Open

ak2k wants to merge 1 commit into
NousResearch:mainfrom
ak2k:fix/nix-workspace-closure

Conversation

@ak2k

@ak2k ak2k commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Problem

.#tui has been red on main since #51639 landed /topup. ui-tui/src/app/slash/commands/topup.ts imports @hermes/shared/charge-settlement, but nix/tui.nix scoped its source to dirs = [ "ui-tui" ], so esbuild can't resolve apps/shared in the build sandbox:

ui-tui/src/app/slash/commands/topup.ts:1:62: ERROR: Could not resolve "@hermes/shared/charge-settlement"

Root cause

mkNpmPassthru's dirs is a hand-maintained allowlist that must mirror each package's file: deps, with nothing enforcing it. web.nix and desktop.nix had already hit this and hardcoded apps/shared; tui.nix never did — so the moment topup.ts pulled in @hermes/shared, the build broke.

Fix

Add workspaceClosure to nix/lib.nix: it derives a member's transitive file:-dependency dir closure from package.json (gathered across every dependency map, so a file: dep can't slip through via optionalDependencies etc.). This matches the single-source-of-truth topology the rest of the file already uses (workspaceMemberDirs, jsWorkspaceTopDirs). Point tui/web/desktop at it.

The derived closure is identical to the current explicit lists for the two packages that already worked:

package derived closure previous explicit dirs
web [web, apps/shared] [web, apps/shared]
desktop [apps/desktop, apps/shared] [apps/desktop, apps/shared]
tui [ui-tui, ui-tui/packages/hermes-ink, apps/shared] [ui-tui]was broken

So this is a no-op for web/desktop and the fix for tui. New file: deps can no longer silently break the source scope.

Verification

nix build .#tui .#web .#desktop all succeed; the built hermes-tui bundle now contains the charge-settlement code that previously failed to resolve.

Supersedes #67082 / #67090, which patch only this one instance.

.#tui has been red on main since NousResearch#51639 landed /topup:
ui-tui/src/app/slash/commands/topup.ts imports
@hermes/shared/charge-settlement, but nix/tui.nix scoped its source to
`dirs = [ "ui-tui" ]`, so esbuild could not resolve apps/shared in the
build sandbox:

  ui-tui/src/app/slash/commands/topup.ts:1:62: ERROR: Could not resolve
  "@hermes/shared/charge-settlement"

Root cause is structural: mkNpmPassthru's `dirs` is a hand-maintained
allowlist that must mirror each package's file: deps, with nothing
enforcing it. web.nix and desktop.nix had already hit this and hardcoded
apps/shared; tui.nix never did.

Add workspaceClosure to nix/lib.nix: it derives a member's transitive
file:-dep dir closure from package.json (gathered across every dependency
map), matching this file's existing single-source-of-truth topology, and
point tui/web/desktop at it. The derived closure is identical to the
current explicit lists for web ([web, apps/shared]) and desktop
([apps/desktop, apps/shared]); tui gains apps/shared, fixing the build.

Verified: nix build .#tui .#web .#desktop.

Supersedes NousResearch#67082 / NousResearch#67090, which patch only this one instance.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/nix Nix flake, NixOS module, container packaging sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related to open #67082: both repair the Nix TUI source-closure failure. This PR generalizes the dependency closure for TUI, Web, and Desktop, so it is a broader competing implementation rather than a duplicate.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks — the current-main premise is verified. nix/tui.nix:4 filters the Nix source to ui-tui, while ui-tui/src/app/slash/commands/topup.ts:1 imports @hermes/shared/charge-settlement; ui-tui/package.json:22 declares that package as file:../apps/shared.

Commit 0c77fc73f981f0d934891c01905a47fc6614c975 replaces the manual source lists for TUI, Web, and Desktop with a recursive closure over their file: dependency declarations. The current manifests' only file: consumers are those three packages, and the PR is cleanly based on current main (36f2a966c7f9f69987494b867c3dcf96b69a5766).

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/nix Nix flake, NixOS module, container packaging needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants