Repository navigation
Conversation
A dispatcher worktree holds tracked files only. In an npm-workspaces repo, Node resolution from <repo>/.worktrees/<id> walks up into the main checkout's node_modules, whose workspace links point at the MAIN checkout's packages, so the worker builds and tests code that is not on its branch (or fails to resolve anything when the worktree lives outside the repo). _ensure_git_worktree and the same-branch reuse path in _resolve_worktree_workspace now run `npm ci` once when the worktree root has package.json with "workspaces" plus package-lock.json and no node_modules yet. It uses Hermes' managed npm, bounds network concurrency (--maxsockets 3) and runtime (300s), and is best effort: a missing npm or a failed install is logged, a partial node_modules is removed so the next dispatch retries, and the task still dispatches.
This branch has not been deployed
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.
What does this PR do?
A dispatcher worktree (
workspace_kind=worktree) holds tracked files only. In an npm-workspaces repo that means Node resolution from<repo>/.worktrees/<id>walks up into the main checkout'snode_modules, whose workspace links (node_modules/@scope/pkg -> ../../packages/pkg) point at the main checkout's packages. The worker then builds, typechecks and tests code that is not on its branch, and nothing reports it. When the worktree is pinned outside the repo (worktree:<path>), resolution fails outright.This change makes
_ensure_git_worktree, and the same-branch reuse path in_resolve_worktree_workspace, runnpm cionce when the worktree root has apackage.jsonwithworkspaces, apackage-lock.json, and nonode_modulesyet.find_node_executable("npm")+with_hermes_node_path()) and never falls back to PATH, perhermes_platform/AGENTS.mdandtests/test_managed_runtime_resolution.py.--maxsockets 3), runtime is capped at 300 s, and it runs with--no-audit --no-fund.node_modulesso the next dispatch retries, and still dispatches the task. Today's behaviour is the worst case.node_modulesis usually harmless there, and running an install for every JS repo would be a much larger behaviour change.Alternatives considered
kanban_worktree_createdhook): if that lands, an operator could do this with a config hook and no core change. It targets a different gap (gitignored secrets) and has no maintainer review yet. If maintainers prefer the hook route for dependency provisioning, I'm happy to close this in its favour..worktreeinclude(CLI-wworktrees): it symlinksnode_modulesback to the main checkout, which reproduces exactly the wrong-package resolution described above for workspace packages. It also isn't wired into kanban worktrees.Known tradeoff
The install runs inline in the dispatch path, just like
git worktree add(60 s timeout) does today, so the first dispatch of a worktree can take up to 300 s longer. Subsequent dispatches skip it becausenode_modulesnow exists. I can move it off the tick if you'd prefer that shape.Related Issue
None found. I searched open and closed PRs and issues for npm ci / node_modules / worktree provisioning; #104226 is the closest.
Type of Change
Changes Made
hermes_cli/kanban_db_workspace.py: new_provision_npm_workspace_dependencies(worktree). It is called aftergit worktree add, on the existing-worktree early return in_ensure_git_worktree, and on the same-branch reuse path in_resolve_worktree_workspace.tests/hermes_cli/test_kanban_worktree_npm_workspace.py(new) builds a real two-package npm-workspaces repo, offline, with an installed main checkout, and covers three cases:@fixture/ainside the worktree;node_modules, and logs the npm error.website/docs/user-guide/features/kanban.md: one sentence on the new worktree behaviour.How to Test
scripts/run_tests.sh tests/hermes_cli/test_kanban_worktree_npm_workspace.pygives 3 passed.kanban_db_workspace.py(plus only the two imported seam names so the monkeypatch targets exist), the two resolution tests fail.require.resolve('@fixture/a')from the worktree returns the main checkout'spackages/a. The failure-path test passes on base by construction, because base never runs npm. It pins the best-effort contract.scripts/run_tests.shon the new file,test_kanban_worktree_{isolation,teardown,unicode_path}.py,tests/test_managed_runtime_resolution.py,test_kanban_core_functionality.py,test_kanban_db.py,test_kanban_notify.py,tests/security/test_gitspawn_config_injection.pyandtests/tools/test_kanban_tools.pygives 10 files, 217 passed, 0 failed, 3 skipped (the skips are pre-existing and outside the new file).python scripts/check --base upstream/maingives 11 checks ok.Checklist
Code
pytest tests/ -qand all tests pass (only the touched/related files were run viascripts/run_tests.sh; see above)python scripts/checkpassesDocumentation & Housekeeping
website/docs/user-guide/features/kanban.mdcli-config.yaml.exampleif I added/changed config keys: N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows: N/Afind_node_executable(handles the.cmdshim) withwindows_hide_flags(),package.jsonis read asutf-8-sig, and the fake-npm failure test isplatforms("posix")