Skip to content

fix: sync ui-tui lockfile for npm ci#18577

Closed
woyin wants to merge 1 commit into
NousResearch:mainfrom
woyin:fix/ui-tui-lockfile-npm-ci
Closed

fix: sync ui-tui lockfile for npm ci#18577
woyin wants to merge 1 commit into
NousResearch:mainfrom
woyin:fix/ui-tui-lockfile-npm-ci

Conversation

@woyin

@woyin woyin commented May 1, 2026

Copy link
Copy Markdown

Summary

  • Refresh ui-tui/package-lock.json so it is in sync with the current dependency graph.
  • Add the missing optional peer entries for @emnapi/core and @emnapi/runtime required through the Rolldown/Vite dependency chain.
  • This lets the updater's deterministic npm ci path succeed instead of falling back to npm install, which can dirty the working tree by rewriting the lockfile.

Verification

  • cd ui-tui && npm ci --silent --no-fund --no-audit --progress=false
  • cd ui-tui && npm run type-check
  • python -m pytest tests/hermes_cli/test_tui_npm_install.py -q -o 'addopts='

Note: I also ran tests/hermes_cli/test_cmd_update.py together with the TUI npm install tests. The TUI npm install tests passed, but one existing update test failed locally because this checkout includes a web/ directory while the test expectation only included repo root and ui-tui npm calls.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #18573 (same lockfile refresh pattern). Consider consolidating with that PR.

@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #18573 (same lockfile refresh pattern).

@hl-luka

hl-luka commented May 25, 2026

Copy link
Copy Markdown

I can reproduce the lockfile side of this on a native Linux checkout from current main.

Environment:

  • Ubuntu/Linux: Linux 7.0.0-15-generic
  • Node: v22.22.1
  • npm: 9.2.0
  • Install style: git checkout at /home/hl/hermes, launched through the checkout venv

Evidence from current main:

$ cd ui-tui && npm ci --dry-run --no-fund --no-audit --progress=false
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync.
npm ERR! Missing: @alcalzone/ansi-tokenize@0.1.3 from lock file
npm ERR! Missing: chalk@5.6.2 from lock file
npm ERR! Missing: signal-exit@4.1.0 from lock file
npm ERR! Missing: type-fest@4.41.0 from lock file
npm ERR! Missing: ansi-styles@6.2.3 from lock file
npm ERR! Missing: is-fullwidth-code-point@4.0.0 from lock file

The specific mismatch is visible here:

  • ui-tui/packages/hermes-ink/package.json declares @alcalzone/ansi-tokenize: ^0.1.0
  • committed ui-tui/package-lock.json currently pins @alcalzone/ansi-tokenize: 0.2.5

Since 0.2.5 does not satisfy ^0.1.0, the updater's deterministic npm ci path cannot succeed and falls back to npm install. That rewrites ui-tui/package-lock.json, leaving the checkout dirty. On subsequent hermes update runs this repeatedly triggers:

→ Local changes detected — stashing before update...

In my local update logs this repeats across several updates with stashes containing M ui-tui/package-lock.json.

So PR #18577 looks like the right direction for the lockfile part of the problem. It should stop the update path from falling back to mutating npm install in normal checkouts.

One related update-path concern: after dependency refresh, @hermes/ink runtime/build artifacts should also be known-good before hermes update reports success. In affected local checkouts the documented workaround is:

cd ui-tui
npm run build --prefix packages/hermes-ink
npm run build

If this PR is intended to solve only the lockfile issue, it may be worth tracking the explicit @hermes/ink/TUI build-after-update behavior separately (possibly related to #31227 / #25351). But the lockfile mismatch itself is reproducible and this PR addresses the first cause cleanly.

@teknium1

Copy link
Copy Markdown
Contributor

This looks implemented on current main by a broader workspace-lockfile consolidation.

Evidence from this automated hermes-sweeper review:

  • a51a7b9b92b63b5f97afa0fc31c9cd349f04b5ec (fix(node/nix): consolidate workspace lockfile + update all consumers) deleted ui-tui/package-lock.json and moved the repo to a single root workspace lockfile.
  • package.json:6 declares the npm workspaces, including ui-tui and ui-tui/packages/*.
  • package-lock.json:21454 now has the ui-tui/node_modules/@alcalzone/ansi-tokenize lock entry at 0.1.3, while package-lock.json:22057 records @hermes/ink depending on @alcalzone/ansi-tokenize: ^0.1.0.
  • hermes_cli/main.py:1290 adds _workspace_root() so TUI lockfile/node_modules resolution and npm install cwd use the workspace root instead of a stale per-directory lockfile.
  • tests/hermes_cli/test_tui_npm_install.py:361 now guards against stray package-lock.json files under workspace subdirectories such as ui-tui/.

The useful reproduction/comment thread here matched the old per-ui-tui lockfile state, but current main fixes that path by removing the per-package lockfile entirely and making the root lock authoritative.

@teknium1 teknium1 closed this Jun 10, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants