Skip to content

ci: add non-blocking sentinel for the #25 OpenTUI chunk regression#47

Merged
ZouR-Ma merged 2 commits into
stepfun-ai:mainfrom
Lion-1209:build-output-assertion
Jul 17, 2026
Merged

ci: add non-blocking sentinel for the #25 OpenTUI chunk regression#47
ZouR-Ma merged 2 commits into
stepfun-ai:mainfrom
Lion-1209:build-output-assertion

Conversation

@Lion-1209

@Lion-1209 Lion-1209 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What

Adds a non-blocking build-output sentinel that warns in CI when the OpenTUI runtime chunk is missing its exports — the exact condition behind #25.

Why

step failed at runtime with OpenTUI runtime did not export createLocalTuiClientApp() (#25) because tsdown/rolldown tree-shook src/runtime/local-tui-app.ts into an empty chunk:

// dist/local-tui-app-<hash>.js — only 3 lines, no exports
import "./local-session-target-*.js";
import "./local-tui-bootstrap-*.js";
import process from "node:process";

This shipped undetected because no existing check exercises the interactive TUI path:

  • setup.ps1 / setup.sh smoke tests only run step --version
  • CI's smoke step runs step --version / step aec status / step vad status / step voice --help — none trigger shouldUseTui (which requires stdin/stdout.isTTY === true)

So a regression that empties the chunk is invisible until a user runs step in a real terminal.

How

scripts/check-build-output.mjs scans dist/ for any local-tui-app*.js chunk and asserts it contains createLocalTuiClientApp and LocalStepCliTuiApp. It is wired into CI as a new step after the Windows build (tsdown output shape is platform-consistent, so one lane covers the regression).

Non-blocking by design (continue-on-error: true, script exits 0 on failure and just logs ):

Tested

Review feedback addressed

Per @ZouR-Ma's review:

  1. Dropped docs/skills/pr-review-decision.md from this PR (was a separate style(docs) commit). File list is now exactly test.yml + the sentinel script + its test.
  2. Rebased onto current main (now includes fix(build): emit OpenTUI runtime chunk and build agent-sdk #28); full three-platform CI will run on this push.

Scope

Single commit, three files (test.yml, scripts/check-build-output.mjs, scripts/check-build-output.test.ts). No runtime changes.

Related

@Lion-1209
Lion-1209 force-pushed the build-output-assertion branch from 1488bc8 to dc705a3 Compare June 16, 2026 01:07
@ZouR-Ma

ZouR-Ma commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the contribution!

To get this ready to merge:

  • Reference an issue in the description with Closes #<number> (our link-check CI requires it; open one first if needed).
  • Make sure pnpm check passes locally (lint, type-check, dependency/dead-code guards, formatting, and tests — also enforced in CI).

Full conventions:
https://github.com/stepfun-ai/Step-Realtime-CLI/blob/main/CONTRIBUTING.md

@Lion-1209

Lion-1209 commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

I've updated the PR description to include Refs #25 as required by the link-check CI.

I've also verified that pnpm check passes locally:

  • All tests pass (1155 passed, 11 skipped)
  • Lint passes (only 8 existing warnings in other files, not related to this PR)
  • Dependency guardrails passed
  • Deadcode check passed
  • TypeScript type-check passed
  • Formatting check passed

The PR is ready for review.

@li-xiu-qi

Copy link
Copy Markdown
Contributor

Hi @Lion-1209, the CI sentinel is a nice preventive fix. The repo's CONTRIBUTING.md requires every PR to reference an issue (CI link-check enforces this). This PR mentions #25 in the description but doesn't formally close or reference it with . Could you add that so the PR passes the link-check CI gate?

@Lion-1209

Copy link
Copy Markdown
Contributor Author

Hi @Lion-1209, the CI sentinel is a nice preventive fix. The repo's CONTRIBUTING.md requires every PR to reference an issue (CI link-check enforces this). This PR mentions #25 in the description but doesn't formally close or reference it with . Could you add that so the PR passes the link-check CI gate?

Hi @li-xiu-qi, it's there — Refs #25 at the bottom of the description (## Related). CI's already green on this PR.
I went with Refs instead of Closes on purpose. This PR is just the sentinel; #28 is the actual fix for #25. If I put Closes #25 here, merging it would auto-close the issue before #28 even lands. Refs keeps the link-check happy without closing #25 early.

@ZouR-Ma

ZouR-Ma commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this PR — three design calls in this sentinel are exactly right: gating it to the Windows lane only (which already builds, and tsdown output shape is platform-consistent, so no wasted compute); non-blocking by default with an --enforce escape hatch (letting detection land independently of the fix in #28); and shipping 97 lines of tests for the CI script itself. The test.yml comment that records why it's shaped this way and when to flip the hard gate is the pattern we'd like to see more of. Two things before this lands:

  1. (Must) Take docs/skills/pr-review-decision.md out of this PR. Those 36/19 lines are unrelated to the sentinel; the identical change also appears in #41, which is being closed; and it preserves the characters mangled by the earlier Prettier pass (replace*all and friends), which the one-line fix riding in #24 is correcting — the two would fight. Docs formatting will get its own chore PR, so it doesn't need to ride here. Acceptance criterion: the file list shrinks to test.yml plus the sentinel script and its test.
  2. (Must) Get the three-platform test matrix green on the latest commit. The current head only has a link-check record; the push that drops the docs file will trigger full CI. Acceptance criteria: all three lanes green, with the sentinel step visible in the Windows lane log (a warning before #28 lands is expected — leave it).

On sequencing, you're near the front of the queue: this PR goes in right after #28 lands, so these two items are basically all that stands between here and merge. After that, please come back and flip the sentinel to a hard gate (--enforce, drop continue-on-error) — worth opening a tracking issue for that now. Ping us after you push.

@Lion-1209
Lion-1209 force-pushed the build-output-assertion branch from dc705a3 to 8388de3 Compare July 13, 2026 02:09
@github-actions github-actions Bot added the area/build scripts, .github, build/config files label Jul 13, 2026
@Lion-1209

Copy link
Copy Markdown
Contributor Author

Hi @ZouR-Ma, both items done.

1. Dropped the docs commit. File list is now just test.yml + the sentinel script + its test — one commit, three files.

2. Rebased onto current main (which now includes #28) and force-pushed. pnpm check passes locally, and with #28 in, the sentinel now reports ✓ build output check passed instead of warning — verified the chunk contains both exports.

One heads-up on the three-platform CI: the test matrix run for this push is in action_required state, which I believe is the first-time-contributor workflow approval. If a maintainer approves the run on the Actions tab, the three-platform tests will kick off.

Also opened #100 to track flipping the sentinel to a hard gate (--enforce) after this lands. Will come back to it once merged.

Lion-1209 and others added 2 commits July 17, 2026 11:48
…ression

Neither the installer smoke test (`step --version`) nor CI exercise the
interactive TUI path, so when tsdown/rolldown tree-shook the
`local-tui-app` dynamic-import target into an empty chunk (stepfun-ai#25), it shipped
undetected — `step` failed at runtime with "OpenTUI runtime did not export
createLocalTuiClientApp()".

Add a build-output sentinel that checks the built `local-tui-app` chunk for
the required exports and warns when they're missing. It is non-blocking
(`continue-on-error`) on purpose so it can land independently of stepfun-ai#28: on
current `main` it warns (expected, since stepfun-ai#25 is unfixed); once stepfun-ai#28 merges the
warning clears automatically. Flipping to `--enforce` (and dropping
`continue-on-error`) turns it into a hard gate for the future.

Runs on the Windows CI lane, which already produces a build; tsdown output
shape is platform-consistent, so one lane covers the build regression.

Complements stepfun-ai#28. See stepfun-ai#25.
stepfun-ai#28 (the build fix that makes the OpenTUI runtime chunk carry its exports)
has landed on main, so the sentinel no longer needs to warn-only. Drop
continue-on-error and pass --enforce so a missing/empty chunk fails the
Windows lane. Rebased onto current main so the chunk is populated.
@ZouR-Ma
ZouR-Ma force-pushed the build-output-assertion branch from 8388de3 to 3d8e49f Compare July 17, 2026 04:48
@ZouR-Ma
ZouR-Ma merged commit 0e4bfed into stepfun-ai:main Jul 17, 2026
5 checks passed
@Lion-1209
Lion-1209 deleted the build-output-assertion branch July 17, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build scripts, .github, build/config files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants