Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"hooks": {
"PostToolUse": [
{
"matcher": "Edit|Write|MultiEdit",
"hooks": [
{
"type": "command",
"command": "input=$(cat); case \"$input\" in *'.go\"'*) cd \"$CLAUDE_PROJECT_DIR\" || exit 0; out=$(go test ./internal/sourceguard/ -count=1 2>&1) || { printf '%s\\n' \"$out\" >&2; exit 2; };; esac",
"statusMessage": "non-ASCII source guard",
"timeout": 60
}
]
}
]
}
}
52 changes: 47 additions & 5 deletions internal/cli/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"
"strings"
"text/tabwriter"

"github.com/skillrig/cli/pkg/skillcore"
)
Expand Down Expand Up @@ -156,8 +157,16 @@ func renderSearchResult(w io.Writer, origin string, matches []skillcore.CatalogE

var b strings.Builder

// Align name/version/description into fixed-width columns so the ragged,
// unreadable output of issue #21 becomes a clean table. The table is rendered
// before the footer so the summary line is not drawn into the columns.
rows := make([][]string, 0, len(matches))
for _, e := range matches {
fmt.Fprintf(&b, "%s %s — %s\n", e.Name, e.Version, truncateDesc(e.Description))
rows = append(rows, []string{e.Name, e.Version, "— " + truncateDesc(e.Description)})
}

if err := writeAlignedColumns(&b, rows); err != nil {
return err
}

fmt.Fprintf(&b, "%d skill(s) · run: skillrig add <name>\n", len(matches))
Expand All @@ -167,18 +176,42 @@ func renderSearchResult(w io.Writer, origin string, matches []skillcore.CatalogE
return err
}

// writeAlignedColumns renders rows as space-padded, fixed-width columns via the
// stdlib text/tabwriter (elastic tabstops) so columns line up across rows in
// compact human output. It is the one shared table renderer for search and the
// verify failure list — no duplicated tabwriter setup. Each row's cells are
// tab-joined; the final cell is newline-terminated, so it is never right-padded
// (no trailing whitespace). Cell width is counted in runes (tabwriter assumes
// equal-width code points), matching truncateDesc's rune budget so the column
// math and the clip agree. An empty rows slice writes nothing.
func writeAlignedColumns(w io.Writer, rows [][]string) error {
tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0)
for _, cells := range rows {
if _, err := fmt.Fprintln(tw, strings.Join(cells, "\t")); err != nil {
return err
}
}

return tw.Flush()
}

// searchEmptyFooter is the next-step hint for an empty search result (still exit 0).
const searchEmptyFooter = "→ broaden the query, or run skillrig search with no filter to list all"

// truncateDesc collapses a description's newlines to spaces and clips it to
// searchDescWidth for compact human output (the complete text is in --json).
// searchDescWidth for compact human output (the complete text is in --json). The
// width budget and the clip are counted in runes, not bytes: byte-slicing could
// split a multibyte rune into invalid UTF-8, and rune counting matches how
// tabwriter measures cell width, so the column budget and the alignment agree.
func truncateDesc(s string) string {
s = strings.ReplaceAll(s, "\n", " ")
if len(s) <= searchDescWidth {

r := []rune(s)
if len(r) <= searchDescWidth {
return s
}

return s[:searchDescWidth-1] + "…"
return string(r[:searchDescWidth-1]) + "…"
}

// indexResult is the presentation-layer view of an index generation: where the
Expand Down Expand Up @@ -283,12 +316,21 @@ func renderVerifyFailure(w io.Writer, r skillcore.Report) error {

fmt.Fprintf(&b, "verify FAILED: %d of %d skills\n", failed, total)

// Align the failing skills' reason column the same way search aligns its
// table — the marker+name stays the first (tab-terminated) cell so "✗ <name>"
// reads as one token, and reasons line up across rows.
rows := make([][]string, 0, failed)

for _, v := range r.Verdicts {
if v.Status == skillcore.StatusOK {
continue
}

fmt.Fprintf(&b, " ✗ %s %s\n", v.Name, verdictLine(v))
rows = append(rows, []string{" ✗ " + v.Name, verdictLine(v)})
}

if err := writeAlignedColumns(&b, rows); err != nil {
return err
}

b.WriteString(verifyFailFooter + "\n")
Expand Down
119 changes: 119 additions & 0 deletions internal/sourceguard/ascii_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Package sourceguard holds a repo-wide source hygiene guard. It has no
// production code — only a test that every .go file in the module stays ASCII
// except for an explicitly sanctioned set of glyphs.
//
// Why: skillrig's human output deliberately uses a few non-ASCII glyphs (the
// "→" next-step hint, "✓/✗" verify status, the "—/·/…" separators). That set is
// intentional and kept (see issue #21 discussion). What we want to prevent is
// the ACCIDENTAL introduction of *other* non-ASCII runes — smart quotes from a
// copy-paste, a non-breaking space, a homoglyph — which render unpredictably in
// terminals/CI and are nearly invisible in review. golangci-lint cannot catch
// this: asciicheck only inspects identifiers, and bidichk only catches dangerous
// invisible/bidi runes, not ordinary glyphs in string literals.
//
// This test is the single source of truth for the allowlist. It runs as part of
// `go test ./...` (so `make check` and CI enforce it for free) and is also what
// the PostToolUse agent-loop hook invokes. To introduce a new glyph, add it to
// sanctioned below with a comment — a deliberate, reviewable edit.
package sourceguard

import (
"os"
"path/filepath"
"strings"
"testing"
)

// sanctioned is the set of non-ASCII runes allowed anywhere in .go source. Each
// entry is intentional; adding one is a deliberate decision recorded here.
var sanctioned = map[rune]string{
'→': "U+2192 next-step footer hints (init/add/search/index/verify)",
'✓': "U+2713 verify OK status",
'✗': "U+2717 verify failure status",
'—': "U+2014 em dash — prose/help/output separator",
'·': "U+00B7 search footer separator (N skill(s) · run:)",
'…': "U+2026 truncation/elision marker",
'•': "U+2022 bullet in `add` help text",
'§': "U+00A7 section sign in Constitution references (comments)",
'≤': "U+2264 bounded-output notes in comments (≤2 lines)",
'–': "U+2013 en dash in numeric ranges in comments (rows 1–7)",
'⇒': "U+21D2 logical implication in a comment",
}

// TestSourceIsASCIIExceptSanctioned walks the module and fails on any non-ASCII
// rune that is not in sanctioned, reporting file:line:col and the offending rune
// so the fix is obvious: either revert to ASCII, or (if genuinely intended) add
// the rune to sanctioned.
func TestSourceIsASCIIExceptSanctioned(t *testing.T) {
t.Parallel()

root := moduleRoot(t)

var violations []string

walkErr := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error {
if err != nil {
return err
}

if d.IsDir() {
if skipDir(d.Name()) {
return filepath.SkipDir
}

return nil
}

if !strings.HasSuffix(d.Name(), ".go") {
return nil
}

violations = append(violations, scanFile(t, path)...)

return nil
})
if walkErr != nil {
t.Fatalf("walking module from %q: %v", root, walkErr)
}

if len(violations) > 0 {
t.Fatalf("found %d unsanctioned non-ASCII rune(s) in .go source:\n%s\n\n"+
"fix: revert to ASCII, or if the glyph is intentional add it to "+
"sanctioned in internal/sourceguard/ascii_test.go with a justification.",
len(violations), strings.Join(violations, "\n"))
}
}

// scanFile returns one violation string per unsanctioned non-ASCII rune in the
// file at path, tracking the path relative to the module root for readability.
func scanFile(t *testing.T, path string) []string {
t.Helper()

data, err := os.ReadFile(path)
if err != nil {
t.Fatalf("reading %q: %v", path, err)
}

var out []string

rel := relOrPath(path)

for i, text := range strings.Split(string(data), "\n") {
col := 0
for _, r := range text {
col++

if r < 128 {
continue
}

if _, ok := sanctioned[r]; ok {
continue
}

out = append(out, formatViolation(rel, i+1, col, r))
}
}

return out
}
64 changes: 64 additions & 0 deletions internal/sourceguard/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package sourceguard

import (
"fmt"
"os"
"path/filepath"
"testing"
)

// modRoot is the module root discovered by moduleRoot, reused to render paths
// relative to it. Set once before the parallel scan reads it.
var modRoot string

// moduleRoot walks up from the test's working directory (the package dir) until
// it finds go.mod, and returns that directory — the module root to scan.
func moduleRoot(t *testing.T) string {
t.Helper()

dir, err := os.Getwd()
if err != nil {
t.Fatalf("getwd: %v", err)
}

for {
if _, statErr := os.Stat(filepath.Join(dir, "go.mod")); statErr == nil {
modRoot = dir

return dir
}

parent := filepath.Dir(dir)
if parent == dir {
t.Fatalf("no go.mod found walking up from working directory")
}

dir = parent
}
}

// skipDir reports whether a directory should be skipped: VCS/tooling/build
// directories that either are not our source or hold generated/vendored trees.
func skipDir(name string) bool {
switch name {
case ".git", ".claude", "vendor", "node_modules", "dist", "testdata":
return true
default:
return false
}
}

// relOrPath renders path relative to the module root, falling back to the
// absolute path if that fails — purely for readable violation messages.
func relOrPath(path string) string {
if rel, err := filepath.Rel(modRoot, path); err == nil {
return rel
}

return path
}

// formatViolation renders one offending rune as "rel:line:col: U+XXXX 'r'".
func formatViolation(rel string, line, col int, r rune) string {
return fmt.Sprintf(" %s:%d:%d: %U %q", rel, line, col, r, string(r))
}
Loading