B5: skill CLI (new/test/bench/audit/suggest) by thrillmot · Pull Request #124 · thrillmade/logmind

thrillmot · 2026-06-02T20:28:17Z

Summary

Wave B5 of the SkDD v1.0 cutover: ports logmind skill new/test/bench/audit/suggest from Python (src/logmind/core/skill_cli.py at v0.6.16) to Go.

internal/skill/ new package mirrors the Python module shape (scaffold, validate, bench, audit, suggest heuristic + LLM transport, provenance).
internal/cli/skill.go wires the cobra command tree; one file per Python click subcommand.
LLM-backed skill suggest via Anthropic Messages API (net/http — no external SDK). --no-llm flag forces heuristic mode; engine config in .logmind/config.yml skill_suggest.* (defaults: engine: llm, claude-haiku-4-5, 2000 max_tokens). Graceful fallback to heuristic when ANTHROPIC_API_KEY (or configured env var) is missing AND fallback_to_heuristic_on_no_key: true.
PROVENANCE.md skeleton emitted alongside SKILL.md by default (per the v0.6.x plan's skill workflow loop). Suppress via --no-provenance.
Byte-identical parity verified vs Python v0.6.14:
- skill new <name> stdout text + generated SKILL.md body
- skill test <name> pass/fail check lines + ok-trailer
- skill bench <name> table + --json output
- skill audit table + --json (status field, last-touched date)
- skill suggest --no-llm human-readable + --json
- Error paths: missing skill, already-exists, malformed --since

Test plan

go test ./internal/skill/ ./internal/cli/ — 30+ unit + snapshot tests, all green
go vet ./... — clean
Manual diff Go binary vs venv/bin/logmind for new, test, bench, audit, suggest — byte-identical (modulo /private/var/folders symlink prefix on macOS)
Anthropic HTTP path covered by httptest.NewServer round-trip test
LLM fallback path (engine=llm + no key) covered end-to-end via runSkillSuggest test

Coordination with B6

internal/config/ is owned by the parallel B6 wave. B5 adds a tiny local YAML helper (internal/cli/skill_config.go) to read only the skill_suggest: block; when B6 lands a richer Config struct, drop the helper and call config.Load(cwd).SkillSuggest directly.

🤖 Generated with Claude Code

… audit, suggest (heuristic)

vercel · 2026-06-02T20:28:23Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
logmind-site	Ignored	Preview	Jun 2, 2026 9:22pm

claude · 2026-06-02T20:28:47Z

Claude finished @thrillmot's task in 4m 26s —— View job

B5 skill CLI review

Fetch PR diff and skills config
Read high-risk files (LLM transport, config, scaffold, CLI)
Review remaining files (audit, bench, validate, provenance, tests, golden)
Post inline findings
Emit structured summary

🐛 Clud Bug review

This round: 2 critical · 1 minor · 0 resolved from prior · 0 still open

Found: 2 🔴 / 1 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: scanned all 27 files. 2 critical findings below.
[evidence-based-review]: applied to all findings. ✓ all anchored to specific lines.
[respect-existing-conventions]: no convention fights; new Go package follows established internal/ patterns.

Critical findings

🔴 [critical-issues-only]: io.ReadAll(resp.Body) with no size limit in the Anthropic HTTP transport (internal/skill/suggest_llm.go:248).

Reasoning

A misbehaving or attacker-controlled proxy can stream an arbitrarily large body; the 30-second timeout only gates on time, not bytes. With max_tokens=2000 the legitimate response is ~10 KB, so a 1 MB LimitReader is a safe guard: io.LimitReader(resp.Body, 1<<20).

🔴 [critical-issues-only]: strings.Count(decisionText, name) uses substring matching for the audit DecisionCount field, breaking the "ghost" skill detector (internal/skill/audit.go:85).

Reasoning

A skill named "go" matches "going", "postgres", etc., producing counts in the hundreds. Classify uses DecisionCount == 0 as the sole ghost gate (line 152); any skill whose name is a common substring is permanently immune from ghost classification. Fix: wrap in \b...\b word-boundary regex via regexp.QuoteMeta.

Minor findings

🟡 [critical-issues-only]: extractJSONBlob uses strings.LastIndex(text, "}") — any trailing } in the LLM's preamble produces invalid JSON and silently falls back to heuristic (internal/skill/suggest_llm.go:342).

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

github-actions · 2026-06-02T20:33:32Z

🐛 Clud Bug review

This round: 2 critical · 1 minor · 0 resolved from prior · 0 still open

Found: 2 🔴 / 1 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: scanned all 27 files. 2 critical findings below.
[evidence-based-review]: applied to all findings. all anchored to specific lines.
[respect-existing-conventions]: no convention fights; new Go package follows established internal/ patterns. 0 findings.

Critical findings

🔴 [critical-issues-only]: io.ReadAll with no size limit on Anthropic HTTP response body risks OOM (internal/skill/suggest_llm.go:248).

Reasoning

A misbehaving or proxy-injected endpoint can stream an arbitrarily large body; the 30-second timeout gates time, not bytes. Legitimate max_tokens=2000 response is ~10 KB — a 1 MB LimitReader is sufficient: io.LimitReader(resp.Body, 1<<20). io is already imported.

🔴 [critical-issues-only]: strings.Count uses substring matching for DecisionCount, permanently masking ghost skills with common short names (internal/skill/audit.go:85).

Reasoning

A skill named 'go' matches 'going', 'postgres', 'logo' etc., producing counts of hundreds. Classify uses DecisionCount == 0 as the sole ghost gate (line 152), so any skill whose name is a common substring is immune from ghost detection. Fix: wrap name in word-boundary regex via regexp.QuoteMeta.

Minor findings

🟡 [critical-issues-only]: extractJSONBlob uses last-} heuristic — trailing } in LLM preamble silently degrades to heuristic fallback (internal/skill/suggest_llm.go:342).

Reasoning

strings.LastIndex(text, "}") grabs the last } in the entire response. If the model emits any commentary containing } after the JSON object, the extracted slice is invalid JSON, json.Unmarshal fails, and the command falls back to heuristic without telling the user the LLM parse failed. The system prompt says 'no commentary' so this is rare but not impossible.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

…der, JSON brace-balance, audit comment)

claude · 2026-06-02T20:36:20Z

Claude finished @thrillmot's task in 2m 45s —— View job

B5 skill CLI — fix-push re-review

Load tools
Get prior SHA (cffc7cc) + incremental diff
Check prior review threads (LimitReader, DecisionCount regex, extractJSONBlob)
Verify fixes are correct; scan delta for new issues
Resolve fixed threads (resolved: LimitReader ✅, extractJSONBlob ✅; kept open: DecisionCount ⚠️)
Emit structured summary

This round: 0 critical · 0 minor · 2 resolved from prior · 1 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Prior findings status

Resolved ✅ — LimitReader (suggest_llm.go:252)
io.LimitReader(resp.Body, maxLLMResponseBytes) added with const maxLLMResponseBytes = 1 << 20. A dedicated TestAnthropicSuggester_LimitReader test pins the behaviour. Fix is correct and complete.

Resolved ✅ — extractJSONBlob brace-balance (suggest_llm.go:346+)
The last-} heuristic was replaced with a proper brace-depth scanner that honours string literals and \" escapes. Five regression test cases (including the trailing-brace and brace-inside-string cases from the prior finding) were added. Fix is correct and complete.

Still open ⚠️ — DecisionCount substring matching (audit.go:102)
The underlying strings.Count call is unchanged. The author added an explicit comment documenting the known limitation and the parity constraint (byte-identical with Python v0.6.16), with a TODO pointing to the regexp.QuoteMeta fix for when v1.0 spec permits a behaviour change. The ghost classifier weakness remains in the code — common short-name skills (e.g. "go", "api") are immune from ghost detection. Leaving the thread open per this acknowledged state.

Per-skill scan

[critical-issues-only]: delta only (cffc7cc → HEAD). 0 new findings. 2 prior criticals resolved, 1 still open (documented-not-fixed).
[evidence-based-review]: all claims anchored to specific lines. 0 violations.
[respect-existing-conventions]: 0 convention fights in the delta.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

github-actions · 2026-06-02T20:39:35Z

🐛 Clud Bug review

This round: 0 critical · 0 minor · 2 resolved from prior · 1 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: delta reviewed (cffc7cc → HEAD). 0 new findings; 2 prior criticals resolved, 1 still open.
[evidence-based-review]: all findings anchored. 0 violations.
[respect-existing-conventions]: 0 convention fights in the delta.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

…ord matching (clud-bug PR #124)

claude · 2026-06-02T20:42:43Z

Claude finished @thrillmot's task in 2m 46s —— View job

B5 skill CLI — fix-push re-review (round 3)

Load tools
Fetch incremental diff (0ef2d95 → 96bacbf)
Verify DecisionCount whole-word fix — correct and complete
Scan delta for new issues — 0 found
Resolve prior DecisionCount thread ✅
Emit structured summary

🐛 Clud Bug review

This round: 0 critical · 0 minor · 1 resolved from prior · 0 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: delta reviewed (0ef2d95 → 96bacbf). 0 new findings. 1 prior critical resolved.
[evidence-based-review]: all prior findings anchored. 0 violations.
[respect-existing-conventions]: 0 convention fights in delta.

Prior findings status

Resolved ✅ — DecisionCount whole-word matching (audit.go)
countWholeWord(corpus, name) replaces the bare strings.Count call. Uses regexp.Compile(\b + regexp.QuoteMeta(name) + \b) — word-boundary anchors prevent "go" from matching "going" / "logo" / etc. Empty-string guards and error handling (returns 0, not panic) are correct. TestCountWholeWord pins the key cases including the go/going regression. Fix is correct and complete.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

github-actions · 2026-06-02T20:45:48Z

🐛 Clud Bug review

This round: 0 critical · 0 minor · 1 resolved from prior · 0 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: delta reviewed (0ef2d95 → 96bacbf). 0 new findings; 1 prior critical resolved.
[evidence-based-review]: all findings anchored. 0 violations.
[respect-existing-conventions]: 0 convention fights in delta.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

Workflow-template audit tests substitute `__version__` into expected pip-install pin; main is at 0.6.16, this branch was at 0.6.14, all 3 audit-test assertions fail pre-existingly on every Go PR opened against v1-go-rewrite (B5 #124, B6 #125, future waves). 20/20 tests in `tests/test_v0_2_1_audit_fixes.py` pass locally with the bump. Once merged, re-running CI on B5 #124 + B6 #125 unblocks both for auto-merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: thrillmot <thrillmot@users.noreply.github.com>

…anch

# Conflicts: # docs/file-structure.md # docs/timeline.md

claude · 2026-06-02T21:22:29Z

Claude finished @thrillmot's task in 1m 17s —— View job

B5 skill CLI — fix-push re-review (round 4)

Load tools + get HEAD SHA (53a6593)
Verify 96bacbf ancestry → confirmed ancestor intact
Fetch incremental diff (96bacbf → 53a6593) — version bumps + docs only
Check prior threads — all 3 resolved (confirmed via GraphQL)
Scan delta for new issues — 0 found
Emit structured summary

This round: 0 critical · 0 minor · 0 resolved from prior · 0 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: delta reviewed (96bacbf → 53a6593). Version bumps + doc updates only. 0 new findings.
[evidence-based-review]: no findings to anchor. 0 violations.
[respect-existing-conventions]: 0 convention fights in delta.

Delta summary

The two new commits cherry-pick the 0.6.14 → 0.6.16 version bump from v1-go-rewrite into this branch to unblock pytest CI. Changes: pyproject.toml, src/logmind/__init__.py, src/logmind/cli.py (version strings only), plus auto-generated docs (timeline.md, file-structure.md, decision-branch files) and .claude/worktrees/ submodule entries. No skill CLI code touched.

All prior B5 findings (LimitReader, brace-balance extractor, DecisionCount whole-word matching) remain resolved.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

github-actions · 2026-06-02T21:24:05Z

🐛 Clud Bug review

This round: 0 critical · 0 minor · 0 resolved from prior · 0 still open

Found: 0 🔴 / 0 🟡 / 0 🟣

Per-skill scan

[critical-issues-only]: delta reviewed (96bacbf → 53a6593). Version bumps + docs only. 0 new findings.
[evidence-based-review]: no findings to anchor. 0 violations.
[respect-existing-conventions]: 0 convention fights in delta.

Skills referenced: [critical-issues-only, evidence-based-review, respect-existing-conventions]

logmind: B5 wave 1/5: skill core package — scaffold, validate, bench,…

cffc7cc

… audit, suggest (heuristic)

vercel Bot deployed to Preview June 2, 2026 20:28 View deployment

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread internal/skill/suggest_llm.go Outdated

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread internal/skill/audit.go Outdated

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread internal/skill/suggest_llm.go Outdated

logmind: B5 review fixes: address clud-bug PR #124 findings (LimitRea…

0ef2d95

…der, JSON brace-balance, audit comment)

logmind: B5 review fix round 2: switch audit DecisionCount to whole-w…

96bacbf

…ord matching (clud-bug PR #124)

thrillmot mentioned this pull request Jun 2, 2026

chore: bump __version__ 0.6.14 → 0.6.16 on v1-go-rewrite #126

Merged

thrillmot added 2 commits June 2, 2026 17:19

logmind: chore: rebase __version__ bump from v1-go-rewrite into B5 br…

3fa9b9e

…anch

merge v1-go-rewrite into B5

53a6593

# Conflicts: # docs/file-structure.md # docs/timeline.md

thrillmot merged commit 35e1dbd into v1-go-rewrite Jun 2, 2026
16 checks passed

thrillmot deleted the feat/go-b5-skills branch June 2, 2026 21:24

thrillmot mentioned this pull request Jun 3, 2026

v1.0.0 cutover: Go rewrite → main #132

Merged

7 tasks

Navigation Menu

Uh oh!

B5: skill CLI (new/test/bench/audit/suggest) - #124

B5: skill CLI (new/test/bench/audit/suggest)#124
thrillmot merged 5 commits into
v1-go-rewritefrom
feat/go-b5-skills

thrillmot commented Jun 2, 2026

Uh oh!

vercel Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

claude Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

claude Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

claude Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

thrillmot commented Jun 2, 2026

Summary

Test plan

Coordination with B6

Uh oh!

vercel Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

B5 skill CLI review

🐛 Clud Bug review

Per-skill scan

Critical findings

Minor findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 2, 2026

🐛 Clud Bug review

Per-skill scan

Critical findings

Minor findings

Uh oh!

claude Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

B5 skill CLI — fix-push re-review

Prior findings status

Per-skill scan

Uh oh!

github-actions Bot commented Jun 2, 2026

🐛 Clud Bug review

Per-skill scan

Uh oh!

claude Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

B5 skill CLI — fix-push re-review (round 3)

🐛 Clud Bug review

Per-skill scan

Prior findings status

Uh oh!

github-actions Bot commented Jun 2, 2026

🐛 Clud Bug review

Per-skill scan

Uh oh!

claude Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

B5 skill CLI — fix-push re-review (round 4)

Per-skill scan

Delta summary

Uh oh!

github-actions Bot commented Jun 2, 2026

🐛 Clud Bug review

Per-skill scan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented Jun 2, 2026 •

edited

Loading

claude Bot commented Jun 2, 2026 •

edited

Loading

claude Bot commented Jun 2, 2026 •

edited

Loading

claude Bot commented Jun 2, 2026 •

edited

Loading

claude Bot commented Jun 2, 2026 •

edited

Loading