mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-10-20 03:05:24 +00:00
fix(app): stage the CLI closure without anchoring on the checkout's absolute path (#1473)
Some checks are pending
Tests / test (26) (push) Waiting to run
Tests / test-platforms (macos-latest) (push) Waiting to run
Tests / test-platforms (windows-latest) (push) Waiting to run
Tests / desktop (push) Waiting to run
CI / semgrep (push) Waiting to run
Tests / test (22) (push) Waiting to run
Tests / test (22.13.0) (push) Waiting to run
Tests / test (24) (push) Waiting to run
Some checks are pending
Tests / test (26) (push) Waiting to run
Tests / test-platforms (macos-latest) (push) Waiting to run
Tests / test-platforms (windows-latest) (push) Waiting to run
Tests / desktop (push) Waiting to run
CI / semgrep (push) Waiting to run
Tests / test (22) (push) Waiting to run
Tests / test (22.13.0) (push) Waiting to run
Tests / test (24) (push) Waiting to run
npm redacts UUID-shaped path segments to `***` in `npm ls --parseable`, so matching each line against the checkout's absolute <root>/node_modules/ prefix found nothing on any path containing a UUID and packaging aborted with the misleading "is the root npm installed?".
The top-level package name is now read by segment position: after the N-th /node_modules segment, N counted off the real root path, which the redaction renames around but cannot move.
Closure verified identical: 160 packages, same names and versions, same 55M, staged from a UUID worktree and from a UUID-free checkout of main.
Reproduced before and after: main aborts from the UUID path, the branch stages 160 packages from that same path.
A non-zero `npm ls` exit still yields its stdout, so an extraneous package in node_modules does not abort staging.
A missing npm_execpath now fails with its own message instead of degrading into the empty-closure one.
Review patch on the branch: the two hand-rolled path scanners collapse to one split('/node_modules/'), and the npm attribution is corrected from 12 to 11 and later, reproduced on npm 11.19.0.
Fixes #1466.
This commit is contained in:
parent
f1f275e27e
commit
0f5a33450c
5 changed files with 132 additions and 22 deletions
|
|
@ -4,6 +4,7 @@
|
|||
|
||||
### Fixed (desktop)
|
||||
- **The Models table shows every model that ran, including the ones under a cent.** The CLI's `models` command defaults `minCost` to $0.01, and the desktop bridge passed neither `--min-cost` nor `--unpriced`, so the table silently dropped every row below a cent — which by construction excluded every unpriced row too (a `0 >= 0.01` filter), leaving #1443's dimming and add-alias affordances unreachable in the shipped app. `codeburn:getModels` now passes `--min-cost 0` (and the demo bridge mirrors it), so sub-cent and unpriced rows arrive and render with their existing dim treatment; on a real lifetime corpus that recovers 10 rows and 2,160 calls the default filter hid (43 → 53 rows, verified in both themes). A row priced between $0.00 and $0.01 renders as "$0.00" without dimming — it is genuinely priced, just below the display floor. Fixes #1465.
|
||||
- **Packaging no longer aborts on a checkout whose path contains a UUID.** `app/scripts/stage-cli.mjs` resolved the CLI's production dependency closure by matching each `npm ls --parseable` line against the checkout's absolute `<root>/node_modules/` prefix, and npm 11 and later redact UUID-shaped path segments to `***` in that output — so on CI runners and scratch worktrees under a UUID directory the match found nothing and packaging died with the misleading "is the root npm installed?". The top-level package name is now read by position (after the N-th `/node_modules` occurrence, N counted off the real root path, which the redaction cannot move) instead of by absolute-prefix match, so redacted, Windows-separated and nested-checkout shapes all resolve; a missing `npm_execpath` also fails with its own message instead of degrading into the empty-closure one. Fixes #1466.
|
||||
|
||||
### Added
|
||||
- **Hovering a dollar amount in the desktop shows the tokens behind it.** Hover or tab to any money figure that has token counts behind it — the Overview hero, the Yesterday, 7-day avg and Month to date figures, the daily chart bars and its Avg/day, Peak and Yesterday chips, the Sessions and Models rows and the two Compare tables — and a small popover lists input, output, cache read and cache write tokens plus the calls they came from. A figure with no token counts behind it shows nothing rather than a fabricated breakdown: projected month and the other estimates, percentages, savings, retry tax, plans and the by-project and by-PR rows are left exactly as they were. The amounts themselves are unchanged in size, weight and position, and the popover is drawn above the card so it never clips. The menubar payload's `periodTotals` now carries the same four counts, so the hero keeps its breakdown when its number comes from the machine-wide generation rather than the payload's own period.
|
||||
|
|
|
|||
1
app/scripts/stage-cli-packages.d.mts
Normal file
1
app/scripts/stage-cli-packages.d.mts
Normal file
|
|
@ -0,0 +1 @@
|
|||
export declare function topLevelPackagesFromNpmLs(listed: string, rootModulesPath: string): Set<string>
|
||||
37
app/scripts/stage-cli-packages.mjs
Normal file
37
app/scripts/stage-cli-packages.mjs
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
// Extract the top-level production package names from `npm ls --parseable`
|
||||
// output without anchoring on the checkout's absolute path.
|
||||
//
|
||||
// npm redacts UUID-shaped path segments to `***` in that output, so on a
|
||||
// checkout whose path contains a UUID (CI runners, scratch worktrees) any
|
||||
// absolute-prefix match finds nothing and staging reports an empty dependency
|
||||
// closure (#1466). `npm ls` runs with its working directory at the package
|
||||
// root, so every line it prints belongs to this project's tree however the
|
||||
// prefix was mangled. The root's own node_modules is identified by POSITION —
|
||||
// at however many `/node_modules` segments the real root path contains, the
|
||||
// top-level name sits right after — and the redaction renames segments but
|
||||
// cannot move one.
|
||||
|
||||
/** The set of top-level package names (`name` or `@scope/name`) named by the
|
||||
* parseable lines, deduped. Nested transitive paths map to their top-level
|
||||
* ancestor, mirroring the layout the stage copies into build/cli/node_modules.
|
||||
* `rootModulesPath` is the real (unredacted) `<root>/node_modules` path; only
|
||||
* the COUNT of its `/node_modules` segments is read, never its text, so a
|
||||
* redacted line resolves by position alone. */
|
||||
export function topLevelPackagesFromNpmLs(listed, rootModulesPath) {
|
||||
// `npm ls --parseable` prints native separators on Windows; normalize both
|
||||
// sides before segmenting so Store builds read the same tree.
|
||||
const depth = rootModulesPath.replaceAll('\\', '/').split('/node_modules').length - 1
|
||||
const names = new Set()
|
||||
for (const raw of listed.split('\n')) {
|
||||
const segments = raw.trim().replaceAll('\\', '/').split('/node_modules/')
|
||||
if (segments.length <= depth) continue
|
||||
const match = segments[depth].match(/^(@[^/]+\/[^/]+|[^/]+)/)
|
||||
if (!match) continue
|
||||
// A truncated tail can leave a bare scope (`@ns`) with no package after
|
||||
// it; that is not a package name, and join() below would copy the whole
|
||||
// scope directory.
|
||||
if (match[1].startsWith('@') && !match[1].includes('/')) continue
|
||||
names.add(match[1])
|
||||
}
|
||||
return names
|
||||
}
|
||||
76
app/scripts/stage-cli-packages.test.ts
Normal file
76
app/scripts/stage-cli-packages.test.ts
Normal file
|
|
@ -0,0 +1,76 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { topLevelPackagesFromNpmLs } from './stage-cli-packages.mjs'
|
||||
|
||||
// `npm ls --parseable` parsed without anchoring on the absolute checkout path.
|
||||
// npm 11 and later redact UUID-shaped path segments to `***` (observed with
|
||||
// npm 11.19.0: a checkout at /tmp/1a2b3c4d-1111-2222-3333-444455556666/probe
|
||||
// prints /private/tmp/***/probe as its root line), which is why #1466's
|
||||
// absolute-prefix match aborted packaging on any UUID-containing path. These
|
||||
// fixtures pin the real output shapes, redacted and not. The root's
|
||||
// node_modules is located by segment position — however many `/node_modules/`
|
||||
// segments the root path itself contains, the top-level name sits one past
|
||||
// that count.
|
||||
|
||||
const ROOT_MODULES = '/repo/codeburn/node_modules'
|
||||
|
||||
describe('topLevelPackagesFromNpmLs', () => {
|
||||
it('reads top-level names from paths whose UUID segments npm redacted', () => {
|
||||
const listed = [
|
||||
'/private/tmp/1a2b3c4d-1111-2222-3333-444455556666/codeburn',
|
||||
'/private/tmp/1a2b3c4d-1111-2222-3333-444455556666/codeburn/node_modules/@modelcontextprotocol/sdk',
|
||||
'/private/tmp/1a2b3c4d-1111-2222-3333-444455556666/codeburn/node_modules/chalk',
|
||||
'/private/tmp/1a2b3c4d-1111-2222-3333-444455556666/codeburn/node_modules/react/node_modules/loose-envify',
|
||||
].join('\n')
|
||||
expect(topLevelPackagesFromNpmLs(listed, ROOT_MODULES)).toEqual(new Set(['@modelcontextprotocol/sdk', 'chalk', 'react']))
|
||||
})
|
||||
|
||||
it('keeps the redacted shape working, which the old prefix match could not', () => {
|
||||
// This is the literal output npm 11.19.0 prints for the checkout above: the
|
||||
// UUID segment is `***` on every line. The old absolute-prefix match found
|
||||
// zero packages here and aborted the build.
|
||||
const listed = [
|
||||
'/private/tmp/***/probe-29473',
|
||||
'/private/tmp/***/probe-29473/node_modules/lodash',
|
||||
].join('\n')
|
||||
expect(topLevelPackagesFromNpmLs(listed, '/tmp/1a2b3c4d-1111-2222-3333-444455556666/probe-29473/node_modules')).toEqual(new Set(['lodash']))
|
||||
})
|
||||
|
||||
it('stays correct when the checkout itself lives inside a node_modules directory', () => {
|
||||
// The root path contains one node_modules segment of its own, so the
|
||||
// project's packages sit one segment deeper. A first-occurrence parser
|
||||
// would read the checkout's directory name (`codeburn`) off every line.
|
||||
const nestedRoot = '/other/repo/node_modules/codeburn/node_modules'
|
||||
const listed = [
|
||||
'/other/repo/node_modules/codeburn',
|
||||
'/other/repo/node_modules/codeburn/node_modules/chalk',
|
||||
'/other/repo/node_modules/codeburn/node_modules/react-dom/node_modules/scheduler',
|
||||
].join('\n')
|
||||
expect(topLevelPackagesFromNpmLs(listed, nestedRoot)).toEqual(new Set(['chalk', 'react-dom']))
|
||||
})
|
||||
|
||||
it('maps a nested transitive path to its top-level ancestor, not the leaf', () => {
|
||||
const listed = '/repo/codeburn/node_modules/react-dom/node_modules/scheduler\n/repo/codeburn/node_modules/react-dom'
|
||||
expect(topLevelPackagesFromNpmLs(listed, ROOT_MODULES)).toEqual(new Set(['react-dom']))
|
||||
})
|
||||
|
||||
it('normalizes Windows separators on both the lines and the root path', () => {
|
||||
const listed = [
|
||||
'C:\\repo\\codeburn',
|
||||
'C:\\repo\\codeburn\\node_modules\\undici',
|
||||
'C:\\repo\\codeburn\\node_modules\\@scope\\pkg\\node_modules\\dep',
|
||||
].join('\n')
|
||||
expect(topLevelPackagesFromNpmLs(listed, 'C:\\repo\\codeburn\\node_modules')).toEqual(new Set(['undici', '@scope/pkg']))
|
||||
})
|
||||
|
||||
it('skips the project-root line, truncated bare scopes, and empty output', () => {
|
||||
expect(topLevelPackagesFromNpmLs('/repo/codeburn', ROOT_MODULES)).toEqual(new Set())
|
||||
expect(topLevelPackagesFromNpmLs('', ROOT_MODULES)).toEqual(new Set())
|
||||
expect(topLevelPackagesFromNpmLs('\n\n', ROOT_MODULES)).toEqual(new Set())
|
||||
// A truncated tail that leaves a bare scope is not a package name: without
|
||||
// the guard, join(root, '@scope') is a real directory and the whole scope
|
||||
// would be copied as one "package".
|
||||
expect(topLevelPackagesFromNpmLs('/repo/codeburn/node_modules/@scope', ROOT_MODULES)).toEqual(new Set())
|
||||
expect(topLevelPackagesFromNpmLs('/repo/codeburn/node_modules/@scope/', ROOT_MODULES)).toEqual(new Set())
|
||||
})
|
||||
})
|
||||
|
|
@ -25,6 +25,8 @@ import { dirname, join } from 'node:path'
|
|||
import { fileURLToPath } from 'node:url'
|
||||
import electronPath from 'electron'
|
||||
|
||||
import { topLevelPackagesFromNpmLs } from './stage-cli-packages.mjs'
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url)) // app/scripts
|
||||
const appDir = join(here, '..') // app
|
||||
const root = join(appDir, '..') // repo root
|
||||
|
|
@ -69,12 +71,14 @@ writeFileSync(
|
|||
|
||||
// `npm ls` prints the tree to stdout even when it exits non-zero on peer/
|
||||
// extraneous warnings, so capture stdout regardless of exit code.
|
||||
// npm_execpath is checked BEFORE the try: swallowing that error here degraded
|
||||
// into the empty-closure message below and hid the real cause (#1466).
|
||||
// Execute npm's JavaScript entry point with the current Node binary. Windows
|
||||
// exposes npm as a .cmd shim, which execFile cannot launch without a shell.
|
||||
const npmCli = process.env.npm_execpath
|
||||
if (!npmCli) throw new Error('stage-cli: npm_execpath is unavailable — run staging through npm scripts (npm run stage-cli), not node directly')
|
||||
let listed = ''
|
||||
try {
|
||||
// Execute npm's JavaScript entry point with the current Node binary. Windows
|
||||
// exposes npm as a .cmd shim, which execFile cannot launch without a shell.
|
||||
const npmCli = process.env.npm_execpath
|
||||
if (!npmCli) throw new Error('npm_execpath is unavailable')
|
||||
listed = execFileSync(process.execPath, [npmCli, 'ls', '--omit=dev', '--all', '--parseable'], {
|
||||
cwd: root,
|
||||
encoding: 'utf8',
|
||||
|
|
@ -84,24 +88,15 @@ try {
|
|||
listed = err.stdout ? String(err.stdout) : ''
|
||||
}
|
||||
|
||||
// Every parseable line is an absolute path to a production package instance.
|
||||
// Map each back to its top-level node_modules entry (`name` or `@scope/name`),
|
||||
// then copy those dirs whole — a package's own nested node_modules comes with
|
||||
// it, which is exactly the closure it needs at runtime.
|
||||
// `npm ls --parseable` uses native separators on Windows. Normalize both sides
|
||||
// before extracting the package name so Store builds do not treat a populated
|
||||
// node_modules tree as empty merely because it uses `\\` instead of `/`.
|
||||
const prefix = rootModules.replaceAll('\\', '/') + '/'
|
||||
const comparisonPrefix = process.platform === 'win32' ? prefix.toLowerCase() : prefix
|
||||
const topLevel = new Set()
|
||||
for (const line of listed.split('\n')) {
|
||||
const normalizedLine = line.trim().replaceAll('\\', '/')
|
||||
const comparisonLine = process.platform === 'win32' ? normalizedLine.toLowerCase() : normalizedLine
|
||||
if (!comparisonLine.startsWith(comparisonPrefix)) continue
|
||||
const rest = normalizedLine.slice(prefix.length)
|
||||
const match = rest.match(/^(@[^/]+\/[^/]+|[^/]+)/)
|
||||
if (match) topLevel.add(match[1])
|
||||
}
|
||||
// Every parseable line is a path to a production package instance. Map each
|
||||
// to its top-level node_modules entry (`name` or `@scope/name`), then copy
|
||||
// those dirs whole — a package's own nested node_modules comes with it, which
|
||||
// is exactly the closure it needs at runtime. Names are extracted by segment
|
||||
// position rather than matched against the checkout's absolute path: npm 11
|
||||
// and later redact UUID-shaped path segments to `***` in this output, so a
|
||||
// checkout under a UUID directory (CI runners, scratch worktrees) broke the
|
||||
// absolute-prefix match and staged an empty closure (#1466).
|
||||
const topLevel = topLevelPackagesFromNpmLs(listed, rootModules)
|
||||
if (topLevel.size === 0) {
|
||||
throw new Error('stage-cli: production dependency closure is empty — is the root `npm install`ed?')
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue