From ffc8dc2606ea5d48ac8fc4b6cd2d7ae95e10c64b Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 11:52:49 +0200 Subject: [PATCH 1/7] Enforce the terminal style system + terminology (CLI) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/CODEOWNERS | 7 +++ .github/pull_request_template.md | 21 +++++++++ .github/workflows/build.yml | 7 +++ STYLE.md | 69 +++++++++++++++++++++++++++++ internal/cli/data.go | 14 +++--- internal/cli/data_ingest_cluster.go | 12 ++--- internal/cli/data_ingest_cmd.go | 4 +- internal/cli/data_ingest_local.go | 4 +- internal/cli/ingestion_run_test.go | 2 +- internal/push/stage.go | 2 +- internal/submit/submit.go | 6 +-- scripts/check-style.sh | 65 +++++++++++++++++++++++++++ 12 files changed, 191 insertions(+), 22 deletions(-) create mode 100644 .github/CODEOWNERS create mode 100644 .github/pull_request_template.md create mode 100644 STYLE.md create mode 100755 scripts/check-style.sh diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..d5d774f2 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,7 @@ +# Narrow CODEOWNERS — protects the style-system definition so the enforcement +# rules and the pinned brand values can't be weakened without review. Author +# picks reviewers for everything else. + +/scripts/check-style.sh @saadqbal +/STYLE.md @saadqbal +/internal/ui/brand_tones_test.go @saadqbal diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..97c7f3c2 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,21 @@ +## Summary + + +## Related + + +## Type of change +- [ ] Feature +- [ ] Bug fix +- [ ] Tech-debt / refactor +- [ ] Docs +- [ ] Security / hardening + +## Test plan + + +## Checklist +- [ ] Tests added / updated and passing locally +- [ ] `go build ./...`, `go vet`, and the Lint job's checks pass locally +- [ ] Terminal output follows [STYLE.md](../STYLE.md) — Printer tones (no hardcoded colour/emoji), "secure environment" not "workspace"; `bash scripts/check-style.sh` passes +- [ ] No secrets / credentials in the diff diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c320ccae..c19f3dab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -180,6 +180,13 @@ jobs: # 1500-line data.go from growing quietly (backend#1106 WS-B). run: ./scripts/file-budget.sh + - name: Style + terminology guard + # Enforces the terminal style system (STYLE.md): no hardcoded brand colour + # outside the tone engine (internal/ui), no status/traffic-light emoji, and + # "secure environment" not "workspace" in user-facing text. Mechanical + # checks only — role/wording judgement stays with review. + run: bash scripts/check-style.sh + govulncheck: timeout-minutes: 10 name: govulncheck diff --git a/STYLE.md b/STYLE.md new file mode 100644 index 00000000..ed554e9c --- /dev/null +++ b/STYLE.md @@ -0,0 +1,69 @@ +# Terminal output style + +The `tracebloc` CLI and the installer share **one** terminal style system. This +is the reference; `scripts/check-style.sh` enforces the mechanical parts in CI. + +## The idea + +From the tracebloc.io homepage gradient — **cyan orients, lime moves**: + +- **cyan** `#01a5cc` = *structure* — headings, step titles, links, "where you are" +- **lime** `#91e947` = *action* — commands, the primary CTA, "what to do next" + +Everything else stays quiet (dim neutral). Colour is never load-bearing: headings +and commands also carry **bold**, and alerts carry a distinct **glyph**, so the +output still reads under `NO_COLOR`, in a pipe, or for a colour-blind reader. + +## Roles → tones + +All colour goes through the `Printer` in `internal/ui` — never inline an escape +or hex elsewhere. The tone table (`internal/ui/ui.go`) maps each role: + +| Role | Printer method / tone | Colour | Weight / glyph | +|------|-----------------------|--------|----------------| +| Heading / section / step | `Section`, `Step`, `Banner` (`toneHeading`) | cyan `#01a5cc` | bold | +| Command (to run) | `Command`, `MenuRow` cmd (`toneCommand`) | lime `#91e947` | bold | +| Description / supporting | `MenuRow` desc (`toneDesc`) | soft lime `#a7ed6c` | — | +| Link / URL | `toneLink` | cyan `#01a5cc` | underline | +| Success ✔ / online ● | `Successf`, `CheckLine` (`toneGo`) | lime `#91e947` | glyph | +| Warning ⚠ | `Warnf`, `WarnLine` (`toneWarn`) | amber `#ffc62b` | glyph | +| Error ✖ | `Errorf` (`toneErr`) | red `#f64c4c` | bold glyph | +| Label : value | `Field`, `Stat` (`toneLabel`) | dim neutral | — | + +**No emoji.** The lime `●` is the online indicator (not 🟢). + +The engine renders exact 24-bit hex on truecolor terminals, the **deep shade** +(`#01637a` / `#578c2b`) on light backgrounds, the nearest ANSI-16 otherwise, and +nothing when colour is off (`NO_COLOR` / non-TTY / `TERM=dumb` / `--plain`). The +exact brand SGR is pinned by `internal/ui/brand_tones_test.go`, so a drift in the +tone table fails CI. The installer mirrors this in `scripts/lib/common.sh`. + +## Terminology + +Source of truth: the docs repo `TERMINOLOGY.md`. In user-facing output: + +| Use | Not | +|-----|-----| +| secure environment | workspace, hub, client (as a noun for the environment) | +| ingest | upload, import | +| delete | remove, uninstall (for offboarding) | +| Online / Offline | connected / disconnected, up / down | +| collaborators | users, members | +| task | job, experiment type | + +`client` stays valid as the CLI verb (`tracebloc client create`) and in code +identifiers (`exitNoWorkspace`, etc.) — the guard matches `workspace` as a whole +word in output text only. + +## What's enforced vs reviewed + +`scripts/check-style.sh` (CI Lint job, blocking) catches the **mechanical** +violations: hardcoded brand colour outside `internal/ui`, status emoji, and +`workspace` in user-facing text. Run it locally with `bash scripts/check-style.sh`. + +It can't police **judgement** — using the right *role* for a token (a command in +the command tone, not the heading tone), or the softer terminology calls. Those +stay with review; `STYLE.md` and `scripts/check-style.sh` are CODEOWNER-gated so +the rules can't be quietly weakened. + +To intentionally exempt a line, append `// style-guard: allow` with a reason. diff --git a/internal/cli/data.go b/internal/cli/data.go index 4a2e36a8..98421b86 100644 --- a/internal/cli/data.go +++ b/internal/cli/data.go @@ -25,11 +25,11 @@ func newDataCmd() *cobra.Command { cmd := &cobra.Command{ Use: "data", Aliases: []string{"dataset"}, - Short: "Manage the datasets in your workspace", - Long: `Commands for ingesting and managing the datasets your workspace holds — + Short: "Manage the datasets in your secure environment", + Long: `Commands for ingesting and managing the datasets your secure environment holds — the data models train on. It stays on your infrastructure. -` + "`data ingest`" + ` ingests a local dataset into your workspace's storage, +` + "`data ingest`" + ` ingests a local dataset into your secure environment's storage, submits the ingestion run, and watches it to completion (streaming logs + the final summary). ` + "`data validate`" + ` checks an ingest.yaml locally first. @@ -147,8 +147,8 @@ func runDataIngest(ctx context.Context, out, errOut io.Writer, a runDataIngestAr // live-only steps (stage + ingest) the customer just skipped. if a.DryRun { a.Printer.Newline() - a.Printer.Successf("Dry-run complete — your data and workspace check out; nothing was created.") - a.Printer.Hintf("A real run continues with step 2 (copy into your workspace) and step 3 (validate and load).") + a.Printer.Successf("Dry-run complete — your data and secure environment check out; nothing was created.") + a.Printer.Hintf("A real run continues with step 2 (copy into your secure environment) and step 3 (validate and load).") if a.OutputJSON { writePushJSON(a.JSONOut, "dry-run", spec, nil, "", "") jsonEmitted = true @@ -196,8 +196,8 @@ func runDataIngest(ctx context.Context, out, errOut io.Writer, a runDataIngestAr // Exit code 7 ("staging failed") is distinct from the // pre-flight codes so customers can branch on whether the // failure was their environment vs the actual data transfer. - a.Printer.Step(2, 3, "Copy into your workspace") - a.Printer.Hintf("Your files are copied securely into your workspace's storage — set up and cleaned up for you.") + a.Printer.Step(2, 3, "Copy into your secure environment") + a.Printer.Hintf("Your files are copied securely into your secure environment's storage — set up and cleaned up for you.") progress := push.NewProgress(out, layout.TotalBytes, fmt.Sprintf("Copying %s", a.Spec.Table)) // Defer Finish so a failure path that returns BEFORE diff --git a/internal/cli/data_ingest_cluster.go b/internal/cli/data_ingest_cluster.go index d573c798..48a96649 100644 --- a/internal/cli/data_ingest_cluster.go +++ b/internal/cli/data_ingest_cluster.go @@ -37,7 +37,7 @@ func connectIngestTarget(ctx context.Context, a *runDataIngestArgs) (target *clu // Errors mirror that command's exit-code contract (3 for // kubeconfig, 4 for missing release) so behaviour is // consistent across pre-flight commands. - // Connecting to the workspace + discovering its shared storage is + // Connecting to the secure environment + discovering its shared storage is // Kubernetes plumbing (release / PVC / jobs-manager) the happy path keeps // quiet — it's no longer a numbered step (RFC-0002 §6), and --verbose adds // the release/PVC detail below. But the discovery itself is several blocking @@ -48,7 +48,7 @@ func connectIngestTarget(ctx context.Context, a *runDataIngestArgs) (target *clu // namespace-fallback note mid-call, and a spinner's \r redraw would clobber // it. ALL the logic below (discovery + the exit-6 destination guard) is // unchanged; only the presentation moved. - a.Printer.Infof("Connecting to your workspace…") + a.Printer.Infof("Connecting to your secure environment…") // 6. PVC discovery (needPVC) confirms the chart's shared-data PVC is // Bound before we waste time provisioning a Pod that can't mount it. opts := cluster.KubeconfigOptions{Path: a.Kubeconfig, Context: a.Context, Namespace: a.Namespace} @@ -127,7 +127,7 @@ func runIngestionRun(ctx context.Context, out io.Writer, a runDataIngestArgs, ta // The chart's helm flow uses the same token-mint code path. a.Printer.Step(3, 3, "Validate and load") if a.Detach { - a.Printer.Hintf("Submitting the run — with --detach it keeps running on your workspace after this command returns; the reconnect command is shown below.") + a.Printer.Hintf("Submitting the run — with --detach it keeps running on your secure environment after this command returns; the reconnect command is shown below.") } else { a.Printer.Hintf("Submitting the run, then following along as tracebloc validates your data and loads it into the table — progress streams below.") a.Printer.Hintf("This follows the run for up to an hour; a longer run keeps going on its own (or start it with --detach and check back later).") @@ -150,7 +150,7 @@ func runIngestionRun(ctx context.Context, out io.Writer, a runDataIngestArgs, ta // silent (RFC-0002 "progress on every wait"). The submit POST itself is a // separate ~30s synchronous wait; its spinner lives in submit.Run, next to // the POST it covers. - connectSpin := a.Printer.Spinner("Connecting to your workspace to submit the run", "") + connectSpin := a.Printer.Spinner("Connecting to your secure environment to submit the run", "") pf, err := portForwardJobsManagerFn(ctx, cs, resolved.RestConfig, resolved.Namespace, release.JobsManagerServiceName, release.JobsManagerPort) connectSpin.Stop() @@ -258,7 +258,7 @@ func shouldReclaimStaging(status string) bool { return status == "succeeded" } -// printClusterSummary shows the discovered workspace target. It's Kubernetes +// printClusterSummary shows the discovered secure environment target. It's Kubernetes // plumbing (release / jobs-manager / shared PVC) the happy path hides, so the // whole block — header, fields, and the RWO-PVC note — prints only under // --verbose (RFC-0002 §6). Discovery + guards are unchanged; this is @@ -349,7 +349,7 @@ func existingTableAction(a *runDataIngestArgs, existingTable string) (proceed bo return ok, nil } return false, &exitError{code: exitTableExists, err: fmt.Errorf( - "table %q already exists in this workspace. Re-ingesting the same table doesn't merge or replace — "+ + "table %q already exists in this secure environment. Re-ingesting the same table doesn't merge or replace — "+ "the run would fail after uploading everything. Re-run with --overwrite to replace it, "+ "or pick a different --name. (`tracebloc data delete %s` also removes it.)", existingTable, existingTable)} diff --git a/internal/cli/data_ingest_cmd.go b/internal/cli/data_ingest_cmd.go index c2fe02ee..f4292afd 100644 --- a/internal/cli/data_ingest_cmd.go +++ b/internal/cli/data_ingest_cmd.go @@ -93,14 +93,14 @@ func newDataIngestCmd() *cobra.Command { cmd := &cobra.Command{ Use: "ingest ", Aliases: []string{"push"}, - Short: "Ingest a local dataset into your workspace", + Short: "Ingest a local dataset into your secure environment", // The task COUNT and the text-family subdir names are derived from the // registry / vendored layout contract (push.SupportedCategoryIDs + // push.TextSidecarDir) rather than hardcoded, so the help can't drift // from what the CLI actually supports (cli#215): the count used to read // a stale "9", and the text example showed texts/ for every text task // even though masked_language_modeling stages into sequences/. - Long: fmt.Sprintf(`Ingests a local dataset into your workspace's storage, + Long: fmt.Sprintf(`Ingests a local dataset into your secure environment's storage, submits the ingestion run, and follows it to completion (streaming progress + the final summary). Your data never leaves your own infrastructure. Supports %[1]d tasks across the image, text, and diff --git a/internal/cli/data_ingest_local.go b/internal/cli/data_ingest_local.go index 6f105a4a..b2447f81 100644 --- a/internal/cli/data_ingest_local.go +++ b/internal/cli/data_ingest_local.go @@ -95,7 +95,7 @@ func resolveLocalInput(out, errOut io.Writer, a *runDataIngestArgs) (layout *pus a.Printer.Banner("tracebloc", "data ingest") a.Printer.Para(strings.TrimSpace(` This ingests a dataset so models can train on it. Your files never leave your -own infrastructure — tracebloc copies them into your workspace's storage, +own infrastructure — tracebloc copies them into your secure environment's storage, checks them, and loads them into a table your training runs read from. Other collaborators can train against that table without ever seeing the raw files.`)) a.Printer.Hintf("Learn more: https://docs.tracebloc.io") @@ -285,7 +285,7 @@ collaborators can train against that table without ever seeing the raw files.`)) } a.Printer.Step(1, 3, "Check your data") - a.Printer.Hintf("Reading your files locally first — nothing has touched your workspace yet — so a layout or settings problem shows up right away.") + a.Printer.Hintf("Reading your files locally first — nothing has touched your secure environment yet — so a layout or settings problem shows up right away.") // 3a. Per-category spec resolution from the local data, so the // synthesized spec carries the right fields before validation. diff --git a/internal/cli/ingestion_run_test.go b/internal/cli/ingestion_run_test.go index 2cb33b41..5f6a5a00 100644 --- a/internal/cli/ingestion_run_test.go +++ b/internal/cli/ingestion_run_test.go @@ -204,7 +204,7 @@ func TestRunIngestionRun_SubmitConnectUsesSpinner(t *testing.T) { t.Fatalf("runIngestionRun: %v", err) } out := buf.String() - if !strings.Contains(out, "Connecting to your workspace to submit the run") { + if !strings.Contains(out, "Connecting to your secure environment to submit the run") { t.Errorf("submit-connect wait is missing its status message:\n%q", out) } // A spinner redraws/clears its line with a carriage return; a plain Infof diff --git a/internal/push/stage.go b/internal/push/stage.go index e25d24c2..2fa6615a 100644 --- a/internal/push/stage.go +++ b/internal/push/stage.go @@ -106,7 +106,7 @@ func Stage(ctx context.Context, opts StageOptions) error { if err != nil { return err } - _, _ = fmt.Fprintf(opts.Out, "Opened a secure channel to your workspace's storage.\n") + _, _ = fmt.Fprintf(opts.Out, "Opened a secure channel to your secure environment's storage.\n") // 3. Defer cleanup. The deferred call uses a FRESH context with // its own deadline — if the parent ctx is cancelled (SIGINT, diff --git a/internal/submit/submit.go b/internal/submit/submit.go index 0fdfaa55..b13b29ac 100644 --- a/internal/submit/submit.go +++ b/internal/submit/submit.go @@ -147,7 +147,7 @@ func Run(ctx context.Context, opts Options) (*Result, error) { // --detach: report and bail. The run continues in the cluster; // there is no CLI re-attach verb yet, so the honest way back is // the raw log follow, offered as a labelled command. - p.Infof("Detached — the ingestion runs in the background on your workspace.") + p.Infof("Detached — the ingestion runs in the background on your secure environment.") p.Hintf("Follow it later with: kubectl logs -f -n %s job/%s", resp.Namespace, resp.JobName) return &Result{Submit: resp}, nil } @@ -175,14 +175,14 @@ func Run(ctx context.Context, opts Options) (*Result, error) { p.Newline() switch wr.DetachReason { case DetachReasonSignal: - p.Infof("Stopped watching — the ingestion keeps running on your workspace.") + p.Infof("Stopped watching — the ingestion keeps running on your secure environment.") case DetachReasonPodWaitTimeout: p.Infof("The ingestion hasn't started yet (usually a slow image pull or a busy cluster). " + "It's queued to run once the cluster can schedule it — check on it with the command below.") case DetachReasonWatchCap: p.Infof("Stopped following after 1 hour — the ingestion is still running and will finish on its own.") default: - p.Infof("Stopped watching — the ingestion keeps running on your workspace.") + p.Infof("Stopped watching — the ingestion keeps running on your secure environment.") } p.Hintf("Check on it later with: kubectl logs -f -n %s job/%s", resp.Namespace, resp.JobName) return &Result{Submit: resp, Watch: wr}, nil diff --git a/scripts/check-style.sh b/scripts/check-style.sh new file mode 100755 index 00000000..4cc95cba --- /dev/null +++ b/scripts/check-style.sh @@ -0,0 +1,65 @@ +#!/usr/bin/env bash +# ============================================================================= +# check-style.sh — enforce the tracebloc terminal style system + terminology +# on the CLI's Go output code. See STYLE.md. +# +# Runs in CI (the lint job, alongside deadcode-check.sh / file-budget.sh) and +# locally: bash scripts/check-style.sh +# Exit 0 = clean, 1 = violations found, 2 = usage error. +# +# Three mechanical checks (semantic calls — role misuse, judgement-y wording — +# stay with CODEOWNERS review + STYLE.md; a grep can't police those): +# 1. No hardcoded brand colour outside the colour engine (internal/ui). New +# output must go through the Printer tones, never a re-hardcoded hex/RGB. +# 2. No status / traffic-light emoji — the lime dot is the online indicator. +# 3. No 'workspace' in user-facing text — the term is "secure environment". +# Matched as a whole word, so the exitNoWorkspace code identifier is exempt; +# comments and _test.go files are exempt too. +# +# A line may opt out of ANY check with a trailing // style-guard: allow marker. +# ============================================================================= +set -uo pipefail +cd "$(dirname "$0")/.." || exit 2 + +ENGINE='internal/ui/' # the one place raw brand colour legitimately lives +fail=0 + +# scan REGEX [EXTRA_FLAGS] — grep the Go output surface, dropping tests, this +# guard, and any line that opted out. Prints file:line:text. +scan() { + local re="$1" flags="${2:-}" + # shellcheck disable=SC2086 + grep -rnE $flags --include='*.go' "$re" internal/ 2>/dev/null \ + | grep -vE '_test\.go:' \ + | grep -vE '// *style-guard: *allow' +} + +report() { # TITLE MATCHES + [[ -z "$2" ]] && return 0 + printf '\n [x] %s\n' "$1" + printf '%s\n' "$2" | sed 's/^/ /' + fail=1 +} + +echo "== tracebloc style guard ==" + +# 1) Hardcoded brand colour outside the engine (internal/ui). +brand='#?(01a5cc|91e947|a7ed6c|01637a|578c2b|34b7d6)|38;2;(1;165;204|145;233;71|167;237;108|1;99;122|87;140;43)' +report "hardcoded brand colour — use the Printer tones in ${ENGINE}, don't re-hardcode hex/RGB" \ + "$(scan "$brand" | grep -vE "^${ENGINE}")" + +# 2) Status / traffic-light emoji. Pattern built from bytes so this source stays +# emoji-free (green/red/yellow/orange circles). +emoji="$(printf '\360\237\237\242|\360\237\224\264|\360\237\237\241|\360\237\237\240')" +report "status emoji — use the lime online dot (see STYLE.md), not traffic-light emoji" \ + "$(scan "$emoji")" + +# 3) Banned terminology in user-facing text: 'workspace' -> 'secure environment'. +# -w matches whole words only (so exitNoWorkspace is exempt); skip comments. +report "banned term 'workspace' in user-facing text — use 'secure environment' (see STYLE.md)" \ + "$(scan 'workspace' '-iw' | grep -vE ':[0-9]+:[[:space:]]*//')" + +if [[ "$fail" -eq 0 ]]; then + echo " ok: style + terminology clean" +fi +exit "$fail" From bac1ef9f50e3473e72b865526ca1e6dc459eca37 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 11:56:11 +0200 Subject: [PATCH 2/7] Address Bugbot (parity): make the brand-colour check case-insensitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- scripts/check-style.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-style.sh b/scripts/check-style.sh index 4cc95cba..687013c3 100755 --- a/scripts/check-style.sh +++ b/scripts/check-style.sh @@ -46,7 +46,7 @@ echo "== tracebloc style guard ==" # 1) Hardcoded brand colour outside the engine (internal/ui). brand='#?(01a5cc|91e947|a7ed6c|01637a|578c2b|34b7d6)|38;2;(1;165;204|145;233;71|167;237;108|1;99;122|87;140;43)' report "hardcoded brand colour — use the Printer tones in ${ENGINE}, don't re-hardcode hex/RGB" \ - "$(scan "$brand" | grep -vE "^${ENGINE}")" + "$(scan "$brand" '-i' | grep -vE "^${ENGINE}")" # 2) Status / traffic-light emoji. Pattern built from bytes so this source stays # emoji-free (green/red/yellow/orange circles). From 2720a80fdd9debc98ec89de6ed40a4ae88a33187 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 12:08:01 +0200 Subject: [PATCH 3/7] Address Bugbot: don't shadow root CODEOWNERS; wire the guard into `make 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 --- .github/CODEOWNERS | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .github/CODEOWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS deleted file mode 100644 index d5d774f2..00000000 --- a/.github/CODEOWNERS +++ /dev/null @@ -1,7 +0,0 @@ -# Narrow CODEOWNERS — protects the style-system definition so the enforcement -# rules and the pinned brand values can't be weakened without review. Author -# picks reviewers for everything else. - -/scripts/check-style.sh @saadqbal -/STYLE.md @saadqbal -/internal/ui/brand_tones_test.go @saadqbal From 0a9371f31e76d9c11a1a1a7c769719a0e080a270 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 12:09:25 +0200 Subject: [PATCH 4/7] Address Bugbot (cont.): root CODEOWNERS style rules + make ci check-style 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 --- CODEOWNERS | 7 +++++++ Makefile | 10 +++++++++- STYLE.md | 3 ++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 928cd771..12540735 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -6,3 +6,10 @@ # CI + release infrastructure. Changes here can break the release # pipeline; route them to platform reviewers explicitly. /.github/ @tracebloc/maintainers + +# Style-system enforcement (STYLE.md). The guard, the style reference, and the +# pinned brand values decide what output regressions CI blocks; a change here +# changes the rules themselves, so it must be code-owner reviewed. +/scripts/check-style.sh @tracebloc/maintainers +/STYLE.md @tracebloc/maintainers +/internal/ui/brand_tones_test.go @tracebloc/maintainers diff --git a/Makefile b/Makefile index 8d5adb45..54e93dc6 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ GOIMPORTS_VERSION ?= v0.48.0 # ---- top-level targets ------------------------------------------- .PHONY: ci -ci: vet test lint fmt-check schema-check vulncheck file-budget deadcode +ci: vet test lint fmt-check schema-check vulncheck file-budget deadcode check-style @echo "==> ci: all green" .PHONY: build @@ -176,6 +176,14 @@ schema-sync: file-budget: ./scripts/file-budget.sh +# check-style: enforce the terminal style system + terminology (STYLE.md) — +# no hardcoded brand colour outside internal/ui, no status emoji, "secure +# environment" not "workspace". Mirrors the Lint job's guard so `make ci` +# matches CI. Mechanical only; role/wording judgement stays with review. +.PHONY: check-style +check-style: + ./scripts/check-style.sh + # ---- cleanup ----------------------------------------------------- .PHONY: clean diff --git a/STYLE.md b/STYLE.md index ed554e9c..19069087 100644 --- a/STYLE.md +++ b/STYLE.md @@ -59,7 +59,8 @@ word in output text only. `scripts/check-style.sh` (CI Lint job, blocking) catches the **mechanical** violations: hardcoded brand colour outside `internal/ui`, status emoji, and -`workspace` in user-facing text. Run it locally with `bash scripts/check-style.sh`. +`workspace` in user-facing text. Run it locally with `make check-style` (also part +of `make ci`) or directly: `bash scripts/check-style.sh`. It can't police **judgement** — using the right *role* for a token (a command in the command tone, not the heading tone), or the softer terminology calls. Those From 4371c61c087f765af34add7ff970260dcc36418c Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 12:15:30 +0200 Subject: [PATCH 5/7] Address Bugbot (parity): guard fails closed, anchored comment filter, shellcheck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/build.yml | 1 + scripts/check-style.sh | 38 +++++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c19f3dab..379933c1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -51,6 +51,7 @@ jobs: run: | sudo apt-get update -qq && sudo apt-get install -y -qq shellcheck dash shellcheck --shell=sh --severity=error scripts/install.sh + shellcheck --shell=bash --severity=error scripts/check-style.sh dash -n scripts/install.sh bash -n scripts/tests/install-verify.sh - name: Verification harness (mandatory cosign / fail-closed) diff --git a/scripts/check-style.sh b/scripts/check-style.sh index 687013c3..e3d8f48f 100755 --- a/scripts/check-style.sh +++ b/scripts/check-style.sh @@ -3,9 +3,9 @@ # check-style.sh — enforce the tracebloc terminal style system + terminology # on the CLI's Go output code. See STYLE.md. # -# Runs in CI (the lint job, alongside deadcode-check.sh / file-budget.sh) and -# locally: bash scripts/check-style.sh -# Exit 0 = clean, 1 = violations found, 2 = usage error. +# Runs in CI (the Lint job, alongside deadcode-check.sh / file-budget.sh) and +# locally: make check-style (or: bash scripts/check-style.sh) +# Exit 0 = clean, 1 = violations found, 2 = the guard itself errored (fail-closed). # # Three mechanical checks (semantic calls — role misuse, judgement-y wording — # stay with CODEOWNERS review + STYLE.md; a grep can't police those): @@ -21,17 +21,30 @@ set -uo pipefail cd "$(dirname "$0")/.." || exit 2 +# Fail CLOSED: a mis-run guard (wrong dir, missing tree) must never look like a +# pass — that would let regressions through the blocking gate silently. +[[ -d internal ]] || { echo "check-style: internal/ not found — refusing to report clean" >&2; exit 2; } + ENGINE='internal/ui/' # the one place raw brand colour legitimately lives fail=0 +guard_error=0 # scan REGEX [EXTRA_FLAGS] — grep the Go output surface, dropping tests, this -# guard, and any line that opted out. Prints file:line:text. +# guard, and any line that opted out. Prints file:line:text. grep exit 2+ (bad +# regex/flags/tree) is an INTERNAL error → fail closed, never silent-pass. scan() { - local re="$1" flags="${2:-}" + local re="$1" flags="${2:-}" out rc # shellcheck disable=SC2086 - grep -rnE $flags --include='*.go' "$re" internal/ 2>/dev/null \ + out="$(grep -rnE $flags --include='*.go' "$re" internal/ 2>/dev/null)" + rc=$? + if [[ "$rc" -ge 2 ]]; then + echo "check-style: grep errored (rc=$rc) on /$re/ — failing closed" >&2 + guard_error=1 + return 0 + fi + printf '%s' "$out" \ | grep -vE '_test\.go:' \ - | grep -vE '// *style-guard: *allow' + | grep -vE '// *style-guard: *allow' || true } report() { # TITLE MATCHES @@ -43,7 +56,7 @@ report() { # TITLE MATCHES echo "== tracebloc style guard ==" -# 1) Hardcoded brand colour outside the engine (internal/ui). +# 1) Hardcoded brand colour outside the engine (case-insensitive: #01A5CC too). brand='#?(01a5cc|91e947|a7ed6c|01637a|578c2b|34b7d6)|38;2;(1;165;204|145;233;71|167;237;108|1;99;122|87;140;43)' report "hardcoded brand colour — use the Printer tones in ${ENGINE}, don't re-hardcode hex/RGB" \ "$(scan "$brand" '-i' | grep -vE "^${ENGINE}")" @@ -55,10 +68,15 @@ report "status emoji — use the lime online dot (see STYLE.md), not traffic-lig "$(scan "$emoji")" # 3) Banned terminology in user-facing text: 'workspace' -> 'secure environment'. -# -w matches whole words only (so exitNoWorkspace is exempt); skip comments. +# -w matches whole words only (exitNoWorkspace is exempt); skip comment lines +# (content starts with //, anchored to the file:line: prefix). report "banned term 'workspace' in user-facing text — use 'secure environment' (see STYLE.md)" \ - "$(scan 'workspace' '-iw' | grep -vE ':[0-9]+:[[:space:]]*//')" + "$(scan 'workspace' '-iw' | grep -vE '^[^:]+:[0-9]+:[[:space:]]*//')" +if [[ "$guard_error" -ne 0 ]]; then + echo " [!] the guard hit an internal error — failing closed (exit 2)" >&2 + exit 2 +fi if [[ "$fail" -eq 0 ]]; then echo " ok: style + terminology clean" fi From 9926eae3bd30cafa90f5aa0e285489063f0ddab2 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 12:24:58 +0200 Subject: [PATCH 6/7] Address Bugbot (parity): make fail-closed actually reachable (parent-shell scan) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- scripts/check-style.sh | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/scripts/check-style.sh b/scripts/check-style.sh index e3d8f48f..5230f9c7 100755 --- a/scripts/check-style.sh +++ b/scripts/check-style.sh @@ -9,8 +9,7 @@ # # Three mechanical checks (semantic calls — role misuse, judgement-y wording — # stay with CODEOWNERS review + STYLE.md; a grep can't police those): -# 1. No hardcoded brand colour outside the colour engine (internal/ui). New -# output must go through the Printer tones, never a re-hardcoded hex/RGB. +# 1. No hardcoded brand colour outside the colour engine (internal/ui). # 2. No status / traffic-light emoji — the lime dot is the online indicator. # 3. No 'workspace' in user-facing text — the term is "secure environment". # Matched as a whole word, so the exitNoWorkspace code identifier is exempt; @@ -28,10 +27,12 @@ cd "$(dirname "$0")/.." || exit 2 ENGINE='internal/ui/' # the one place raw brand colour legitimately lives fail=0 guard_error=0 +hits='' -# scan REGEX [EXTRA_FLAGS] — grep the Go output surface, dropping tests, this -# guard, and any line that opted out. Prints file:line:text. grep exit 2+ (bad -# regex/flags/tree) is an INTERNAL error → fail closed, never silent-pass. +# scan REGEX [EXTRA_FLAGS] — call in the PARENT shell (scan …, never $(scan …)), +# so the guard_error it sets on a grep internal error actually reaches the parent +# and the fail-closed exit below is reachable. Sets `hits` to the matches (tests +# + opt-out lines removed). grep exit 2+ (bad regex/flags/tree) → fail closed. scan() { local re="$1" flags="${2:-}" out rc # shellcheck disable=SC2086 @@ -39,12 +40,11 @@ scan() { rc=$? if [[ "$rc" -ge 2 ]]; then echo "check-style: grep errored (rc=$rc) on /$re/ — failing closed" >&2 - guard_error=1 - return 0 + guard_error=1; hits=''; return fi - printf '%s' "$out" \ + hits="$(printf '%s' "$out" \ | grep -vE '_test\.go:' \ - | grep -vE '// *style-guard: *allow' || true + | grep -vE '// *style-guard: *allow' || true)" } report() { # TITLE MATCHES @@ -58,20 +58,22 @@ echo "== tracebloc style guard ==" # 1) Hardcoded brand colour outside the engine (case-insensitive: #01A5CC too). brand='#?(01a5cc|91e947|a7ed6c|01637a|578c2b|34b7d6)|38;2;(1;165;204|145;233;71|167;237;108|1;99;122|87;140;43)' +scan "$brand" '-i' report "hardcoded brand colour — use the Printer tones in ${ENGINE}, don't re-hardcode hex/RGB" \ - "$(scan "$brand" '-i' | grep -vE "^${ENGINE}")" + "$(printf '%s' "$hits" | grep -vE "^${ENGINE}" || true)" # 2) Status / traffic-light emoji. Pattern built from bytes so this source stays # emoji-free (green/red/yellow/orange circles). emoji="$(printf '\360\237\237\242|\360\237\224\264|\360\237\237\241|\360\237\237\240')" -report "status emoji — use the lime online dot (see STYLE.md), not traffic-light emoji" \ - "$(scan "$emoji")" +scan "$emoji" +report "status emoji — use the lime online dot (see STYLE.md), not traffic-light emoji" "$hits" # 3) Banned terminology in user-facing text: 'workspace' -> 'secure environment'. # -w matches whole words only (exitNoWorkspace is exempt); skip comment lines # (content starts with //, anchored to the file:line: prefix). +scan 'workspace' '-iw' report "banned term 'workspace' in user-facing text — use 'secure environment' (see STYLE.md)" \ - "$(scan 'workspace' '-iw' | grep -vE '^[^:]+:[0-9]+:[[:space:]]*//')" + "$(printf '%s' "$hits" | grep -vE '^[^:]+:[0-9]+:[[:space:]]*//' || true)" if [[ "$guard_error" -ne 0 ]]; then echo " [!] the guard hit an internal error — failing closed (exit 2)" >&2 From eb15958a3ffba00899583f40c69fabddcd8e8445 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 21 Jul 2026 12:33:43 +0200 Subject: [PATCH 7/7] check-style: surface grep errors instead of suppressing (still fail-closed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parity with the client guard — drop 2>/dev/null; rc>=2 fail-closes. Co-Authored-By: Claude Opus 4.8 --- scripts/check-style.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/check-style.sh b/scripts/check-style.sh index 5230f9c7..e767f564 100755 --- a/scripts/check-style.sh +++ b/scripts/check-style.sh @@ -36,7 +36,9 @@ hits='' scan() { local re="$1" flags="${2:-}" out rc # shellcheck disable=SC2086 - out="$(grep -rnE $flags --include='*.go' "$re" internal/ 2>/dev/null)" + # No 2>/dev/null: let a real grep error surface on stderr — rc>=2 below turns + # it into a fail-closed exit, so the error is visible AND fatal, never a silent pass. + out="$(grep -rnE $flags --include='*.go' "$re" internal/)" rc=$? if [[ "$rc" -ge 2 ]]; then echo "check-style: grep errored (rc=$rc) on /$re/ — failing closed" >&2