From f7eeda8aa304ce210e0aac2fd56da98fa30bd3eb Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 27 Oct 2026 11:38:50 +0000 Subject: [PATCH] fix: pre-landing review fixes (pass 4) The pack's staged-blob list comes from the copied index, holds only blobs staged against HEAD (not every tracked blob), skips gitlinks whose commit the superproject lacks, and dies with its own message when pack-objects fails. The live-checkout test covers a gitlink entry and a valueless remote key replayed as true. CHANGELOG names the staged-blob carry. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 +- scripts/ubicloud/ubi-runner.sh | 16 ++++++++++------ test/ubicloud-runner.test.ts | 6 ++++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2baf067e9..271fde730 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ ### Fixed - **`lib/redact-patterns.ts` `isCiRunIdentifier`**: an 11-digit GitHub Actions run id right after a run word on the same line (`screen 40477638211 GREEN`, `gate=…`, `rerun passed …`, `run #…`) is not `pii.phone.e164`, in every scanner (`gstack-approvals add`, `gstack-decision-trail`, the pregate secrets check, `gstack-post`); the run-word regex moved there from `lib/gstack-post.ts`, which imports it for its chained-id allowlist. A 10-digit national number after a run word, a bare 11-digit number, an E.164 `+` number and `job`, `id`, `workflow` or `attempt` before one still report; a national number written without its `+` right after a run word reads as a run id, the accepted residual of the ruling that a run id is not a phone number (`gstack-post` has read it that way since GSTA-121). -- **`scripts/ubicloud/ubi-runner.sh pack`**: the live `.git` is never tarred. The refs are fetched into a fresh repository through git's transport (`--update-shallow`, so a shallow CI or Capy checkout packs), the `remote.*` and `branch.*` config is replayed so `origin/` and each branch's upstream resolve on the VM, a detached HEAD gets a `detached-` branch, the index is copied as before, and a pack that fails removes its temporary repository; a review subagent's `git diff` or a trial merge's `worktree add` during the first minute of a remote gate no longer makes tar exit 1 (`PREGATE_REMOTE_INFRA`). The linked-worktree path uses the same snapshot. +- **`scripts/ubicloud/ubi-runner.sh pack`**: the live `.git` is never tarred. The refs are fetched into a fresh repository through git's transport (`--update-shallow`, so a shallow CI or Capy checkout packs), the `remote.*` and `branch.*` config is replayed so `origin/` and each branch's upstream resolve on the VM, a detached HEAD gets a `detached-` branch, the index is copied with the blobs staged against HEAD (a VM `gstack-wtree` can read a staged file), and a pack that fails removes its temporary repository; a review subagent's `git diff` or a trial merge's `worktree add` during the first minute of a remote gate no longer makes tar exit 1 (`PREGATE_REMOTE_INFRA`). The linked-worktree path uses the same snapshot. - **`lib/pregate/import-graph.ts`**: a relative string literal (`'../ship/sections/pr-body.md.tmpl'` through `new URL(…, import.meta.url)` or `readFileSync`) is a dependency edge from the file that names it, like a relative import; `test/ship-publication-gates.test.ts` is now selected when the pr-body template changes (the remote free gate missed it in GSTA-121's ship). A literal that climbs out of the repository names nothing. - **`scripts/lib/windows-curation.ts`**: `test/pregate-remote-stamp-only.test.ts` is `KNOWN_WINDOWS_INCOMPATIBLE` (it expects the Linux-host lanes verdict and skips itself on win32), as #3485's follow-up asked. diff --git a/scripts/ubicloud/ubi-runner.sh b/scripts/ubicloud/ubi-runner.sh index 1a65a629e..4f0fba5f5 100755 --- a/scripts/ubicloud/ubi-runner.sh +++ b/scripts/ubicloud/ubi-runner.sh @@ -626,15 +626,19 @@ cmd_pack() { git -C "$tmp/.git" config --add "${entry%%$'\n'*}" "${entry#*$'\n'}" done < <(git -C "$gitdir" config --local --null --get-regexp '^(remote|branch)\.' 2>/dev/null || true) # The checkout's index rides along so the VM's status matches, except a - # split index, whose shared half lives only in the original git dir. Its - # staged blobs are not behind any ref, so they travel as a pack of their - # own; without them the VM's write-tree (gstack-wtree) cannot read a + # split index, whose shared half lives only in the original git dir. The + # blobs staged against HEAD are behind no ref, so they travel as a pack of + # their own; without them the VM's write-tree (gstack-wtree) cannot read a # staged file. if [ "$(git -C "$src" config --bool core.splitIndex 2>/dev/null)" != true ]; then cp "$(cd "$src" && cd "$(dirname "$(git rev-parse --git-path index)")" && pwd)/index" "$tmp/.git/index" - git -C "$src" ls-files -s | awk '{print $2}' | sort -u \ - | git -C "$src" pack-objects -q --stdout 2>/dev/null \ - | git -C "$tmp/.git" unpack-objects -q + # Read the copy, not the live index, so the pack matches it; a gitlink's + # sha (mode 160000) is a submodule commit this object store lacks. + GIT_INDEX_FILE="$tmp/.git/index" git -C "$src" diff --cached --raw --no-abbrev HEAD \ + | awk '$2 != "160000" && $4 !~ /^0+$/ {print $4}' \ + | git -C "$src" pack-objects -q --stdout \ + | git -C "$tmp/.git" unpack-objects -q \ + || die "pack: could not snapshot the staged blobs of $src" fi ( cd "$src" diff --git a/test/ubicloud-runner.test.ts b/test/ubicloud-runner.test.ts index a4ee26692..6122d0bc9 100644 --- a/test/ubicloud-runner.test.ts +++ b/test/ubicloud-runner.test.ts @@ -340,9 +340,11 @@ test('pack ships a linked worktree with a self-contained .git on the checked-out const broken = Bun.spawnSync(['bash', '-c', `rm -f .git/index && TMPDIR=../tmpd ${pack} > ../pack-broken.tgz; echo rc=$?; ls ../tmpd`], { cwd: src, timeout: 60_000 }); expect(broken.stderr.toString()).toContain('.git/index'); expect(broken.stdout.toString().trim()).toBe('rc=1'); - sh('git reset -q', src); + // A gitlink in the index (a submodule commit this object store lacks) is not packed, and a valueless + // remote key (`[remote "origin"] mirror`) replays as true. + sh('git reset -q && git update-index --add --cacheinfo 160000,1111111111111111111111111111111111111111,sub && printf \'[remote "origin"]\\n\\tmirror\\n\' >> .git/config', src); sh('mkdir vm2 && ' + pack.replace('pack .', 'pack src') + ' | tar -xzf - -C vm2', d); - expect(sh('git rev-parse --abbrev-ref @{u}', join(d, 'vm2')).trim()).toBe('origin/main'); + expect(sh('git rev-parse --abbrev-ref @{u} && git config --bool --get remote.origin.mirror && git ls-files -s sub | cut -c1-6', join(d, 'vm2')).split('\n').filter(Boolean)).toEqual(['origin/main', 'true', '160000']); } finally { rmSync(d, { recursive: true, force: true }); } }, 90_000);