ci: protoc resolution + typecheck/lint/test workflow#47
Closed
electron-rare wants to merge 6 commits into
Closed
Conversation
Context: build-proto.mjs hard-coded the grpc-tools bundled protoc path. When the grpc-tools postinstall download fails, that binary is absent and `npm run protos` exits with status 127, leaving the proto TS modules ungenerated and ~431 tsc errors. Approach: replace the rigid constant with resolveProtoc(), a fallback chain. Changes: - Try the bundled grpc-tools/bin/protoc(.exe) first, gated on an executable-access check, to keep version consistency. - Fall back to a protoc found on PATH via which/where. - Fall back to a bare protoc if it answers --version. - Otherwise print a clear remediation message and exit. Impact: npm run protos now succeeds with a system protoc; no other proto pipeline behaviour changes.
Context: .github/workflows only had pack-cli.yml, triggered on v*-cli tags. No workflow validated push or pull requests. Changes: - Add ci.yml triggered on push to master and on pull_request. - build job: install, generate protos, check-types, lint (blocking). - test job: run the mocha unit suite with continue-on-error, since it is flaky in CI and should signal without gating. Impact: every push and PR now gets typecheck and lint coverage.
Patches from the critic review of PR #33: - ci.yml: drop the redundant "Generate Protocol Buffers" step in the build job — `check-types` already runs `npm run protos`. - ci.yml: add a concurrency group (cancel superseded runs), `permissions: contents: read`, and `timeout-minutes` on both jobs. - ci.yml: rename the test job "Unit Tests (non-blocking)" so the continue-on-error behaviour is explicit rather than implied. - build-proto.mjs: warn when falling back to a system protoc so a version divergence from grpc-tools is visible in the log.
The first CI run exposed a pre-existing bootstrap defect: the `cli` workspace has a `prepare` script that builds (and runs tsc) during `npm install`, which fails before protos exist. Approach: install with `--ignore-scripts` so the cli prepare build does not run at install time, install a system protoc (grpc-tools' postinstall is skipped too), then generate protos and run the checks in the correct order. Changes: - Both jobs: `npm install --ignore-scripts`. - Both jobs: add an "Install protoc" step (apt protobuf-compiler) so build-proto.mjs has a protoc to fall back to.
Context: npm --ignore-scripts does not suppress prepare for local workspace packages (known npm v7+ behaviour). The cli workspace prepare runs tsc which fails before protos are generated. Approach: split install into phases so protos exist before cli prepare runs. Changes: - Phase 1: npm install --workspaces=false --ignore-scripts (root deps only, cli workspace not linked, no prepare triggered) - Phase 2: sudo apt-get install protobuf-compiler + npm run protos - Phase 3: npm install --workspace=cli (prepare now succeeds) - Phase 4: cd webview-ui && npm install (needed for webview-ui tsc) - Same fix applied to the test job for consistency Impact: Typecheck & Lint job can now install and typecheck without hitting the chicken-and-egg proto/prepare deadlock.
Remove the unused `os` import in ailiance-memory.test.ts (dead code left from a refactor that replaced os.homedir() mocking with vi.mock). Replace the Logger.warn/console.warn fallback pattern in ailiance-memory.ts with a direct Logger.warn call. The fallback was unnecessary: Logger is always available in this context, and biome's no-console rule flags the console.warn at error level.
Author
|
Closing: superseded. The protoc-resolution + CI workflow changes on this branch are already in master (ci.yml, build-proto.mjs, ailiance-memory.ts all byte-identical to master). The branch forked from an old master and is ~5500 lines behind; merging would add nothing. Safe to delete the branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the GitHub Actions CI workflow (Typecheck & Lint + Unit Tests), a protoc binary resolution fallback chain in build-proto.mjs, and a minor lint cleanup. Foundational CI/infra; merges cleanly into master.
Opened as part of merging the remaining clean branches. CI on this PR validates the workflow itself.