Repository navigation
ci: select affected checks and separate full integration verification - #256
Conversation
Run PR and main checks from changed inputs and the Go dependency graph. Keep complete verification on nightly/manual runs and global CI/dependency changes, and require that explicit result before releasing. Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude <81847+claude@users.noreply.github.com>
NanaseInori
left a comment
There was a problem hiding this comment.
Reviewed against 669dc78817f3ab2463d15a70519b1dc38348d43e. Full CI is green, but I found three reproducible gaps in the partial-selection paths:
-
[P2] Validate the actual production bundle in the frontend lane —
.github/workflows/ci.yml:317–318internal/webuitests use synthetic bundles, whilewebhas no test files, sogo test -tags desktop ./webonly checks that embedding compiles. Frontend-only changes now skip the native jobs and therefore skipTestEmbeddedProductionUIBundleLoadsApprovedFontsAndNotices. Using the unchanged loader, tests and embedding adapter, I reproduced both new Go commands passing with an embedded.js.mapasset, while the actual Desktop startup loader fails withunsupported extension ".map". Please runLoadEmbeddedFSagainst the actual built production assets in this lane; this can be a Linux-runnable test without restoring packaging. -
[P2] Preserve the Go-to-OpenAPI golden check for indirect HTTP changes —
scripts/ci/run_go_checks.py:166–173The real package graph selects only compilation for
httpapiwheninternal/application/github_review_service.gochanges, with no selected HTTP integration test and no web job. This file contains DTOs directly reflected by the OpenAPI generator. Adding an optional JSON field toGitHubReviewCredentialViewin an isolated overlay makesTestOpenAPIGoldenDocumentMatchesGoDTOsfail becausedocs/openapi.jsonis stale, but the selected plan omits that check.npm run check:apionly validates the committed JSON-to-TypeScript step. Please retain the golden test when HTTP is affected but not fully tested, and when the OpenAPI snapshot changes. -
[P2] Include shared testdata consumers in the affected set —
scripts/ci/run_go_checks.py:141–144internal/agentpackages/testdata/launch-handoff/mcp.jsonis also read directly by MCP integration tests, but a change to this fixture selects onlyinternal/agentpackages. I changed onlydefault-cwd.commandfrom./bin/helper.exeto./work/bin/helper.exe: the actual selected runner, including vet and the complete agentpackages suite, passes, while the omittedTestAgentPluginsSharedLaunchHandofffails atclient_agentplugins_integration_test.go:103. Please model these existing shared fixture consumers explicitly, or use a conservative fallback for shared fixture paths.
These can be addressed while preserving the affected-check design. The current PR selects full CI because it changes CI machinery, so its successful full run does not exercise these partial-selection cases.
Run the existing production bundle loader test from the Linux frontend lane and preserve its native and packaging coverage. Keep the OpenAPI golden check for indirect HTTP consumers and snapshot-only changes. Declare shared fixture consumers without propagating test-only input changes through production edges. Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude <81847+claude@users.noreply.github.com>
NanaseInori
left a comment
There was a problem hiding this comment.
Re-reviewed at 78ac92b7d53ce597351c0846d0ab045ee0077993. All three issues from my previous review are resolved.
- The frontend lane now executes the relocated production-bundle test, including the actual
LoadEmbeddedFScall and the existing font/license assertions. The previous.js.mapfault is now rejected by that test, and removing the fault restores a passing result. Native and packaging callers also retain the relocated test. - Indirect HTTP changes now retain the Go DTO-to-OpenAPI golden check. Snapshot-only changes also select the targeted Go check alongside the frontend checks. Both the DTO-overlay and snapshot-mutation cases are detected.
- Shared launch-handoff fixtures now select both the agentpackages and MCP suites. Repeating the previous command-path mutation causes the actual selected runner to fail in the MCP consumer; restoring the fixture makes the same selection pass.
The 54 CI helper tests pass locally, and both full CI and Desktop release validation have succeeded for this exact head.
I found no remaining blocking issues in this re-review. Approve.
Summary
Small frontend and Go PRs currently start the entire backend, Store, LSP, Rust and native desktop matrix. Select PR and main-push checks from changed inputs instead. Frontend changes retain API drift, tests, build, dependency audit and actual production bundle loading without starting the backend integration suites or release packaging.
Go selection uses the real import graph. Directly changed packages run complete tests; indirect consumers run tests, with the four large integration packages compiling and running selected existing provider/MCP regressions. Production dependencies propagate; test imports add only their test consumer. Store shards and real native/LSP/browser/analyzer checks run when their inputs are affected.
Keep cross-package contracts in partial runs:
LoadEmbeddedFSloader used by Desktop startup. Native and packaging scripts also run this test after its move toweb.Full verification runs nightly at 03:23 Asia/Hong_Kong and through Actions CI → Run workflow. CI machinery, Go dependency changes and unclassified inputs also select full checks. Authority race checks and the Go audit run independently of ordinary Go tests. Native-only changes keep desktop boundary tests but omit archive/reproducibility builds.
The release workflow now limits PR packaging to release tooling and distribution inputs. Tags/manual releases require a successful Full CI verification job for the exact commit and run attempt; a successful partial CI run is insufficient. Existing font/branding assertions remain in the central release-contract tests, with obsolete requirements to trigger packaging removed.
Direct edits to large packages such as
applicationstill run their complete suites. This change does not claim that every PR finishes within a fixed number of minutes.CI redesign idea credited to Claude; implementation credited to Codex in the commit coauthor trailers.
Validation
.js.mapcaused the selected test to fail with the loader's unsupported-extension error. It passed again after removing the probe.78ac92b7: CI and Desktop release validation are running. These full runs are separate from the partial-selection regression evidence above.Surface governance
Go control planeresult and add an explicit full-only release result.Audit
docs/ci.mddocuments selection, full runs and release prerequisites.