Skip to content

CI runner fixes - #2418

Merged
dividedmind merged 2 commits into
mainfrom
ci/runner-fixes
Oct 27, 2026
Merged

dividedmind merged 2 commits into
mainfrom
ci/runner-fixes

Conversation

@dividedmind

@dividedmind dividedmind commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Two fixes:

  • skip chromatic publish if the token is not available (for example with external PRs),
  • replace outdated native smoke test (appmap install) with more current tests (index + query); this also allows dropping the ruby dependency.

@dividedmind
dividedmind requested review from kgilpin and a balanced review from Copilot October 27, 2026 11:43
@dividedmind dividedmind self-assigned this Oct 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Both changes are narrowly scoped, match the stated intent, and have no identified blocking issues.

No open issues 🎉

What changed in this PR

This PR updates CI runner compatibility and prevents Chromatic publishing from failing when credentials are unavailable.

Changes:

  • Update the test job’s Ruby version from 3.1 to 3.4.
  • Skip Chromatic publishing when CHROMATIC_PROJECT_TOKEN is empty, while retaining the build.
File Description
.github/​workflows/​main.yml Updates Ruby setup and adds a token guard to Chromatic publishing.

Review effort: Balanced

Pull requests from forks and Dependabot don't receive repository
secrets, so the publish step failed there. Keep building the
components, but only publish when CHROMATIC_PROJECT_TOKEN is set.
The binary smoke test ran `appmap install` on a scaffolded Ruby
project, which made test_native depend on setup-ruby, rubygems.org
and the appmap gem's own Ruby version allowlist (appmap 1.1.1 rejects
Ruby 3.4, and the runner image no longer has 3.1). `appmap install`
is rarely used now anyway.

Exercise what the binary is used for instead: build the query DB with
`appmap index` and read it back with `appmap query endpoints` and
`appmap query tree`. This also covers the bundled better-sqlite3
native addon, the part of the binary most prone to packaging
breakage. The test project is now just appmap.yml and the fixture
AppMap, so the Ruby setup step goes away.

Assisted-by: Claude:claude-opus-5-5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

Static review found no blocking issues, and the new smoke assertions match the fixture data and CLI contracts.

No open issues 🎉

Review effort: Balanced

@dividedmind
dividedmind merged commit cac2f2b into main Oct 27, 2026
27 checks passed
@dividedmind
dividedmind deleted the ci/runner-fixes branch October 27, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants