improve(ci): remove remaining warm-cache critical paths#109593
Conversation
openclaw-barnacle
Bot
added
docs
63714a3 to
ec87bd9
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21e1c11cc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| with: | ||
| path: .artifacts/build-all-cache | ||
| key: ${{ runner.os }}-build-all-v3-${{ hashFiles('package.json', 'pnpm-lock.yaml', 'npm-shrinkwrap.json', 'packages/plugin-sdk/package.json', 'packages/llm-core/package.json', 'packages/model-catalog-core/package.json', 'packages/memory-host-sdk/package.json', 'scripts/build-all.mjs', 'scripts/write-plugin-sdk-entry-dts.ts', 'scripts/lib/plugin-sdk-entries.mjs', 'tsconfig.json', 'tsconfig.plugin-sdk.dts.json', 'src/plugin-sdk/**', 'packages/llm-core/src/**', 'packages/model-catalog-core/src/**', 'packages/memory-host-sdk/src/**', 'src/types/**', 'src/video-generation/dashscope-compatible.ts', 'src/video-generation/types.ts', 'scripts/copy-export-html-templates.ts', 'scripts/lib/copy-assets.ts', 'src/auto-reply/reply/export-html/**') }} | ||
| key: ${{ runner.os }}-build-all-v4-${{ hashFiles('package.json', 'pnpm-lock.yaml', 'npm-shrinkwrap.json', 'scripts/build-all.mjs', 'scripts/write-plugin-sdk-entry-dts.ts', 'scripts/lib/plugin-sdk-entries.mjs', 'scripts/lib/plugin-sdk-entrypoints.json', 'scripts/lib/plugin-sdk-private-local-only-subpaths.json', 'scripts/lib/plugin-sdk-deprecated-public-subpaths.json', 'scripts/lib/plugin-sdk-deprecated-barrel-subpaths.json', 'scripts/copy-export-html-templates.ts', 'scripts/lib/copy-assets.ts', 'tsconfig.json', 'tsconfig.plugin-sdk.dts.json', 'src/**', 'packages/**', '!src/**/dist/**', '!src/**/node_modules/**', '!packages/**/dist/**', '!packages/**/node_modules/**') }} |
There was a problem hiding this comment.
When a PR changes one of the newly hashed plugin SDK JSON files here (for example scripts/lib/plugin-sdk-entrypoints.json), this cache key now correctly treats it as a build input, but the preflight predicate that decides whether build-artifacts runs still skips all scripts/lib/*.json: BUILD_INPUT_RE in scripts/lib/ci-changed-node-test-plan.mjs:42-43 only allows copy-assets.ts and plugin-sdk-entries.mjs. On canonical PRs the manifest sets run_build_artifacts from that predicate, so SDK export/deprecation-list changes can avoid pnpm build:ci-artifacts and the plugin SDK export gate entirely; please add these JSON inputs to the build-impact regex and its tests.
Useful? React with 👍 / 👎.
21e1c11 to
b3d2954
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3d2954418
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| extensions: [".ts", ".tsx", ".mts", ".cts", ".json"], | ||
| excludeDirectories: ["dist", "node_modules"], |
There was a problem hiding this comment.
When a PR changes ambient declaration inputs such as src/types/*.d.ts, build-artifacts restores the broad build-all-v4- cache via the coarse restore key, but the new internal fingerprint for the self-built plugin SDK declarations only hashes .ts/.tsx/.mts/.cts/.json files under src and packages. tsconfig.plugin-sdk.dts.json explicitly includes src/types/**/*.d.ts, so those changes can leave write-plugin-sdk-entry-dts marked fresh and restore stale flat/package-bridge declarations before check-plugin-sdk-exports runs. Please include .d.ts in both broad source input lists so declaration-only type changes invalidate this cache.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
|
Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 12:43 AM ET / 04:43 UTC. Summary PR surface: Source +49, Tests +80, Docs +1, Config 0, Other +39. Total +169 across 10 files. Reproducibility: yes. from source: a change limited to one of the four newly fingerprinted SDK JSON manifests is not matched by the unchanged build-impact predicate, so build-artifacts can be omitted. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Align build-impact detection with all four SDK manifest inputs, add a manifest-only changed-path regression test, and provide a cold/warm exact-head run showing the artifact gate executes and restores the corrected cache safely. Do we have a high-confidence way to reproduce the issue? Yes from source: a change limited to one of the four newly fingerprinted SDK JSON manifests is not matched by the unchanged build-impact predicate, so build-artifacts can be omitted. Is this the best way to solve the issue? No, not yet; the cache design is coherent, but its changed-path gate must recognize every newly declared input so validation cannot be skipped. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7641aa80c23b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +49, Tests +80, Docs +1, Config 0, Other +39. Total +169 across 10 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
Review history (1 earlier review cycle)
|
b3d2954 to
d4c7873
Compare
clawsweeper
Bot
added
status: 📣 needs proof
|
Merged via squash.
|
1 participant
Related: #109425
What Problem This Solves
Warm transform caches exposed two remaining CI critical-path costs: the broad core shard started late in the full matrix, one registration test spent minutes evaluating the complete command-handler graph for two order assertions, and self-built plugin SDK declarations were regenerated on every build-artifact job.
Why This Change Was Made
Admit the broad core-fast cache writer in the first full-matrix wave without changing shard layout. Make command precedence a typed, data-only contract so the order test avoids importing every handler while the runtime still proves complete handler coverage. Persist self-built declaration outputs behind a content fingerprint of the repository-owned TypeScript/JSON graph, excluding installed and generated directories; rotate the outer CI cache key to match.
The declaration cache is job-local while writing and immutable between jobs. Stale coarse restores remain safe because
build-allverifies its own versioned input fingerprint before restoring outputs. Cache size is bounded by the existing public declaration graph ratchet; the measured entry is 7 MB.User Impact
No product behavior changes. Maintainers should see the first reusable transform seed start earlier, the former core-fast quiet tail disappear, and warm build-artifact jobs skip roughly 24 seconds of declaration generation even after partial non-source changes.
Evidence
build:ci-artifacts: declaration phase 23.8s to 157ms; total build 41.2s to 7.94s. The 7 MB cache restored flat declarations aftertsdowncleareddist, and the plugin SDK export graph gate passed both runs.SIGKILL; its exact rerun passed 88 files / 1,457 tests in 66.59s, matching two earlier green runs (66.09s and 64.92s) with the same Vitest/cache configuration. No assertion, timeout, cache error,ENOTEMPTY,EEXIST,EBUSY,ENOSPC, or cancellation signature appeared.build-all-v4key:build-artifactsfell from 152s to 76s, while representativechecks-node-compact-large-1fell from 169s to 116s. Across all 26 paired Linux Node jobs, summed job duration fell from 3301s to 2954s; 20 improved, with a median 8.5s reduction. The registration test completed in 0ms inside the shard.AI-assisted: yes. I reviewed the cache invalidation/output contracts, handler ordering, admission behavior, and validation results.