diff --git a/internal/agentkit/agentkit_test.go b/internal/agentkit/agentkit_test.go index dfbdb63..281020e 100644 --- a/internal/agentkit/agentkit_test.go +++ b/internal/agentkit/agentkit_test.go @@ -3,6 +3,7 @@ package agentkit import ( "os" "path/filepath" + "strings" "testing" ) @@ -130,11 +131,11 @@ func TestScaffold_FileCount(t *testing.T) { var commands, skills, agents int for _, r := range results { switch { - case len(r.Path) > 9 && r.Path[:9] == "commands/": + case strings.HasPrefix(r.Path, "commands/"): commands++ - case len(r.Path) > 7 && r.Path[:7] == "skills/": + case strings.HasPrefix(r.Path, "skills/"): skills++ - case len(r.Path) > 7 && r.Path[:7] == "agents/": + case strings.HasPrefix(r.Path, "agents/"): agents++ } } diff --git a/openspec/changes/agentkit-hasprefix-refactor/.openspec.yaml b/openspec/changes/agentkit-hasprefix-refactor/.openspec.yaml new file mode 100644 index 0000000..ed4982b --- /dev/null +++ b/openspec/changes/agentkit-hasprefix-refactor/.openspec.yaml @@ -0,0 +1,2 @@ +schema: unbound-force +created: 2026-06-30 diff --git a/openspec/changes/agentkit-hasprefix-refactor/design.md b/openspec/changes/agentkit-hasprefix-refactor/design.md new file mode 100644 index 0000000..87a9db7 --- /dev/null +++ b/openspec/changes/agentkit-hasprefix-refactor/design.md @@ -0,0 +1,28 @@ +## Context + +`TestScaffold_FileCount` in `internal/agentkit/agentkit_test.go` categorizes scaffold results by directory prefix using manual length checks and slice indexing. This is fragile because the integer length constants must stay in sync with the prefix strings. The issue was surfaced during PR #20 review when the `command/` to `commands/` rename required updating a constant from 8 to 9. + +Go's standard library provides `strings.HasPrefix` which eliminates this entire class of error by handling length checks internally. + +## Goals / Non-Goals + +### Goals +- Replace manual `len()+slice` prefix matching with `strings.HasPrefix` in `TestScaffold_FileCount` +- Ensure `strings` package is imported (add if not already present) +- Maintain identical test behavior -- same assertions, same pass/fail conditions + +### Non-Goals +- Refactoring any production code (this is test-only) +- Changing the test's assertion values or logic +- Modifying any other test functions in the file + +## Decisions + +**Use `strings.HasPrefix` directly**: The simplest approach. Replace the three `case` clauses in the `switch` statement with `strings.HasPrefix` calls. No helper functions, no abstractions -- just a direct stdlib call. + +**Rationale**: This is a one-line-per-case replacement. Introducing a helper or table-driven approach would be over-engineering for three static prefix checks. The constitution's Testability principle (IV) is upheld -- the test remains isolated and independently runnable. + +## Risks / Trade-offs + +- **Risk**: Near-zero. `strings.HasPrefix` is a well-established stdlib function with identical semantics to the manual check it replaces. +- **Trade-off**: None meaningful. The refactored code is shorter, clearer, and immune to length/index drift. diff --git a/openspec/changes/agentkit-hasprefix-refactor/proposal.md b/openspec/changes/agentkit-hasprefix-refactor/proposal.md new file mode 100644 index 0000000..fdd3e61 --- /dev/null +++ b/openspec/changes/agentkit-hasprefix-refactor/proposal.md @@ -0,0 +1,64 @@ +## Why + +`TestScaffold_FileCount` in `internal/agentkit/agentkit_test.go` uses manual length-check plus raw slice indexing to categorize scaffold results by directory prefix: + +```go +case len(r.Path) > 9 && r.Path[:9] == "commands/": +case len(r.Path) > 7 && r.Path[:7] == "skills/": +case len(r.Path) > 7 && r.Path[:7] == "agents/": +``` + +This pattern is fragile -- the integer constants (9, 7, 7) must be manually kept in sync with the prefix string lengths. An off-by-one error is easy to introduce, as happened during the `command/` to `commands/` rename where the constant changed from 8 to 9. + +Go's `strings.HasPrefix` eliminates this entire class of error. + +Addresses upstream issue unbound-force/replicator#22. + +## What Changes + +Replace manual `len()+slice` prefix matching in `TestScaffold_FileCount` with `strings.HasPrefix` calls. + +## Capabilities + +### New Capabilities +- None + +### Modified Capabilities +- `TestScaffold_FileCount`: Uses `strings.HasPrefix` for prefix matching, removing fragile manual length constants + +### Removed Capabilities +- None + +## Impact + +- **Affected file**: `internal/agentkit/agentkit_test.go` (lines 132-138) +- **Risk**: Minimal -- test-only change, no production code affected +- **Behavioral change**: None -- identical logic, safer implementation + +## Constitution Alignment + +Assessed against the Unbound Force org constitution. + +### I. Autonomous Collaboration + +**Assessment**: N/A + +This change is internal to a test file and does not affect artifact-based communication or inter-agent interfaces. + +### II. Composability First + +**Assessment**: N/A + +No new dependencies introduced. `strings` is a Go standard library package already imported in the project. + +### III. Observable Quality + +**Assessment**: N/A + +Test-only change. No effect on machine-parseable output or provenance metadata. + +### IV. Testability + +**Assessment**: PASS + +The change improves test maintainability by removing fragile manual index calculations, making the test more robust against future directory renames. diff --git a/openspec/changes/agentkit-hasprefix-refactor/specs/agentkit-test-prefix-matching.md b/openspec/changes/agentkit-hasprefix-refactor/specs/agentkit-test-prefix-matching.md new file mode 100644 index 0000000..7ffa587 --- /dev/null +++ b/openspec/changes/agentkit-hasprefix-refactor/specs/agentkit-test-prefix-matching.md @@ -0,0 +1,30 @@ +## ADDED Requirements + +None. + +## MODIFIED Requirements + +### Requirement: Test prefix matching in TestScaffold_FileCount + +The `TestScaffold_FileCount` test MUST use `strings.HasPrefix` for directory prefix categorization instead of manual length-check and slice indexing. + +Previously: The test used `len(r.Path) > N && r.Path[:N] == "prefix/"` for each directory category, requiring manual synchronization of integer constants with prefix string lengths. + +#### Scenario: Prefix matching uses strings.HasPrefix +- **GIVEN** the `TestScaffold_FileCount` function in `internal/agentkit/agentkit_test.go` +- **WHEN** categorizing scaffold results by directory prefix +- **THEN** all prefix checks MUST use `strings.HasPrefix(r.Path, "prefix/")` instead of manual length and slice operations + +#### Scenario: Test behavior remains identical +- **GIVEN** the refactored `TestScaffold_FileCount` function +- **WHEN** the test suite is executed +- **THEN** the test MUST pass with the same assertions (commands=5, skills=7, agents=3) + +#### Scenario: strings package is imported +- **GIVEN** the refactored test file +- **WHEN** `strings.HasPrefix` is used in the test +- **THEN** the `strings` package MUST be imported + +## REMOVED Requirements + +None. diff --git a/openspec/changes/agentkit-hasprefix-refactor/tasks.md b/openspec/changes/agentkit-hasprefix-refactor/tasks.md new file mode 100644 index 0000000..726f8aa --- /dev/null +++ b/openspec/changes/agentkit-hasprefix-refactor/tasks.md @@ -0,0 +1,14 @@ +## 1. Refactor prefix matching + +- [x] 1.1 Add `"strings"` to the import block in `internal/agentkit/agentkit_test.go` (if not already present) +- [x] 1.2 Replace the three manual `len()+slice` prefix checks in `TestScaffold_FileCount` (lines 132-138) with `strings.HasPrefix` calls: + - `case len(r.Path) > 9 && r.Path[:9] == "commands/"` → `case strings.HasPrefix(r.Path, "commands/")` + - `case len(r.Path) > 7 && r.Path[:7] == "skills/"` → `case strings.HasPrefix(r.Path, "skills/")` + - `case len(r.Path) > 7 && r.Path[:7] == "agents/"` → `case strings.HasPrefix(r.Path, "agents/")` + +## 2. Verification + +- [x] 2.1 Run `go test ./internal/agentkit/...` and confirm `TestScaffold_FileCount` passes with identical assertions (commands=5, skills=7, agents=3) +- [x] 2.2 Run `go vet ./internal/agentkit/...` to confirm no issues +- [x] 2.3 Run `goimports` or confirm import ordering is correct (stdlib group) +- [x] 2.4 Verify constitution alignment: Testability (IV) -- test remains isolated, uses `t.TempDir()`, no external dependencies