fix(ci,engine): repair test sharding, case-variant ambiguity detection, post-merge CI
Test shards 3 and 4 were silently failing on every open PR because vitest's CLI parser was treating `--shard X/Y` as positional file filters whenever the arg arrived after a `--` separator. Removing the `--` in ci-test-shard.mjs restores per-shard slicing; verified locally that shard 1/4 and 2/4 now run distinct subsets. The two consistently-failing engine tests: 1. self-healing in-review-branch-rebind ambiguous case-variant detection: dedup keyed on lowercase branch name collapsed two physically distinct refs (allowed on Linux ext4) into one candidate, so the "applied" path ran instead of "ambiguous-candidates". Dedup now keys on the resolved SHA — macOS APFS still collapses (same ref, same SHA), Linux keeps both (distinct SHAs) and the ambiguity skip path fires as designed. 2. worktree-acquisition resume-misbinding spy: the production verifyResumeBranchNotMisbound returns early when `git merge-base HEAD main` fails, which is exactly what happens on shallow checkouts. Bumping the test-shards checkout to fetch-depth: 0 makes CI mirror the local git state these engine tests rely on. Also adds `push: branches: [main]` to PR Checks so regressions like this (which slipped into v0.33.0 with no post-merge run) go red immediately on landing instead of being discovered on the next PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
10
.changeset/ci-engine-test-fixes.md
Normal file
10
.changeset/ci-engine-test-fixes.md
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix two engine reliability bugs surfaced by CI sharding repair:
|
||||||
|
|
||||||
|
- Self-healing in-review branch rebind now dedups case-variant candidate refs by resolved SHA rather than lowercase name, so two distinct branches sharing a case-insensitive name on case-sensitive filesystems (Linux) are correctly flagged as ambiguous instead of one being silently picked.
|
||||||
|
- CI test sharding: removed the `--` separator between `pnpm test` and `--shard`, which vitest's CLI parser was treating as end-of-flags and turning the shard selector into a positional file filter — silently disabling sharding so every shard ran the full suite. Test shards now run their actual slice.
|
||||||
|
- CI test-shards jobs now check out with `fetch-depth: 0` so engine tests that depend on real git history (merge-base, ref resolution) behave the same on CI as locally.
|
||||||
|
- PR Checks workflow now also runs on push to `main`, so post-merge regressions surface immediately instead of waiting for the next PR.
|
||||||
10
.github/workflows/pr-checks.yml
vendored
10
.github/workflows/pr-checks.yml
vendored
@@ -3,6 +3,10 @@ name: PR Checks
|
|||||||
on:
|
on:
|
||||||
pull_request:
|
pull_request:
|
||||||
branches: [main]
|
branches: [main]
|
||||||
|
# Also run on every push to main so post-merge regressions surface
|
||||||
|
# immediately instead of being discovered on the next PR.
|
||||||
|
push:
|
||||||
|
branches: [main]
|
||||||
|
|
||||||
concurrency:
|
concurrency:
|
||||||
group: pr-checks-${{ github.ref }}
|
group: pr-checks-${{ github.ref }}
|
||||||
@@ -69,6 +73,12 @@ jobs:
|
|||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
# Engine tests run real git operations (merge-base against main,
|
||||||
|
# case-variant ref checks) that require full history. Shallow
|
||||||
|
# clones silently break tests like worktree-acquisition's resume
|
||||||
|
# misbinding path.
|
||||||
|
fetch-depth: 0
|
||||||
|
|
||||||
- name: Setup Node.js and pnpm
|
- name: Setup Node.js and pnpm
|
||||||
uses: ./.github/actions/setup-node-pnpm
|
uses: ./.github/actions/setup-node-pnpm
|
||||||
|
|||||||
@@ -2946,16 +2946,26 @@ export class SelfHealingManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const integrationBase = task.baseBranch || await resolveIntegrationBranch(this.options.rootDir, undefined);
|
const integrationBase = task.baseBranch || await resolveIntegrationBranch(this.options.rootDir, undefined);
|
||||||
const existingCandidatesByRef = new Map<string, { branch: string; aheadCount: number }>();
|
// Dedup by resolved SHA, not by lowercase name. On case-insensitive
|
||||||
|
// filesystems (macOS APFS default) two case-variant refs resolve to the
|
||||||
|
// same underlying ref → same SHA → collapse to canonical. On
|
||||||
|
// case-sensitive filesystems (Linux) two case-variants are physically
|
||||||
|
// distinct refs with distinct SHAs → keep both, so downstream detects
|
||||||
|
// the ambiguity rather than silently picking one.
|
||||||
|
const candidateByRefSha = new Map<string, { branch: string; aheadCount: number }>();
|
||||||
|
const normalizedCandidate = canonicalFusionBranchName(task.id);
|
||||||
for (const branch of candidates) {
|
for (const branch of candidates) {
|
||||||
|
let branchSha: string;
|
||||||
try {
|
try {
|
||||||
await execAsync(`git show-ref --verify --quiet ${shellQuote(`refs/heads/${branch}`)}`, {
|
const { stdout } = await execAsync(`git rev-parse --verify ${shellQuote(`refs/heads/${branch}`)}`, {
|
||||||
cwd: this.options.rootDir,
|
cwd: this.options.rootDir,
|
||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
});
|
});
|
||||||
|
branchSha = stdout.trim();
|
||||||
} catch {
|
} catch {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
if (!branchSha) continue;
|
||||||
|
|
||||||
let comparisonBase = integrationBase;
|
let comparisonBase = integrationBase;
|
||||||
try {
|
try {
|
||||||
@@ -2980,18 +2990,16 @@ export class SelfHealingManager {
|
|||||||
timeout: 30_000,
|
timeout: 30_000,
|
||||||
});
|
});
|
||||||
const aheadCount = Number.parseInt(aheadCountRaw.stdout.trim(), 10);
|
const aheadCount = Number.parseInt(aheadCountRaw.stdout.trim(), 10);
|
||||||
const normalizedBranchRef = branch.toLowerCase();
|
const existing = candidateByRefSha.get(branchSha);
|
||||||
const existingCandidate = existingCandidatesByRef.get(normalizedBranchRef);
|
if (!existing || branch === normalizedCandidate) {
|
||||||
const normalizedCandidate = canonicalFusionBranchName(task.id);
|
candidateByRefSha.set(branchSha, {
|
||||||
if (!existingCandidate || branch === normalizedCandidate) {
|
|
||||||
existingCandidatesByRef.set(normalizedBranchRef, {
|
|
||||||
branch,
|
branch,
|
||||||
aheadCount: Number.isFinite(aheadCount) ? aheadCount : 0,
|
aheadCount: Number.isFinite(aheadCount) ? aheadCount : 0,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const existingCandidates = [...existingCandidatesByRef.values()];
|
const existingCandidates = [...candidateByRefSha.values()];
|
||||||
|
|
||||||
if (existingCandidates.length === 0) {
|
if (existingCandidates.length === 0) {
|
||||||
await this.emitBranchRebindAuditEvent({
|
await this.emitBranchRebindAuditEvent({
|
||||||
|
|||||||
@@ -396,9 +396,15 @@ export function main(argv = process.argv.slice(2), env = process.env) {
|
|||||||
console.log(
|
console.log(
|
||||||
`[ci-test-shard] shard ${shard}/${total}: running ${entry.name} --shard ${entry.shardIndex}/${entry.shardCount}`,
|
`[ci-test-shard] shard ${shard}/${total}: running ${entry.name} --shard ${entry.shardIndex}/${entry.shardCount}`,
|
||||||
);
|
);
|
||||||
run("pnpm", ["--filter", entry.name, "test", "--", "--shard", `${entry.shardIndex}/${entry.shardCount}`], {
|
// NB: no `--` between `test` and `--shard`. pnpm 10 forwards extra args to
|
||||||
env: shardEnv,
|
// the script regardless, and inserting `--` causes vitest's CLI parser
|
||||||
});
|
// (cac) to treat `--shard X/Y` as positional file filters → sharding is
|
||||||
|
// silently disabled and every shard runs the full suite.
|
||||||
|
run(
|
||||||
|
"pnpm",
|
||||||
|
["--filter", entry.name, "test", `--shard=${entry.shardIndex}/${entry.shardCount}`],
|
||||||
|
{ env: shardEnv },
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user