Skip to content

Fix(cli): A push after merging the default branch measures only the branch's own change - #1824

Merged
daniboomerang merged 1 commit into
mainfrom
task/issue-1823
Nov 2, 2026
Merged

daniboomerang merged 1 commit into
mainfrom
task/issue-1823

Conversation

@daniboomerang

@daniboomerang daniboomerang commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Closes #1823

For: Claude Opus (coding-agent CLI on a dev machine, dispatched locally, unattended)

Project: cli

Decisions

  • Shape: resolveRemoteBase now composes two private steps, startingRemoteBase (the unchanged chain) and mergedCommitAfter (the merged-commit rule), so the starting-base logic stays byte-for-byte as before and the new rule is one readable function.
  • Merge without a second parent (<sha>^2 fails or prints nothing): skipped, and the next older merge is tried; with none left the start is returned, per the brief's "a read fails or prints nothing" clause.
  • Ancestry read: git merge-base --is-ancestor answers by exit status only, so it is read through the file's existing ReadFailure opt-in — a non-zero exit counts as "not an ancestor".
  • Commit header shortened to fit the commit-header ceiling; the PR title stays the brief's exact title.

Test plan

bun scripts/test-suites/local-run.ts exact --context 1f14415fad80d84e92b5744d1d668de2b4db76a1ff8215a49b2d3b48f6861257 -- apps/cli/tests/checks/changeset-coverage-bin.test.ts@default apps/cli/tests/checks/changeset-coverage.test.ts@default apps/cli/tests/checks/check-exec-bits.test.ts@default apps/cli/tests/checks/own-workflow.test.ts@default apps/cli/tests/ci-shards.test.ts@default apps/cli/tests/commands/task-status.test.ts@default apps/cli/tests/import-boundary.test.ts@default apps/cli/tests/lib/log-callers.test.ts@default apps/cli/tests/run-paths-only.test.ts@default apps/cli/tests/surface-spec-exports.test.ts@default → every selected test passes under its own native runner
cd apps/cli && bun test --timeout=60000 tests/lib/remote-base.test.ts tests/lib/pre-push-failed-tests.test.ts tests/lib/pre-push-mode.test.ts tests/lib/test-selector.test.ts → 0 fail, including the merged-commit cases

Premise

Premise:

  • apps/cli/src/lib/remote-base.ts contains: const upstream = git(repoRoot, ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@{u}'])

Evidence

Head: a69a0f2
Summary: 3 files changed, 141 insertions(+), 4 deletions(-)

Group A — recomputable

git diff 9dda4a41a7baeda648b406a6498683b0a021c2a6...a69a0f2c6eee9dcdcb7d5295ac015ce42a749d8b --numstat

5	0	.changeset/push-after-merge-measures-branch.md
45	4	apps/cli/src/lib/remote-base.ts
91	0	apps/cli/tests/lib/remote-base.test.ts

Group B — attested

vinaya check --all --diff-only

Graded body: the live pull-request body (--push)

ready-gate: pass

Group C — Test Plan commands

C1: bun scripts/test-suites/local-run.ts exact --context 1f14415fad80d84e92b5744d1d668de2b4db76a1ff8215a49b2d3b48f6861257 -- apps/cli/tests/checks/changeset-coverage-bin.test.ts@default apps/cli/tests/checks/changeset-coverage.test.ts@default apps/cli/tests/checks/check-exec-bits.test.ts@default apps/cli/tests/checks/own-workflow.test.ts@default apps/cli/tests/ci-shards.test.ts@default apps/cli/tests/commands/task-status.test.ts@default apps/cli/tests/import-boundary.test.ts@default apps/cli/tests/lib/log-callers.test.ts@default apps/cli/tests/run-paths-only.test.ts@default apps/cli/tests/surface-spec-exports.test.ts@default

[... 15631 earlier characters truncated ...]
vel folders as a path segment [65.47ms]

tests/surface-spec-exports.test.ts:
(pass) apps/cli/src/lib/**'s real exports are internally sane (O2) > has at least one real export to check (sanity) [0.04ms]
(pass) apps/cli/src/lib/**'s real exports are internally sane (O2) > has at least one real export to check for @attalabs/aeg-core's barrel (sanity) [1691.10ms]
(pass) apps/cli/src/lib/**'s real exports are internally sane (O2) > every retiresVia target that already names a real lib export names a function [144.68ms]

 108 pass
 0 fail
 1506 expect() calls
Ran 108 tests across 10 files. [32.52s]

Reused from a green run recorded 2026-11-02T06:06:36.929Z (pre-push) — not re-run.

C2: cd apps/cli && bun test --timeout=60000 tests/lib/remote-base.test.ts tests/lib/pre-push-failed-tests.test.ts tests/lib/pre-push-mode.test.ts tests/lib/test-selector.test.ts

[... 45 earlier characters truncated ...]
lved failure stays an obligation even when the base widens; an unnamed one widens with none [0.13ms]
(pass) the native push: its base, its obligations and its proof > a refusal on another branch or outside this history is ignored [0.02ms]
(pass) the native push: its base, its obligations and its proof > a complete green native run of exactly the plan, on an unchanged head, tree and context, is proof [7.22ms]
(pass) the native push: its base, its obligations and its proof > anything short of that is no proof [0.28ms]

 211 pass
 0 fail
 749 expect() calls
Ran 211 tests across 4 files. [286.64s]

Reused from a green run recorded 2026-11-02T06:11:25.280Z (pr-report) — not re-run.

Scope

One function in apps/cli/src/lib/remote-base.ts (resolveRemoteBase), its file header and comment, new fixture-repository cases in apps/cli/tests/lib/remote-base.test.ts, and a patch changeset for @attalabs/vinaya. Consumers run unchanged: the pre-push Biome step and test selector, and scripts/test-suites/local-run.ts, all of which call resolveRemoteBase or the changed-file readers that default to it. With no merge after the starting base every caller resolves exactly as before. Non-goals: the merge-base/rerun/since-green base choice, the ready push's environment variables, CI test selection, and the loop driver's pre-turn merge are untouched.

Tier: 1

@vercel

vercel Bot commented Nov 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
vinaya Ignored Ignored Nov 2, 2026 6:11am UTC

@daniboomerang

Copy link
Copy Markdown
Member Author

VERDICT: APPROVE

Judged head: a69a0f2

Objectives version: 33ff68c313ae563f337e21468ea2b63410c803c4dc3e26b152c45fa3229084c8

Ruling ordinal: 0

Brief hash: 048ee7c3e318aea62547a6b384c8f405123b572376a766e0ead056ed13025f28

Policy digest: bf51ddb9c7bd4776744d56ac91f4005ee8b2c1e4a5c50d1653eaa0ba8789accc

Judged base: 9dda4a4

Manifest version: 2

Pause-bound ruling: (none)

BRIEF CONFORMANCE: Brief conformance: resolveRemoteBase now builds on startingRemoteBase, which is the old chain moved into its own function without changes, and passes its result to mergedCommitAfter. That function runs git rev-list --first-parent --merges <start>..HEAD and lists merges newest first. For each merge it checks whether <merge>^2 is an ancestor of the start using git merge-base --is-ancestor, read through the existing ReadFailure opt-in, and returns the first second parent that is not an ancestor. If no merge qualifies, or a read fails, it returns the start. This matches the brief's Boundary, and the remote-tracking ref is never read for the merge rule. Spec conformance: apps/cli/specs/self-hosting.md only refers to lib/remote-base.ts as the source of the changed files and does not set out a base order, so there is no drift. Scope: the diff touches only apps/cli/src/lib, apps/cli/tests/lib and .changeset, all inside the Surface. Callers, environment variables and CI selection are untouched. Tests: they use real fixture repositories with a bare remote and nothing is mocked. The merge commit is fetched by URL into FETCH_HEAD, so origin/main stays stale, and the tests assert this. They also check the base and the changed files [f1, f2], the same result for a ready push whose named base is the stale origin/main, and the no-merge case where the upstream stands and a ready push keeps its named base. Docs: the file header and the comment above resolveRemoteBase both describe the merged-commit rule correctly. The changeset is a patch entry for @attalabs/vinaya. I found nothing that blocks this change.
SPEC CONFORMANCE: (covered by the review summary)

OBJECTIVES:
O1: MET — apps/cli/src/lib/remote-base.ts:90-123: resolveRemoteBase returns mergedCommitAfter(repoRoot, startingRemoteBase(repoRoot)). The starting chain is unchanged: named base, then @{u}, then origin/main, then HEAD~1. mergedCommitAfter takes the merges from rev-list --first-parent --merges ${start}..HEAD newest first and returns the first ${merge}^2 for which merge-base --is-ancestor merged start exits non-zero. Only HEAD's history is read; origin/main is consulted only in the old starting-base fallback.
O2: MET — When the range holds no qualifying merge, rev-list prints nothing (or fails, which the git() helper turns into ''), so the loop does not run and start is returned. startingRemoteBase keeps the old logic line for line (lines 94-106). The test at remote-base.test.ts 'with no merge, the upstream stands…' asserts origin/feature and [f2] for the upstream case, and the named SHA for a ready push.
O3: MET — apps/cli/tests/lib/remote-base.test.ts, new describe block. mergedBranchFixture builds a bare remote, pushes a branch, adds a main commit from a second clone, merges it via FETCH_HEAD and adds the branch-only f2.txt. Test 1 asserts origin/main != merged, base === merged, and changed files [f1.txt, f2.txt]. Test 2 sets VINAYA_PUSH_MODE=ready and VINAYA_PUSH_BASE=origin/main (the stale ref) and gets the same base and files. Test 3, with no merge, gets base origin/feature with files [f2.txt], and a ready push keeps its named initial SHA.

FINDINGS (ordered by severity):
None.

SCOPE: (covered by the review summary)
TESTS: (covered by the review summary)
DOCS: (covered by the review summary)

Tokens: 1823: review — Reviewer — claude — 17/3853/—
Cast by: Reviewer (session 3d9a17ff-c8b6-4dd0-99a0-faec2d0fa05b)

@daniboomerang

Copy link
Copy Markdown
Member Author

VERDICT: PASS

Judged head: a69a0f2

Objectives version: 33ff68c313ae563f337e21468ea2b63410c803c4dc3e26b152c45fa3229084c8

Ruling ordinal: 0

Brief hash: 048ee7c3e318aea62547a6b384c8f405123b572376a766e0ead056ed13025f28

Policy digest: bf51ddb9c7bd4776744d56ac91f4005ee8b2c1e4a5c50d1653eaa0ba8789accc

Judged base: 9dda4a4

Manifest version: 2

Pause-bound ruling: (none)

FINDINGS (ordered by severity):
None.

OBJECTIVES:
O1: MET — apps/cli/src/lib/remote-base.ts: resolveRemoteBase now returns mergedCommitAfter(repoRoot, startingRemoteBase(repoRoot)). startingRemoteBase is the unchanged order: ready push's named base, then @{u}, then origin/main, then HEAD~1. mergedCommitAfter runs rev-list --first-parent --merges <start>..HEAD, which lists newest first, takes each merge's ^2 and returns the first one where merge-base --is-ancestor <merged> <start> exits non-zero (read through ReadFailure). It reads only HEAD's history and never the origin/main ref.
O2: MET — When the rev-list output is empty, or every merge's second parent is already an ancestor of the start, or a read fails, mergedCommitAfter returns start unchanged. startingRemoteBase is the old body moved verbatim. The test 'with no merge, the upstream stands and a ready push keeps its named base' asserts 'origin/feature' with ['f2.txt'], and that the ready push keeps its named initial SHA.
O3: MET — apps/cli/tests/lib/remote-base.test.ts, new describe block. mergedBranchFixture builds a bare remote and pushes the feature branch. It pushes a default-branch commit from a second clone, fetches it by URL into FETCH_HEAD and merges it --no-ff, so origin/main stays stale (asserted with not.toBe(merged)). It then adds one branch-only commit. The tests assert base === merged and changed files ['f1.txt','f2.txt']. The same expectations hold under VINAYA_PUSH_MODE=ready with VINAYA_PUSH_BASE=origin/main. A no-merge case covers the upstream and the ready push's named base.

CONFIG SCAN: (covered by the review summary)

SECRETS: SECRETS: none found on my own read of the diff. The atta-labs/secret-scan check has not run yet: CI has not run because no pull request exists, so this rests only on my read of the diff. The only identity-like strings are the placeholder fixture values test@example.com and Test. AGENT-CONFIG: not applicable — this pull request changes no agent configuration (.claude/, .mcp.json, .agents/), so no scanner was run. My own read found no agent, hook or MCP config touched. Security read: the new mergedCommitAfter in apps/cli/src/lib/remote-base.ts calls git only through the file's existing git() helper, which uses execFileSync with an argument array (no shell) and a GIT_*-stripped environment. Its inputs are the existing starting base (unchanged trust: the local VINAYA_PUSH_BASE environment variable, @{u}, origin/main or HEAD~1) and commit SHAs printed by git rev-list and rev-parse, so no new injection surface. There are no new dependencies, and no auth, crypto, BYOK, network or permission surface. Every failure path falls back to the starting base, so nothing new can fail outright. Functionally, all three objectives are met by the code and fixture tests. No findings.

Tokens: 1823: security — Security — claude — 6/2344/—
Cast by: Security (session 4a01eee7-493d-449b-98c4-bbd56f35fce0)

@daniboomerang

Copy link
Copy Markdown
Member Author

No description provided.

@daniboomerang
daniboomerang merged commit 61594df into main Nov 2, 2026
22 of 28 checks passed
@daniboomerang
daniboomerang deleted the task/issue-1823 branch November 2, 2026 06:24
@daniboomerang

Copy link
Copy Markdown
Member Author

The dev-review-loop paused: infrastructure — an uncaught error ended round 1's own processing: check-publish-mergeability found no open pull request for branch task/issue-1823.

A Principal ruling is needed before this can continue. A loop process still running picks up a posted ruling by itself; use this --resume command only when that process has stopped:

vinaya task run --issue 1823 --agent claude --model opus --runner atta-engine

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

AEG provenance — task (backlog Issue #1823)

Tokens: 1823: archive — Archivist — — — —

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix(cli): A push after merging the default branch measures only the branch's own change

1 participant