Uh oh!
There was an error while loading. Please reload this page.
Enforce the terminal style system + terminology (installer) - #364
Conversation
Guards the style system (STYLE.md, shipped in #363) against regressions from other contributors, layered strongest-first: - scripts/check-style.sh — a CI gate (blocking, in the "Static analysis" job) with three MECHANICAL checks: no hardcoded brand colour outside the tone engine (common.sh), no status/traffic-light emoji, and no "workspace" in user-facing text (→ "secure environment"). Internal identifiers (the DNS-1123 sanitisers), comments, and scripts/tests/ are exempt; a line can opt out with a trailing `# style-guard: allow`. Semantic calls (role misuse, softer wording) stay with review — a grep can't police those. - STYLE.md — the reference: cyan=structure / lime=action, the role→tone table, where the engine lives, and the terminology glossary (SoT = docs TERMINOLOGY.md). - CODEOWNERS — the guard + STYLE.md are code-owned, so the rules themselves can't be quietly weakened. (The engine files under scripts/lib/ + the ps1 are already owned via the R8 trust root.) - PR template — a checklist line pointing at STYLE.md + the guard. Cleaned the two pre-existing violations the guard surfaced so it ships blocking with develop clean: - common.sh --help: "Namespace / workspace label" → "Secure-environment name". - install-k8s.ps1 summary: "Workspace :" → "Environment :" (aligns with the bash summary; re-padded the Version/Mode labels to match). No Pester assertion keys on the label text. Manifest regenerated (R8) for the common.sh + ps1 edits. Guard passes; bash -n + shellcheck clean; the 3 local bats failures are pre-existing on develop (confirmed with the change stashed), unrelated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
@BugBot run |
Uh oh!
There was an error while loading. Please reload this page.
The hex scan matched only lowercase (01a5cc), so an uppercase #01A5CC would slip past. Add -i to check 1. (Same fix applied to the cli guard.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
@BugBot run |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…ck the guard Three findings on #364. 1. Fails open on errors (Medium). scan swallowed grep stderr and read empty as clean, so a scan failure (bad regex/flags, missing tree) passed the blocking gate silently. Now: a missing scripts/ tree exits 2 up front, and grep exit ≥2 sets guard_error → the guard exits 2 (fail closed), never a silent pass. 2. Comment filter matched mid-line (Low). ':[0-9]+: *#' could exempt a user-facing string that merely embeds that shape (e.g. a URL port + fragment). Anchored to the file:line: prefix: '^[^:]+:[0-9]+:[[:space:]]*#'. 3. Guard omitted from ShellCheck (Low). Added scripts/check-style.sh to the Static-analysis ShellCheck step so regressions in the guard itself are caught. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
@BugBot run |
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to the previous fail-closed attempt, which didn't work: guard_error was set inside scan, but scan was invoked via $(scan …) — a command-substitution subshell — so the flag never reached the parent and the `exit 2` path was unreachable (Bugbot: "fail-closed path never triggers"). A grep error could still print ok / exit 0. Fix: call scan in the PARENT shell (scan …, not $(scan …)); it now sets a global `hits` plus guard_error directly, and the per-check display filtering moves to the report() argument. Verified: a forced grep error (invalid regex) now propagates guard_error and exits 2. (The other two re-flags — guard omitted from ShellCheck, and the make-ci gap on the cli side — were already fixed in earlier commits; confirmed present.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
@BugBot run |
There was a problem hiding this comment.
✅ 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 ee6be13. Configure here.
…losed) Drop 2>/dev/null on the scan grep so a real internal error prints to stderr; rc>=2 already turns it into a fail-closed exit 2 (verified). Also removes the pattern that kept tripping Bugbot's (now-inaccurate) 'fails open' heuristic — the rc check on the very next line is the actual fail-closed path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 21, 2026
@BugBot run |
There was a problem hiding this comment.
✅ 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 83b808d. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Guards the terminal style system (STYLE.md, shipped in #363) against regressions from other contributors — Option 1 from the enforcement discussion, layered strongest-first. Blocking CI gate for the mechanical rules + docs + CODEOWNERS for the judgement calls.
What's added
scripts/check-style.sh— a CI gate (blocking, in the Static analysis job) with three mechanical checks:common.sh) — use theTB_*tones●is the online indicatorworkspacein user-facing text → secure environmentExempt: internal identifiers (the DNS-1123 sanitisers), comments, and
scripts/tests/. A line can opt out with a trailing# style-guard: allow.STYLE.md— the reference: cyan = structure, lime = action, the role→tone table, where the engine lives, and the terminology glossary (SoT = docsTERMINOLOGY.md).CODEOWNERS — the guard +
STYLE.mdare code-owned so the rules can't be quietly weakened. (The enginescripts/lib/+ the ps1 are already owned via the R8 trust root.)PR template — a checklist line pointing at STYLE.md + the guard.
Why only these three checks
Mechanical, low-false-positive. Role misuse (a command shown in the heading tone) and softer wording calls can't be greped reliably — those stay with CODEOWNERS review.
clientis deliberately not flagged (legit CLI verb + code identifier); onlyworkspaceis unambiguous.Cleaned so it ships blocking (develop clean)
The guard surfaced two real violations, both fixed here:
common.sh--help: "Namespace / workspace label" → "Secure-environment name"install-k8s.ps1summary: "Workspace :" → "Environment :" (re-padded Version/Mode to align; no Pester assertion keys on the label)Test plan
bash scripts/check-style.sh→ clean; wired blocking into the Static-analysis job.gen-manifest.sh --checkpasses.bash -n+ shellcheck clean on the guard. Workflow YAML validates. The 3 local bats failures pre-date this change (confirmed with the diff stashed).🤖 Generated with Claude Code
Note
Low Risk
Changes are docs, CI linting, and user-facing label text only; no runtime installer logic beyond wording and supply-chain manifest hashes.
Overview
Adds a blocking installer style system:
STYLE.mddocuments cyan/lime roles, tone helpers, and terminology;scripts/check-style.shenforces three mechanical rules onscripts/**/*.shand*.ps1(excluding tests): no brand hex/RGB outsidecommon.sh, no traffic-light emoji, and no user-facingworkspace(with comment/sanitizer exemptions and# style-guard: allow).CI and governance: the Static analysis job runs the guard and includes it in ShellCheck; CODEOWNERS gates
STYLE.mdand the guard; the PR template adds a STYLE checklist line.Copy fixes so the gate passes: bash
--helpuses “Secure-environment name” forTB_NAMESPACE; Windows connected summary shows Environment : instead of Workspace :.manifest.sha256updated for the touched signed sub-scripts.Reviewed by Cursor Bugbot for commit 83b808d. Bugbot is set up for automated code reviews on this repo. Configure here.