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
7 changes: 4 additions & 3 deletions internal/agentkit/agentkit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package agentkit
import (
"os"
"path/filepath"
"strings"
"testing"
)

Expand Down Expand Up @@ -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++
}
}
Expand Down
2 changes: 2 additions & 0 deletions openspec/changes/agentkit-hasprefix-refactor/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: unbound-force
created: 2026-06-30
28 changes: 28 additions & 0 deletions openspec/changes/agentkit-hasprefix-refactor/design.md
Original file line number Diff line number Diff line change
@@ -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.
64 changes: 64 additions & 0 deletions openspec/changes/agentkit-hasprefix-refactor/proposal.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 14 additions & 0 deletions openspec/changes/agentkit-hasprefix-refactor/tasks.md
Original file line number Diff line number Diff line change
@@ -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