Skip to content

feat(boxen): A.0 -- vendor termbox2 v2.5.0, build integration, macOS CI#724

Merged
jsavin merged 3 commits into
developfrom
worktree-boxen-a0-termbox2-vendor-ci
Jun 7, 2026
Merged

feat(boxen): A.0 -- vendor termbox2 v2.5.0, build integration, macOS CI#724
jsavin merged 3 commits into
developfrom
worktree-boxen-a0-termbox2-vendor-ci

Conversation

@jsavin

@jsavin jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

First PR in the boxen Phase A chain. Vendors termbox2 v2.5.0, wires it into the frontier-cli build, and adds macOS CI. No boxen code yet — this PR is build infrastructure.

Locked decisions:

  • termbox2 v2.5.0 at SHA 9b5a5da862c06c554148c14fd38d2f796be22d57, copied in place
  • macOS-only CI for A.0; Linux/Windows deferred until a test environment is set up (frontier-cli currently has Apple-specific build flags). The tb2-smoke target exists to keep cross-platform CI a config change rather than a code change later.

Discovery during implementation

termbox2 v2.5.0 is a single-header library, not separate .h/.c. The plan assumed two upstream files; reality is one. The Frontier-authored termbox2_impl.c defines TB_IMPL and includes termbox2.h as a separate translation unit so the implementation symbols are emitted exactly once. The smoke_main.c does NOT define TB_IMPL — defining it twice would produce 38 duplicate symbol errors at link time. PHASE_A_MILESTONES.md updated to reflect this.

Test plan

  • make -C frontier-cli clean, no new warnings
  • make -C frontier-cli tb2-smoke && ./frontier-cli/tb2-smoketermbox2 vendored OK (tb_utf8_char_length('A') = 1)
  • ./tools/run_headless_tests.sh → 591/591 (baseline preserved)
  • cd tests && make test-integration → 2186 pass / 21 baseline failures (baseline preserved exactly)
  • frontier-cli/boxen/boxen.h placeholder exists with header guard
  • .github/workflows/boxen-ci.yml triggers on this PR (will validate post-push)

Files

File Change
frontier-cli/third_party/termbox2/termbox2.h New (upstream, unmodified, 3519 lines)
frontier-cli/third_party/termbox2/termbox2_impl.c New (Frontier-authored, defines TB_IMPL)
frontier-cli/third_party/termbox2/smoke_main.c New (Frontier-authored, ~20 LOC link smoke)
frontier-cli/third_party/termbox2/LICENSE New (upstream MIT)
frontier-cli/third_party/termbox2/VERSION New (Frontier-authored pin metadata + SHAs)
frontier-cli/boxen/boxen.h New (placeholder header guard)
frontier-cli/boxen/README.md New (points to planning/boxen/)
frontier-cli/Makefile Modified (TB2 block + tb2-smoke target)
.github/workflows/boxen-ci.yml New (macOS-only; first build/test CI in the repo)
third_party/README.md Modified (added termbox2 manifest row)
planning/boxen/PHASE_A_MILESTONES.md Modified (A.0 scope reality note)
.gitignore Modified (added frontier-cli/tb2-smoke)

Net: 3727 insertions / 22 deletions across 12 files. Vast majority is the upstream termbox2.h drop.

Note

This PR introduces the first build/test CI in the repository. Existing .github/workflows/ are claude-bot mention and external monitor only. The new boxen-ci.yml coexists with them.

🤖 Generated with Claude Code /auto

jsavin and others added 2 commits June 6, 2026 15:04
…on, macOS CI

Vendors termbox2 v2.5.0 (single-header library, MIT) into
frontier-cli/third_party/termbox2/ and wires it into the frontier-cli
build. Adds macOS CI workflow and placeholder boxen/ directory.

Key implementation notes:
- termbox2 v2.5.0 is a single-header library (no upstream .c file).
  termbox2_impl.c is the one TU that defines TB_IMPL to activate the
  implementation; all other TUs include termbox2.h for declarations only.
- TB_IMPL is defined inside termbox2_impl.c (not via Makefile flag) because
  frontier-cli uses a single-invocation clang build where per-file Makefile
  CFLAGS overrides are ineffective.
- Zero new warnings under clang -std=c17 -Wall -Wextra on macOS.

tb2-smoke target: compiles termbox2_impl.c + smoke_main.c as a standalone
binary (no frontier-cli deps, no TTY required). Exists so adding
Linux/Windows CI later is a CI-config change, not a code change.

CI is macOS-only for A.0. frontier-cli uses -fpascal-strings and
Mach-O linker flags that are clang/macOS-specific. Linux/Windows CI
deferred until a test environment is available. See PHASE_A_MILESTONES.md
A.0 scope note for the full rationale.

Baselines preserved: 591/591 unit tests pass; integration baseline
(21 pre-existing failures) unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bar-raiser: PASS with 5 P2; security: CLEAN with 1 P1 + 2 P2. All
addressed in this commit per completionist policy.

P1 (security): add permissions: contents: read top-level to
boxen-ci.yml. Defense-in-depth against future steps or third-party
actions attempting writes / comments / OIDC tokens.

P2 (bar-raiser + security overlap): tb2-smoke obj files now land in
frontier-cli/obj/tb2-smoke/ (local, gitignored) instead of /tmp.
Eliminates predictable-path race risk on shared dev machines; CI
runners are unaffected.

P2 (bar-raiser): forward-proof comment on smoke_main.c
tb_utf8_char_length call explaining why this specific symbol proves
TB_IMPL activation -- the definition lives under #ifdef TB_IMPL in
termbox2.h around line 1980. Future maintainers will not weaken the
smoke into a header-availability check.

P2 (bar-raiser): tb2-smoke Makefile recipe gets a comment block
explaining the intentional CFLAGS divergence from $(CFLAGS). Recipe
uses minimal portable flags so the smoke compiles cleanly on
Linux/Windows when those CI jobs land later.

P2 (bar-raiser): VERSION file gets a one-line hint showing how to
verify the SHA256 pins. Future maintainers can drift-check the vendor
drop with a single shasum invocation.

P2 (bar-raiser): boxen-ci.yml paths: filter now applies to push: as
well as pull_request:. A direct push to develop touching unrelated
files no longer triggers the boxen workflow.

P2 (security, informational): noted that termbox2/LICENSE and the
license header inside termbox2.h have different copyright lines
(termbox developers 2021 vs nsf/Adam Saponara). Both are MIT and the
attribution is correct; flagged so future maintainers do not
"normalize" one into the other.

Verified: build clean, tb2-smoke runs, unit 591/591 preserved.
@jsavin

jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

/gate Round-1 Review @ 5579ab620

Verdict: PASS WITH NOTES

Tests

  • unit: 591/591 pass (cached at 5579ab6)
  • integration: 2186 pass, 21 baseline failures preserved (cached at 5579ab6)
  • tb2-smoke: builds + runs, emits expected message

Reviewers

  • bar-raiser: ran (always) — PASS, 5 P2
  • security: ran (always) — CLEAN with 1 P1 + 2 P2
  • concurrency: skipped (auto: vendored upstream termbox2 code is not in-scope for project concurrency review)
  • swiftui: skipped (excluded)

Findings (all addressed in round 2 commit 201cf8c8d per completionist policy)

# Sev File Finding Action
1 P1 .github/workflows/boxen-ci.yml Missing top-level permissions: block FIXED — added permissions: contents: read
2 P2 frontier-cli/Makefile tb2-smoke recipe /tmp/tb2_*.o predictable-path race risk on shared dev machines FIXED — obj files now in frontier-cli/obj/tb2-smoke/
3 P2 frontier-cli/third_party/termbox2/smoke_main.c Future maintainer could weaken tb_utf8_char_length smoke into header-availability check FIXED — added forward-proof comment
4 P2 frontier-cli/Makefile tb2-smoke recipe CFLAGS divergence from $(CFLAGS) not explained FIXED — added rationale comment
5 P2 frontier-cli/third_party/termbox2/VERSION No instructions for SHA256 verification FIXED — added shasum -a 256 hint
6 P2 .github/workflows/boxen-ci.yml paths: filter applies to PR but not push FIXED — applied to push too
info LICENSE attribution Distinct copyrights in LICENSE (termbox developers 2021) vs termbox2.h header (nsf/Adam Saponara) Both correct/MIT; noted for future maintainers

Reviewer praise

bar-raiser:

  • Scope discipline excellent — diff is exactly vendor + build + CI + placeholders + docs
  • termbox2_impl.c comment is exemplary, explains why TB_IMPL is in the C file
  • Smoke test is meaningful: tb_utf8_char_length is declared at h:681 but defined under #ifdef TB_IMPL at h:1980, so the link proves BOTH vendor inclusion AND TB_IMPL activation
  • Per-file CFLAGS correctly isolates termbox2 from -fpascal-strings
  • INCLUDES change is purely additive, no collateral damage to other targets
  • Milestone doc honestly captures the macOS-only scope reduction rather than papering over it

security:

  • Supply-chain pin integrity verified: shasum matches VERSION exactly; URL/tag/SHA/date/pinner/rationale all present
  • No sneak-in: only 5 files in third_party/termbox2/, all ASCII text, no binaries
  • Workflow uses pull_request (not pull_request_target); actions/checkout@v4 major-version pinned; no secrets referenced
  • termbox2 v2.5.0 is the current release from the actively-maintained nsf/Adam Saponara fork; no known public CVEs

Round-2 verification pending.

@jsavin

jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

/gate Round-2 Review @ 201cf8c8d

Verdict: PASS

Tests

  • unit: 591/591 pass (cached at 201cf8c)
  • tb2-smoke: builds + runs in frontier-cli/obj/tb2-smoke/, emits expected message

Reviewer

  • bar-raiser: ran (round-2 verification) — PASS

Findings

All 6 round-1 items (1 P1 + 5 P2) verified fixed:

# Round-1 item Status
1 CI permissions: contents: read block FIXED — read-only, no other scopes set, idiomatic placement
2 tb2-smoke /tmp race risk FIXED — obj files in frontier-cli/obj/tb2-smoke/, mkdir -p present, .gitignore covers frontier-cli/obj/
3 smoke_main.c forward-proof comment FIXED — explicit #ifdef TB_IMPL reference + "don't replace" warning
4 tb2-smoke CFLAGS divergence comment FIXED — calls out -fpascal-strings + Linux/Windows portability rationale
5 VERSION shasum verification hint FIXED — shasum -a 256 termbox2.h LICENSE command added
6 CI paths: filter under push FIXED — push paths now match pull_request paths exactly

Diff scope: exactly 5 files modified, no creep. No new issues introduced.

Branch ready to merge.

CI on Xcode 15.4 caught a pre-existing bug in debug_handler.c:1179: a
label (after_stepping:) followed directly by a variable declaration is
illegal in C until C23. Local newer-clang accepts it as an extension;
CI's older clang rejects it with "expected expression" at the
declaration.

This bug pre-dates boxen A.0 -- it would have surfaced on any CI
configuration with a strict-enough compiler. The new boxen-ci.yml is
the first build CI in the repo and exposed it on the first run.

Fix: add a single ';' after the label to make it a labeled empty
statement. The original code's intent is preserved exactly.

Pulled into A.0's PR rather than filed separately because:
  - A.0 is the PR that introduces CI; the same PR should leave CI
    green
  - The fix is one character, no risk to A.0's scope
  - Blocking A.0 on a separate side-fix would stall the boxen chain
    indefinitely

Verified: build clean, unit 591/591 preserved.
@jsavin jsavin merged commit c58bb89 into develop Jun 7, 2026
1 check passed
@jsavin jsavin deleted the worktree-boxen-a0-termbox2-vendor-ci branch June 7, 2026 01:28
@jsavin

jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

/auto Summary

What went well: The plan agent caught the cross-platform reality up front (frontier-cli is Mac-only today; -fpascal-strings + Mach-O linker flags), proposed the tb2-smoke fork as the pragmatic resolution, and the user-locked decision to defer Linux/Windows CI made the milestone scope honest rather than aspirational. Single-PR shape stayed clean: 12 files, mostly vendor drop. Bar-raiser + security review was unusually thorough on a vendor PR (catching SHA256 verification UX, defense-in-depth on workflow permissions, predictable /tmp paths), and every nit was a real improvement, not bikeshedding.

What could improve: The new CI immediately caught a pre-existing label-before-declaration bug at debug_handler.c:1179 that local clang accepts as an extension. CI's Xcode 15.4 rejected it. The one-character fix landed in this PR rather than blocking on a side-fix. Lesson worth codifying: when adding the first CI to a project, expect ONE pre-existing bug to surface on first run, and budget for the inline fix.

Recommendations: When boxen Phase A continues (A.1 backend interface), pre-flight a quick make clean && make on each PR's branch before pushing — the cold-build reproduces CI behavior more faithfully than incremental local builds.

🤖 Generated with Claude Code /auto

jsavin added a commit that referenced this pull request Jun 7, 2026
Milestone breakdown for Phase B of the boxen plan, mirroring Phase A's
structure. Closes #691 (interactive UserTalk debugger TUI). Consumes
the substrate shipped in PRs #724-#731 and the runtime contract from
PR #722 (thread-debug-attach).

Each milestone is sized as a /auto-shaped PR.

Includes code-verified corrections to the handoff doc: transport_t
write_line takes 3 params (ctx, json, len), not 2 as the handoff doc
states. Verified debug/getSource, debug/getStack, and debug/getLocals
all exist and their wire formats are documented from source.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.

1 participant