fix(ax): align plan IDs and validation helpers

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Virgil 2026-03-30 14:32:01 +00:00
parent 44bfe6224e
commit 7672e0922f
9 changed files with 57 additions and 59 deletions

View file

@ -435,6 +435,7 @@ func (s *PrepSubsystem) gitCmd(ctx context.Context, dir string, args ...string)
## Changelog
- 2026-03-30: main now logs startup failures with structured context, and the workspace contract reference restored usage-example comments for the Action lifecycle messages.
- 2026-03-30: plan IDs now come from core.ID(), workspace prep validates org/repo names with core.ValidateName, and plan paths use core.SanitisePath.
- 2026-03-29: cmd/core-agent no longer rewrites `os.Args` before startup. The binary-owned commands now use named handlers, keeping the entrypoint on Core CLI primitives instead of repo-local argument mutation.
- 2026-03-29: brain/provider.go no longer imports net/http for Gin handlers. Handler responses now use named status constants and shared response helpers. HTTP remains intentionally centralised in pkg/agentic/transport.go.
- 2026-03-26: WIP — net/http consolidated to transport.go (ONE file). net/url + io/fs eliminated. RFC-025 updated with 3 new quality gates (net/http, net/url, io/fs). 1:1 test + example test coverage. Array[T].Deduplicate replaces custom helpers.

View file

@ -22,7 +22,7 @@ type DispatchSyncInput struct {
// DispatchSyncResult is the output of a synchronous task run.
//
// if result.OK { fmt.Println("done:", result.Status) }
// if result.OK { core.Print(nil, "done: %s", result.Status) }
type DispatchSyncResult struct {
OK bool
Status string

View file

@ -158,8 +158,7 @@ func TestPlan_ValidPlanStatus_Bad(t *testing.T) {
func TestPlan_GeneratePlanID_Good(t *testing.T) {
id := generatePlanID("Fix the login bug in auth service")
assert.True(t, len(id) > 0)
assert.True(t, strings.Contains(id, "fix-the-login-bug"))
assertCoreIDFormat(t, id)
}
// --- DefaultBranch ---

View file

@ -4,8 +4,6 @@ package agentic
import (
"context"
"crypto/rand"
"encoding/hex"
"time"
core "dappco.re/go/core"
@ -14,7 +12,7 @@ import (
// Plan represents an implementation plan for agent work.
//
// plan := &Plan{ID: "migrate-core-abc", Title: "Migrate Core", Status: "draft", Objective: "..."}
// plan := &Plan{ID: "id-1-a3f2b1", Title: "Migrate Core", Status: "draft", Objective: "..."}
// writePlan(PlansRoot(), plan)
type Plan struct {
ID string `json:"id"`
@ -58,7 +56,7 @@ type PlanCreateInput struct {
// PlanCreateOutput is the output for agentic_plan_create.
//
// out := agentic.PlanCreateOutput{Success: true, ID: "migrate-pkg-agentic-abc123"}
// out := agentic.PlanCreateOutput{Success: true, ID: "id-1-a3f2b1"}
type PlanCreateOutput struct {
Success bool `json:"success"`
ID string `json:"id"`
@ -67,14 +65,14 @@ type PlanCreateOutput struct {
// PlanReadInput is the input for agentic_plan_read.
//
// input := agentic.PlanReadInput{ID: "migrate-pkg-agentic-abc123"}
// input := agentic.PlanReadInput{ID: "id-1-a3f2b1"}
type PlanReadInput struct {
ID string `json:"id"`
}
// PlanReadOutput is the output for agentic_plan_read.
//
// out := agentic.PlanReadOutput{Success: true, Plan: agentic.Plan{ID: "migrate-pkg-agentic-abc123"}}
// out := agentic.PlanReadOutput{Success: true, Plan: agentic.Plan{ID: "id-1-a3f2b1"}}
type PlanReadOutput struct {
Success bool `json:"success"`
Plan Plan `json:"plan"`
@ -82,7 +80,7 @@ type PlanReadOutput struct {
// PlanUpdateInput is the input for agentic_plan_update.
//
// input := agentic.PlanUpdateInput{ID: "migrate-pkg-agentic-abc123", Status: "verified"}
// input := agentic.PlanUpdateInput{ID: "id-1-a3f2b1", Status: "verified"}
type PlanUpdateInput struct {
ID string `json:"id"`
Status string `json:"status,omitempty"`
@ -103,14 +101,14 @@ type PlanUpdateOutput struct {
// PlanDeleteInput is the input for agentic_plan_delete.
//
// input := agentic.PlanDeleteInput{ID: "migrate-pkg-agentic-abc123"}
// input := agentic.PlanDeleteInput{ID: "id-1-a3f2b1"}
type PlanDeleteInput struct {
ID string `json:"id"`
}
// PlanDeleteOutput is the output for agentic_plan_delete.
//
// out := agentic.PlanDeleteOutput{Success: true, Deleted: "migrate-pkg-agentic-abc123"}
// out := agentic.PlanDeleteOutput{Success: true, Deleted: "id-1-a3f2b1"}
type PlanDeleteOutput struct {
Success bool `json:"success"`
Deleted string `json:"deleted"`
@ -126,7 +124,7 @@ type PlanListInput struct {
// PlanListOutput is the output for agentic_plan_list.
//
// out := agentic.PlanListOutput{Success: true, Count: 2, Plans: []agentic.Plan{{ID: "migrate-pkg-agentic-abc123"}}}
// out := agentic.PlanListOutput{Success: true, Count: 2, Plans: []agentic.Plan{{ID: "id-1-a3f2b1"}}}
type PlanListOutput struct {
Success bool `json:"success"`
Count int `json:"count"`
@ -361,21 +359,15 @@ func (s *PrepSubsystem) planList(_ context.Context, _ *mcp.CallToolRequest, inpu
// --- Helpers ---
func planPath(dir, id string) string {
// Sanitise ID to prevent path traversal
safe := core.PathBase(id)
if safe == "." || safe == ".." || safe == "" {
safe = "invalid"
}
safe := core.SanitisePath(id)
return core.JoinPath(dir, core.Concat(safe, ".json"))
}
func generatePlanID(title string) string {
slug := sanitisePlanSlug(title)
// Append short random suffix for uniqueness
b := make([]byte, 3)
rand.Read(b)
return core.Concat(slug, "-", hex.EncodeToString(b))
// generatePlanID returns a Core ID for a plan file.
//
// id := generatePlanID("Migrate Core") // "id-1-a3f2b1"
func generatePlanID(_ string) string {
return core.ID()
}
// readPlanResult reads and decodes a plan file as core.Result.

View file

@ -43,7 +43,7 @@ func TestPlan_PlanCreate_Good(t *testing.T) {
require.NoError(t, err)
assert.True(t, out.Success)
assert.NotEmpty(t, out.ID)
assert.Contains(t, out.ID, "migrate-core")
assertCoreIDFormat(t, out.ID)
assert.NotEmpty(t, out.Path)
assert.True(t, fs.Exists(out.Path))
@ -367,8 +367,7 @@ func TestPlan_PlanCreate_Ugly_VeryLongTitle(t *testing.T) {
require.NoError(t, err)
assert.True(t, out.Success)
assert.NotEmpty(t, out.ID)
// The slug portion should be truncated
assert.LessOrEqual(t, len(out.ID), 50, "ID should be reasonably short")
assertCoreIDFormat(t, out.ID)
}
func TestPlan_PlanCreate_Ugly_UnicodeTitle(t *testing.T) {
@ -383,6 +382,7 @@ func TestPlan_PlanCreate_Ugly_UnicodeTitle(t *testing.T) {
require.NoError(t, err)
assert.True(t, out.Success)
assert.NotEmpty(t, out.ID)
assertCoreIDFormat(t, out.ID)
// Should be readable from disk
assert.True(t, fs.Exists(out.Path))
}
@ -498,8 +498,8 @@ func TestPlan_ValidPlanStatus_Ugly_UnicodeStatus(t *testing.T) {
}
func TestPlan_ValidPlanStatus_Ugly_NearMissStatus(t *testing.T) {
assert.False(t, validPlanStatus("Draft")) // capital D
assert.False(t, validPlanStatus("DRAFT")) // all caps
assert.False(t, validPlanStatus("Draft")) // capital D
assert.False(t, validPlanStatus("DRAFT")) // all caps
assert.False(t, validPlanStatus("in-progress")) // hyphen instead of underscore
assert.False(t, validPlanStatus(" draft")) // leading space
assert.False(t, validPlanStatus("draft ")) // trailing space
@ -508,16 +508,13 @@ func TestPlan_ValidPlanStatus_Ugly_NearMissStatus(t *testing.T) {
// --- generatePlanID Bad/Ugly ---
func TestPlan_GeneratePlanID_Bad(t *testing.T) {
// Empty title — slug will be empty, but random suffix is still appended
id := generatePlanID("")
assert.NotEmpty(t, id, "should still generate an ID with random suffix")
assert.Contains(t, id, "-", "should have random suffix separated by dash")
assertCoreIDFormat(t, id)
}
func TestPlan_GeneratePlanID_Ugly(t *testing.T) {
// Title with only special chars — slug will be empty
id := generatePlanID("!@#$%^&*()")
assert.NotEmpty(t, id, "should still generate an ID with random suffix")
assertCoreIDFormat(t, id)
}
// --- planList Bad/Ugly ---

View file

@ -19,7 +19,7 @@ func TestPlan_PlanPath_Good_BasicFormat(t *testing.T) {
}
func TestPlan_PlanPath_Good_NestedIDStripped(t *testing.T) {
// PathBase strips directory component — prevents path traversal
// SanitisePath strips directory components — prevents path traversal
result := planPath("/plans", "../../../etc/passwd")
assert.Equal(t, "/plans/passwd.json", result)
}

View file

@ -11,6 +11,16 @@ import (
"github.com/stretchr/testify/require"
)
func assertCoreIDFormat(t *testing.T, id string) {
t.Helper()
parts := strings.Split(id, "-")
if assert.Len(t, parts, 3) {
assert.Equal(t, "id", parts[0])
assert.NotEmpty(t, parts[1])
assert.NotEmpty(t, parts[2])
}
}
func TestPlan_PlanPath_Good(t *testing.T) {
assert.Equal(t, "/tmp/plans/my-plan-abc123.json", planPath("/tmp/plans", "my-plan-abc123"))
assert.Equal(t, "/data/test.json", planPath("/data", "test"))
@ -134,28 +144,19 @@ func TestPlan_WriteRead_Good_Roundtrip(t *testing.T) {
assert.Equal(t, "Working on it", read.Phases[1].Notes)
}
func TestPlan_GeneratePlanID_Good_Slugifies(t *testing.T) {
func TestPlan_GeneratePlanID_Good_CoreFormat(t *testing.T) {
id := generatePlanID("Add Unit Tests for Agentic")
assert.True(t, strings.HasPrefix(id, "add-unit-tests-for-agentic"), "got: %s", id)
// Should have random suffix
parts := strings.Split(id, "-")
assert.True(t, len(parts) >= 5, "expected slug with random suffix, got: %s", id)
assertCoreIDFormat(t, id)
}
func TestPlan_GeneratePlanID_Good_TruncatesLong(t *testing.T) {
func TestPlan_GeneratePlanID_Good_IgnoresTitleLength(t *testing.T) {
id := generatePlanID("This is a very long title that should be truncated to a reasonable length for file naming purposes")
// Slug part (before random suffix) should be <= 30 chars
lastDash := strings.LastIndex(id, "-")
slug := id[:lastDash]
assert.True(t, len(slug) <= 36, "slug too long: %s (%d chars)", slug, len(slug))
assertCoreIDFormat(t, id)
}
func TestPlan_GeneratePlanID_Good_HandlesSpecialChars(t *testing.T) {
func TestPlan_GeneratePlanID_Good_IgnoresSpecialChars(t *testing.T) {
id := generatePlanID("Fix bug #123: auth & session!")
assert.True(t, strings.Contains(id, "fix-bug"), "got: %s", id)
assert.NotContains(t, id, "#")
assert.NotContains(t, id, "!")
assert.NotContains(t, id, "&")
assertCoreIDFormat(t, id)
}
func TestPlan_GeneratePlanID_Good_Unique(t *testing.T) {

View file

@ -373,7 +373,19 @@ func workspaceDir(org, repo string, input PrepInput) (string, error) {
// r := workspaceDirResult("core", "go-io", PrepInput{Issue: 15})
// if r.OK { wsDir := r.Value.(string) }
func workspaceDirResult(org, repo string, input PrepInput) core.Result {
base := core.JoinPath(WorkspaceRoot(), org, repo)
orgName := core.ValidateName(org)
if !orgName.OK {
err, _ := orgName.Value.(error)
return core.Result{Value: core.E("workspaceDir", "invalid org name", err), OK: false}
}
repoName := core.ValidateName(repo)
if !repoName.OK {
err, _ := repoName.Value.(error)
return core.Result{Value: core.E("workspaceDir", "invalid repo name", err), OK: false}
}
base := core.JoinPath(WorkspaceRoot(), orgName.Value.(string), repoName.Value.(string))
switch {
case input.PR > 0:
return core.Result{Value: core.JoinPath(base, core.Sprintf("pr-%d", input.PR)), OK: true}
@ -417,12 +429,8 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques
metaDir := workspaceMetaDir(wsDir)
out := PrepOutput{WorkspaceDir: wsDir, RepoDir: repoDir}
// Source repo path — sanitise to prevent path traversal
repoName := core.PathBase(input.Repo)
if repoName == "." || repoName == ".." || repoName == "" {
return nil, PrepOutput{}, core.E("prep", core.Concat("invalid repo name: ", input.Repo), nil)
}
repoPath := core.JoinPath(s.codePath, input.Org, repoName)
// Source repo path — org and repo were validated by workspaceDirResult.
repoPath := core.JoinPath(s.codePath, input.Org, input.Repo)
// Ensure meta directory exists
if r := fs.EnsureDir(metaDir); !r.OK {

View file

@ -22,7 +22,7 @@ import (
// fs provides unrestricted filesystem access (root "/" = no sandbox).
//
// r := fs.Read(core.Concat(wsRoot, "/", name, "/status.json"))
// if text, ok := resultString(r); ok { json.Unmarshal([]byte(text), &st) }
// if text, ok := resultString(r); ok { _ = core.JSONUnmarshalString(text, &st) }
var fs = agentic.LocalFs()
type channelSender interface {