Skip to content

fix(ci): isolate compact planner inventory growth fixture - #140549

Merged
vincentkoc merged 1 commit into
mainfrom
fix/compact-planner-inventory-fixture
Sep 7, 2026
Merged

fix(ci): isolate compact planner inventory growth fixture#140549
vincentkoc merged 1 commit into
mainfrom
fix/compact-planner-inventory-fixture

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Sep 7, 2026

Copy link
Copy Markdown
Member

Related: #140512
Related: #137893

What Problem This Solves

Fixes an issue where the compact planner inventory-growth regression duplicated its fixture filename when another PR added that file for real, causing a false 81 planned failure even though the real merged inventory remained within the 80-job cap.

Why This Change Was Made

The test now constructs its baseline and one-file growth inventories explicitly. Each inventory uses a separate dynamic import because the planner initializes SPLIT_NODE_SHARDS at module load: the baseline removes the growth path, and the growth case adds exactly one copy.

All existing planner policy assertions remain unchanged, including the 80-job and 150-second limits, exact-once coverage, family separation, runner rank, build/dist policy, non-tooling placement, and deterministic mixed-runner ordering.

This test-only PR is policy-compliant: main is green for this case, while #137893 turns the fixture's formerly synthetic filename into a real tracked path and exposes the duplicate-fixture bug.

User Impact

Maintainers can validate legitimate tooling inventory growth without a false compact-plan cap failure. Production behavior and the planner algorithm are unchanged.

Evidence

  • Pre-fix controlled reproduction with the fix(docker): support legacy fs-safe package exports #137893 path present: focused test failed with compact github node test plan exceeds 80 jobs (81 planned).
  • Focused regression after the fix: 1 passed.
  • Full planner suite: 51 passed.
  • oxfmt, targeted oxlint, and git diff --check: passed.
  • check:changed: all reachable guards passed; local test-root tsgo stopped only because linked-worktree declaration inputs resolve through the shared install outside the checkout. Exact-head CI covers that lane.
  • Fresh autoreview through P2: scoped clean.
  • LOC: production 0; tests +73/-65.
  • Codex hard gate: sibling Codex 5f49aba876922d6f2f55caa153bbb0ed1b46feba was inspected at codex-rs/core/src/tools/registry.rs, codex-rs/core/src/session/mcp.rs, and codex-rs/core/src/tools/context.rs; no planner or runtime coupling exists.

@clawsweeper

clawsweeper Bot commented Sep 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@vincentkoc vincentkoc self-assigned this Sep 7, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review September 7, 2026 00:46
@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Sep 7, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 7, 2026
@clawsweeper

clawsweeper Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 8:51 PM ET / September 7, 2026, 00:51 UTC.

ClawSweeper review

What this changes

The PR gives the CI planner regression separate baseline and growth inventories so a newly tracked test filename cannot be counted twice.

Merge readiness

✅ Ready for maintainer review

Keep open: current main still contains the fixture collision, and this focused repair preserves the existing planner checks. No introduced correctness or security defects were found.

Priority: P2
Reviewed head: 713f6768d21495f69838c1a5bc78aab1bc9d280a

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused fixture repair with preserved assertions, relevant reported validation, and no actionable findings.
Proof confidence 🌊 off-meta tidepool Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored test repair; its reported controlled failure and passing planner suite directly address the changed fixture.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored test repair; its reported controlled failure and passing planner suite directly address the changed fixture.
Evidence reviewed 7 items Introduced repair and retained assertions: The verified merge-base-to-head diff removes the growth filename before constructing either inventory, adds it once for the growth case, and preserves the existing coverage, placement, runner, build, ordering, and capacity assertions.
Current main still needs the repair: Fetched main still appends syntheticToolingFile to the real inventory without removing an existing occurrence. Its tracked tree does not yet contain that filename, so the reported trigger is conditional on the related addition rather than an established failing main run.
Inventory initialization boundary: The planner constructs tooling shards in SPLIT_NODE_SHARDS during module evaluation. Separate dynamic imports therefore give each controlled inventory its own initialized plan; finally blocks remove the mock and reset modules.
Findings None None.
Security None None.

How this fits together

OpenClaw’s CI planner groups tracked test files into jobs with runner, build, and capacity limits. This regression checks that adding a tooling test preserves coverage and stays within those limits.

flowchart TD
  A[Tracked test files] --> B[Controlled test inventory]
  B --> C[Baseline without growth file]
  B --> D[Growth file included once]
  C --> E[Fresh CI planner]
  D --> E
  E --> F[Coverage and job limit assertions]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Tests +8. Total +8 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 73 65 +8
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 73 65 +8

Review metrics

None.

Technical review

Best possible solution:

Keep the controlled one-file inventory transition and fresh planner initialization while retaining all existing policy assertions.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes the conditional trigger: track the chosen growth filename and the original test appends a duplicate. The reported 81-job failure was not independently executed during this read-only review.

Is this the best way to solve the issue?

Yes. Repairing the test inventory producer is narrower than changing production packing or relaxing capacity and coverage checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 2413c5e3dcde.

Labels

Label changes:

  • add P2: The repair prevents a bounded CI regression from rejecting legitimate tooling-test additions.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored test repair; its reported controlled failure and passing planner suite directly address the changed fixture.

Label justifications:

  • P2: The repair prevents a bounded CI regression from rejecting legitimate tooling-test additions.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this MEMBER-authored test repair; its reported controlled failure and passing planner suite directly address the changed fixture.

Evidence

What I checked:

  • Introduced repair and retained assertions: The verified merge-base-to-head diff removes the growth filename before constructing either inventory, adds it once for the growth case, and preserves the existing coverage, placement, runner, build, ordering, and capacity assertions. (test/scripts/ci-node-test-plan.test.ts:2132, 713f6768d214)
  • Current main still needs the repair: Fetched main still appends syntheticToolingFile to the real inventory without removing an existing occurrence. Its tracked tree does not yet contain that filename, so the reported trigger is conditional on the related addition rather than an established failing main run. (test/scripts/ci-node-test-plan.test.ts:2146, 2413c5e3dcde)
  • Inventory initialization boundary: The planner constructs tooling shards in SPLIT_NODE_SHARDS during module evaluation. Separate dynamic imports therefore give each controlled inventory its own initialized plan; finally blocks remove the mock and reset modules. (scripts/lib/ci-node-test-plan.mts:1837, 713f6768d214)
  • Production contract remains intact: Tooling discovery retains file entries without deduplication, while compact planning enforces the job cap. The test corrects its input producer instead of weakening these production rules. The workflow calls this OpenClaw planner directly; no Codex dependency contract is implicated. (scripts/lib/ci-node-test-plan.mts:2259, 713f6768d214)
  • Related work and reported validation: The supplied GitHub snapshot identifies fix(ci): keep compact GitHub plans within the job cap #140512 as the merged planner repair and fix(docker): support legacy fs-safe package exports #137893 as the separate open change exposing the filename collision. The reviewed body reports a controlled pre-fix 81-job failure, a passing focused regression, and 51 passing planner tests. These are contributor-reported results; this review did not execute tests. (713f6768d214)
  • Area history and routing: Current-main history records Vincent Koc’s prior merged compact-planner work at c844442. Raw commit metadata was inspected, but unavailable parent blobs prevented exact source-line introduction verification; no introducing-author claim is made. (test/scripts/ci-node-test-plan.test.ts:2097, c8444424e0c3)

Likely related people:

  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@vincentkoc
vincentkoc merged commit b044ff9 into main Sep 7, 2026
170 checks passed
@vincentkoc
vincentkoc deleted the fix/compact-planner-inventory-fixture branch September 7, 2026 00:55
@vincentkoc

vincentkoc commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Sep 7, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 7, 2026
@vincentkoc
fix(ci): isolate compact planner inventory growth fixture (openclaw#1…
75cd642 
…40549)
@steipete steipete mentioned this pull request Sep 7, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vincentkoc