Enforce the terminal style system + terminology (CLI) by LukasWodka · Pull Request #368 · tracebloc/cli

LukasWodka · 2026-07-21T09:53:14Z

Summary

The cli half of the style/terminology enforcement (mirrors tracebloc/client#364) — Option 1: a blocking CI gate for the mechanical rules + docs + CODEOWNERS for the judgement calls.

What's added

scripts/check-style.sh — CI gate (blocking, in the Lint job alongside deadcode / file-budget), three mechanical checks:
1. no hardcoded brand colour outside the tone engine (internal/ui) — use the Printer tones
2. no status/traffic-light emoji — the lime ● is the online indicator
3. no workspace in user-facing text → secure environment (matched whole-word, so exitNoWorkspace and other identifiers are exempt; comments + _test.go exempt; opt out per line with // style-guard: allow)
STYLE.md — the reference: cyan = structure, lime = action, the role→tone table (Printer methods), terminology glossary (SoT = docs TERMINOLOGY.md).
CODEOWNERS (new) — the guard, STYLE.md, and the brand-value pin (brand_tones_test.go) are code-owned, so the rules can't be quietly weakened.
PR template (new) — with a STYLE.md checklist line.

Terminology cleanup (so it ships blocking, develop clean)

The guard surfaced 19 user-facing workspace strings → swept to secure environment across submit.go, data*.go, and push/stage.go. Code identifiers (exitNoWorkspace) untouched (whole-word match). Updated the one test assertion that keyed on the old spinner copy.

Test plan

bash scripts/check-style.sh → clean; wired blocking into the Lint job.
go build ./..., go vet, go test (cli/submit/push), gofmt -s, staticcheck (all,-ST1005), deadcode, file-budget — all clean. brand_tones_test.go still pins the exact SGR. Workflow YAML validates.

🤖 Generated with Claude Code

Note

Low Risk
Mechanical CI and documentation plus user-facing string renames; no changes to auth, cluster logic, or data handling behavior.

Overview
Adds a blocking style/terminology gate for CLI output: scripts/check-style.sh (brand hex only in internal/ui, no traffic-light emoji, no user-facing workspace) plus STYLE.md as the reference. The guard runs in the Lint job and make ci; the installer job shellchecks the script; CODEOWNERS protects the guard, STYLE.md, and brand_tones_test.go. A new PR template checklist reminds contributors to run the guard.

User-facing copy is updated so the repo passes the new rule: workspace → secure environment across data ingest/help paths, submit, and push/stage, with one test assertion updated for the submit spinner text.

^{Reviewed by Cursor Bugbot for commit eb15958. Bugbot is set up for automated code reviews on this repo. Configure here.}

The cli half of the enforcement (mirrors client#364). Guards STYLE.md against regressions, layered strongest-first: - scripts/check-style.sh — CI gate (blocking, in the Lint job alongside deadcode/file-budget) with three MECHANICAL checks: no hardcoded brand colour outside the tone engine (internal/ui), no status/traffic-light emoji, and no 'workspace' in user-facing text (matched whole-word, so exitNoWorkspace and other identifiers are exempt; comments + _test.go exempt; opt out per-line with // style-guard: allow). - STYLE.md — the reference: cyan=structure / lime=action, the role→tone table (Printer methods in internal/ui), and the terminology glossary. - CODEOWNERS (new) — the guard, STYLE.md, and the brand-value pin (brand_tones_test.go) are code-owned so the rules can't be quietly weakened. - pull_request_template.md (new) — with a STYLE.md checklist line. Cleaned the terminology debt the guard surfaced so it ships blocking with develop clean: 19 user-facing "workspace" strings → "secure environment" across submit.go, data*.go, and push/stage.go (code identifiers like exitNoWorkspace untouched). Updated the one test assertion that keyed on the old spinner copy. go build/vet/test, gofmt -s, staticcheck, deadcode, file-budget, and the guard all clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T09:53:24Z

@BugBot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit ffc8dc2. Configure here.}

Mirror of the client-guard fix — the hex scan matched only lowercase, so an uppercase #01A5CC would slip past. Add -i to check 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T09:56:27Z

@BugBot run

aptracebloc

Reviewed — good intent (blocking style/terminology gate + STYLE.md + ownership + the 19-string workspace → secure environment sweep), but I'm holding: all three open Bugbot findings are real, and one is a governance regression. Verified each against develop.

🔴 Findings 1 & 2 — .github/CODEOWNERS silently disables the existing root CODEOWNERS (must-fix).
develop already ships a root /CODEOWNERS:

* @tracebloc/maintainers # default owner for the whole repo
/.github/ @tracebloc/maintainers # CI + release infra

GitHub uses only the first CODEOWNERS it finds, and .github/outranks the repo root. So on merge, GitHub will use only the new 3-line .github/CODEOWNERS and silently drop both existing rules — the repo-wide * maintainer default and the /.github/ CI/release-infra protection. That's the opposite of this PR's goal ("rules can't be quietly weakened"): it quietly removes the existing ownership. Fix: fold the three style-file rules into the existing root/CODEOWNERS (and drop .github/CODEOWNERS), or replicate the root rules into the new file so nothing is lost.

🟠 Finding 3 — style gate missing from make ci (should-fix).
check-style.sh is wired into the CI Lint job (build.yml) but is nowhere in the Makefile — not in lint, not in ci. Its siblings deadcode and file-budgetare in make ci, so this is inconsistent: a dev running make ci before pushing won't catch a style violation, only CI will. Fix: add check-style.sh to the Makefile lint/ci target for parity.

Scope note: I focused on the open findings since 1 & 2 block. I did not deep-verify the guard's regex robustness or the whole-word workspace sweep (sparing identifiers like exitNoWorkspace) — worth a pass once the CODEOWNERS fix lands.

Not approving yet on account of findings 1 & 2 (real ownership regression) — both are cheap to fix. Happy to re-review once they land.

— drafted with Claude (Opus 4.8), sent by @aptracebloc

…ke ci` Two Bugbot findings on #368. 1. CODEOWNERS shadows root rules (Medium). The new .github/CODEOWNERS would take precedence over the existing root /CODEOWNERS, silently dropping its default `*` and /.github/ ownership for the rest of the repo. Move the three style rules into the ROOT /CODEOWNERS instead and delete the .github/ file, so the default owner is preserved and the style paths are added on top. 2. Style gate missing from `make ci` (Medium). The Lint job runs check-style.sh but `make ci` didn't, so a local `make ci` could pass while CI's Lint job fails on style/terminology. Add a `check-style` target and include it in `ci` (mirrors deadcode/file-budget); STYLE.md now points at `make check-style`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…tyle The previous commit only captured the .github/CODEOWNERS deletion (a bad git-add pathspec aborted staging the rest). This adds what belongs with it: - root /CODEOWNERS gains the three style-enforcement rules (guard, STYLE.md, brand-tone pin), so the default `*` owner is preserved and the style paths are code-owned on top. - Makefile: `check-style` target added and wired into `ci`. - STYLE.md: points at `make check-style`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T10:10:13Z

@BugBot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 0a9371f. Configure here.}

… shellcheck Mirror of the client-guard hardening (the same three findings apply to this guard's shared design): - fail CLOSED: missing internal/ tree exits 2; grep exit ≥2 → guard_error → exit 2 (no more silent-pass when a scan errors). - anchor the // comment exemption to the file:line: prefix. - shellcheck scripts/check-style.sh (bash) in the Installer-shell job. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T10:15:49Z

@BugBot run

…shell scan) Same subshell bug as the client guard: guard_error set inside a $(scan …) command-substitution never reached the parent, so the fail-closed exit was unreachable. Call scan in the parent shell, set a global `hits` + guard_error there, move display filtering to report()'s argument. Verified a forced grep error now exits 2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T10:25:18Z

@BugBot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9926eae. Configure here.}

…losed) Parity with the client guard — drop 2>/dev/null; rc>=2 fail-closes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

LukasWodka · 2026-07-21T10:34:31Z

@BugBot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit eb15958. Configure here.}

The e2e workflow's PR trigger was label-only (`e2e`), so PRs that break the suites' assertions merge unexercised: #363/#367/#368 reworded the `tracebloc delete` teardown copy the offboard e2e asserts on, no PR run ever executed the suite, and every develop push went red until #378 realigned the assertions post-merge. Add a ~5s dorny/paths-filter probe (`changes`, PR events only) and gate both e2e jobs on it alongside the existing label opt-in. PRs touching the union of the suites' dependency surface — the offboard flow (delete*.go, nodeboot, api, config), the rendering layer (ui — the #367 lesson), the kind-suite seams (cluster, push), the suites themselves, or the build inputs (Makefile, go.mod/sum, cmd) — now run the e2e pre-merge (~2 min, jobs in parallel). Unrelated PRs skip both jobs as before. Push / nightly / dispatch behavior is unchanged: the probe is skipped there, hence the !cancelled() guards — the implicit success() on a skipped need would otherwise skip those runs too. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

LukasWodka · 2026-07-21T14:05:17Z

/fr-pass — verified live: no user-facing "workspace" remains in help/output; "secure environment" reads throughout.

LukasWodka self-assigned this Jul 21, 2026

LukasWodka requested a review from saadqbal July 21, 2026 09:53