Repository navigation
Every Git call ignores an inherited repository location - #478
Conversation
Quest-Claim: 27ba81576c70c2b5
Move discoveryEnv() to src/cli/git.ts with one shared git() helper, fold the per-file helpers into it, and drop GIT_DIR and the other repository-local variables from every Git call on an explicit path and from the command `quest worktree run` and `quest agent` start, keeping -c config. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — initial review of head 915b521 against main (58696bc), all 20 files.
- Shared Git helper — new
src/cli/git.tswithdiscoveryEnv(),git(), andGitError; the variable filter is verbatim Git's own other-repository exception list (sanitize_repo_envkeeps exactlyGIT_CONFIG_PARAMETERSandGIT_CONFIG_COUNT), so-C-path discovery wins over a hook's inherited location while-cconfig survives, matching Git itself. - Helper fold-in —
gitOutput(claims.ts) and the localgithelpers inworktree.ts,worktree-hook.ts,move.ts, andgithub.tscollapse into the shared helper, along with the one-off calls instart.tspush,agent.tsspawnDirectory,move.tsmentions, andworktree.tsignored. I traced every caller across the old-vs-new semantics (trim vs trailing-newline strip, maxBuffer, error shape,configexit 1, stdin): the only observable deltas are the intended ones, and the oldgitOutputmessage format is byte-identical. - Environment for shape-differing helpers —
tree.ts,landed.ts,worktree.tsgitAsync,land-apply.tsuserGit, andland-fix.tskeep their own spawns and gainenv: discoveryEnv(); the left-as-is set (resolve.ts,land.ts,gate,src/runner/) checks out against the sweep of every git spawn insrc/. - Child-command environments —
quest worktree runandquest agent's agent now drop location variables, keeping-cconfig, as decided on #455; README and--helpagree with the behavior. - Tests and quest cleanup — the two new tests exercise the hook-inherited environment end to end, and the finished quest is deleted with every link to it (
quest check --base origin/mainpasses).
Verified locally: typecheck, prettier, the suites covering every refactored file, and the new snapshot test. test/worktree.run.test.ts needs a systemd user manager this runner lacks, and test/worktree.test.ts's concurrent-creator test fails identically on origin/main here — both environmental, not from this change.
ℹ️ runner-review.md cites the deleted gitOutput
quest/m0/kix/runner-review.md still cites gitOutput (src/cli/claims.ts) as execFileSync when describing why review()'s network Git calls stall the runner. This PR removes that symbol, so the citation no longer resolves; the doc's conclusion still holds, since the replacement in src/cli/git.ts is also a synchronous spawnSync. No change is required in this PR — noting it so the record can be touched up whenever that quest doc is next edited.
Technical details
# Stale doc citation after `gitOutput` removal
## Affected sites
- `quest/m0/kix/runner-review.md:26-27` — cites `review()` running `git ls-remote --symref` (`src/cli/review.ts:807`) and `git fetch` (`:888`) "through `gitOutput` (`src/cli/claims.ts`), which is `execFileSync`". PR #478 deletes `gitOutput`; `review.ts` now calls the shared `git()` in `src/cli/git.ts`, which is `spawnSync`.
## Required outcome
- The doc's evidence record should name a symbol that exists (or be reworded to describe the mechanism without naming one). Its underlying claim — network Git calls block the runner process — remains true under `spawnSync`.
## Suggested approach (optional)
- A one-line touch-up in a later docs pass; not required for this PR.glm-5.3-flash ｜ 𝕏
|
quest land watch: the merge queue removed this pull request at 915b521 because it conflicts with The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs. (written by quest land watch) |
|
quest land watch: armed auto-merge on this pull request at 5c71e6a, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix ( (written by quest land watch) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — incremental delta against the prior review of 915b521: one merge commit, 5c71e6a, that resolved conflicts against the moved base in worktree-hook.ts and worktree.ts.
- Extracted
superproject()inworktree-hook.ts— the superproject walks insubagentCheckout()andclaudeHook()were identical; the helper now carries them once, canonicalizing Git's forward-slash output on Windows exactly as the inlined code did. TheWORKFLOW_CHECKOUTcomment moved with it. - Rewrote
submodules()inworktree.tsfrom agit submodule foreach --quiet --recursiveshell call to recursion over the index's gitlinks, avoiding the seconds it costs the shell script on Windows. I traced the semantics: initialized modules only, nested ones under each, canonical paths, no traversal into uninitialized parents — equivalent toforeach --recursive. - Extracted
gitlinks(), deduplicating a conflicted gitlink's three index stages (ls-files --stageemits one entry per stage). Theclean()uninitialized-submodule check that previously inlined the same loop now uses it. - Added the base's per-submodule cleanliness checks to
clean(): each initialized submodule is checked for modified or untracked files deepest-first, alongside the existing root check. - Gated
submodule update --init --recursiveinWorktrees.create()on the index holding any gitlinks, skipping the no-op call in repositories without submodules. - Converted every Git call in the two files to array arguments matching
src/cli/git.ts'sgit(root, args, input?)— mechanical, no behavior change.
Verified locally: bun run check clean; test/worktree.test.ts (45 tests, 1 skipped) and test/snapshot.test.ts (12 tests) pass, including the nested-submodule initialization/preservation suites and the Claude hook suites that exercise the rewritten recursion and extracted helper.
glm-5.3-flash ｜ 𝕏
|
quest land watch: the merge queue removed this pull request at 5c71e6a because it conflicts with The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs. (written by quest land watch) |
|
codex / Codex land fixer note:
(written by codex / Codex) |
|
quest land watch: armed auto-merge on this pull request at 42a2919, so it joins the back of the merge queue once its required checks pass, with the land fixer's fix ( (written by quest land watch) |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — incremental delta against the prior review of 5c71e6a: one merge commit, 42a2919, that resolved conflicts in src/cli/review.ts against the moved base.
- Async fetch redesign folded in — main's
fetchBase()/remoteGit()(promisifiedexecFile, network calls off the event loop) replaced the branch's inline syncls-remote/fetchpair and the standalonedefaultBranch(). The dedup is real: remote lookup, fetch, and tracking-ref construction now live in one place, and the duplicated fetch inreview()is gone. - The environment fix survived the merge —
remoteGit's env changed from{ ...process.env, GIT_TERMINAL_PROMPT: "0" }to{ ...discoveryEnv(), GIT_TERMINAL_PROMPT: "0" }, so the network Git calls now drop a hook's inherited repository-local variables exactly like the syncgit()helper does; otherwise the branch's whole thesis would have been silently broken for the calls that reach a remote. - Every remaining Git call in
review.tsroutes throughsrc/cli/git.ts'sgit(); thegitOutputimport and call sites are gone.
Verified locally: tsc (repo + src/core), prettier on the delta, and the full test/review.test.ts suite (51 tests) pass.
|
quest land watch: the merge queue removed this pull request at 42a2919 because its The land fixer works on it in the background, and the watcher re-enqueues it at the back of the queue once it is fixed or judged flaky, or hands it back here with what it needs.
…ads…
�[90m |�[39m �[31m^�[39m
�[90m1547|�[39m await enrolled();
�[90m1548|�[39m await github({ activity: history() }).service.projectState(repo, "en…
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯�[22m�[39m
�[41m�[1m FAIL �[22m�[49m test/projects.test.ts�[2m > �[22menrolled project observations�[2m > �[22mwithholds candidates after push {"ref":"refs/heads/trunk"} until a refresh rereads occupancy
�[31m�[1mAssertionError�[22m: expected [] to be null�[39m
�[32m- Expected:�[39m
null
�[31m+ Received:�[39m
[]
�[36m �[2m❯�[22m test/projects.test.ts:�[2m1551:37�[22m�[39m
�[90m1549|�[39m expect((await delivery(name, { ...event, ...payload })).status).toBe…
�[90m1550|�[39m const cached = await github().service.projectState(repo, "enrollment…
�[90m1551|�[39m expect(cached!.project.available).toBeNull();
�[90m |�[39m �[31m^�[39m
�[90m1552|�[39m expect(readBoard(cached!.project).quests.get("quest/first.md")!.stat…
�[90m1553|�[39m const second = github({
�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯�[22m�[39m
�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[2m | �[22m�[1m�[32m20 passed�[39m�[22m�[90m (21)�[39m
�[2m Tests �[22m �[1m�[31m2 failed�[39m�[22m�[2m | �[22m�[1m�[32m529 passed�[39m�[22m�[90m (531)�[39m
�[2m Start at �[22m 09:46:07
�[2m Duration �[22m 115.49s�[2m (tests 66%, transform 31%, setup 3%)�[22m
##[error]Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".
❯ test/projects.test.ts:1546:13
##[error]AssertionError: expected [] to be null
- Expected:
null
+ Received:
[]
❯ test/projects.test.ts:1551:37
error: script "test" exited with code 1
error: recipe `_worker-test` failed on line 86 with exit code 1
error: recipe `worker-test` failed on line 83 with exit code 1
error: recipe `_test` failed on line 41 with exit code 1
error: recipe `test` failed on line 30 with exit code 1
##[error]Process completed with exit code 1.
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/159acc17-772a-432f-9926-f670f985a337' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
Post job cleanup.
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git version
git version 2.55.0
Temporarily overriding HOME='/var/cache/github-runner-work/_temp/1473e241-bff4-4281-8245-e398cdf0081e' before making global git config changes
Adding repository directory to the temporary git global config as a safe directory
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --global --add safe.directory /var/cache/github-runner-work/quest/quest
Removing SSH command configuration
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :"
Removing HTTP extra header
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive sh -c "git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :"
Removing includeIf entries pointing to credentials config files
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git config --local --name-only --get-regexp ^includeIf\.gitdir:
[command]/nix/store/lg46w7hrjx7kylsh6645c9l975i8h43b-git-2.55.0/bin/git submodule foreach --recursive git config --local --show-origin --name-only --get-regexp remote.origin.url
A job completed hook has been configured by the self-hosted runner administrator
##[group]Run '/nix/store/gx2v6q84b0w43bafm0whkdx93qyxbpps-ci-cache-gc.sh/bin/ci-cache-gc.sh'
shell: /nix/store/1mv3qz005gkbalxfghk4y3s8ayv4d2dl-bash-interactive-5.3p15/bin/bash --noprofile --norc -e -o pipefail {0}
##[endgroup]
evicted 0 objects and 0 action results (5.0 MiB logical); 49.8 GiB logical remain
dropped 1 stale checkout records
dropped 6 session event streams
Cleaning up orphan processes
Current runner version: '2.338.2'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20261021.618
Commit: 21505ad2541d5be6135ed84fc33697146cf75b8e
Build Date: 2026-10-21T00:45:21Z
Worker ID: {43db67b9-ce44-4943-9359-3123b6f0c723}
Region: northcentralus
Cloud: Azure
##[endgroup]
##[group]Operating System
Ubuntu
24.04.5
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20261025.353.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20261025.353/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20261025.353
##[endgroup]
##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
PullRequests: read
##[endgroup]
Secret source: Actions
Cache mode: write
Prepare workflow directory
Prepare all required actions
Complete job name: Queue
##[group]Run [[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }
�[36;1m[[ "$RESULT" == success ]] || { echo "::error::the queue build ended $RESULT" >&2; exit 1; }�[0m
shell: /usr/bin/bash -e {0}
env:
RESULT: failure
##[endgroup]
##[error]the queue build ended failure
##[error]Process completed with exit code 1.
Cleaning up orphan processes
(written by quest land watch) |
|
codex / Codex land fixer note:
(written by codex / Codex) |
|
quest land watch: re-enqueued this pull request at 42a2919, at the back of the merge queue, unchanged, since the land fixer judged the failure flaky: (written by quest land watch) |
Finishes quest/m1/git-env.md.
Problem
When
questruns from a Git hook orgit submodule foreach, it inheritsGIT_DIR,GIT_WORK_TREE,GIT_INDEX_FILEand the rest ofgit rev-parse --local-env-vars. Those override discovery fromgit -C <path>, so a Git call on another path reads the hook's repository instead. moq-dev/quest#445 fixed this only for the worktree hook. Everything else still passed the environment through. For example,quest checkfrom a superproject's pre-commit hook read a submodule's pins from the superproject and reported links that resolve as broken.Change
src/cli/git.tsholdsdiscoveryEnv(env?), moved fromworktree.ts, and onegit(root, args, input?)helper. The helper drops the location variables, keeps-cconfig, never prompts, strips trailing newlines from the output, treats an unsetconfigkey as empty, and throws aGitErrorthat carriesstatus,stdoutandstderr.gitOutput(claims.ts, also used byreview.ts,setup.ts,start.ts), andgitinworktree.ts,worktree-hook.ts,move.tsandgithub.ts. It also replaces the one-off calls instart.tspush,agent.tsspawnDirectory,move.tsmentionsandworktree.tsignored.env: discoveryEnv():tree.ts(Buffer output,--git-dir),landed.ts(status-based),worktree.tsgitAsync,land-apply.tsuserGit, andland-fix.ts's branch lookup.quest worktree run(Worktrees.run) drops the variables from the user's command, keeping-cconfig, as decided on moq-dev/quest#455.quest agent's spawned agent gets the same treatment, since it also works in its own checkout. README and--helpsay so.Public API and wire impact: none.
quest worktree runbehavior changes as described above.Choices made without asking
$(...), rather thantrim(). OldgitOutputcallers lose nothing they relied on, and leading whitespace inshowoutput and-zpaths is now kept.git <command> failed: <stderr>. Before,move.tsprinted bare stderr andworktree.tsprinted Node'sCommand failed: ....src/runner/(not started by Git, per the plan);land.tstryGit/bytes(they already build the environment from scratch);resolve.ts, the merge driver, which runs in the repository being merged, so its inherited location is the right one;quest gate, which runs its command in place.land-apply.tsuserGit,land-fix.tsandquest agent's agent environment. The quest's list did not name them, but the Goal covers every Git call on an explicit path.quest checknow reads submodule pins from the checkout's own index, not from agit commit -atemporary index. That matches the work treequest checkreads its quests from.Validation
just checkpasses.just testpasses (1044 passed and 5 skipped in the main suite, 531 in the Worker suite).test/snapshot.test.ts:quest checkwith links into a deinitialized submodule nested in a checked-out one, under an inheritedGIT_DIR,GIT_WORK_TREEandGIT_INDEX_FILE.test/worktree.run.test.ts:quest worktree runwith a main-checkout hook's variables andGIT_CONFIG_PARAMETERS. The command sees the reserved checkout's toplevel and still gets the-cvalue.Review
mechanical fix by codex / Codex:
Merged base and re·solved review.ts conflict, preserving async fetch and Git environment isolation; bun run build, bun run check, and bun run test passed (1056 tests passed, 5 skipped).(head 42a2919)mechanical fix by codex / Codex:
Merged base and re·solved worktree conflicts preserving both sides' intent; bun run build, bun run check, and bun run test passed (1044 tests, 5 skipped).(head 5c71e6a)Round 1: Codex
gpt-6.1-sol, effort xhigh, viaquest review --base origin/mainon head915b521315c4e760dcc4b06d746f91669262243e(base58696bc0). No findings.Suggested follow-ups
None.
(Written by Claude Opus 5.5)