core/go-scm
8
0
Fork 0
Code Issues 4 Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(review): address CodeRabbit PR #2 findings

Browse source 
Critical/Major:
- Remove dead functions syncRepoNameFromArg and repoNameFromArg (used url pkg without import, would cause compile error)
- Migrate forge.lthn.ai/core/config → dappco.re/go/core/config in forge/config.go and gitea/config.go
- Propagate ListIssueCommentsIter errors in forge/meta.go and gitea/meta.go (was silently returning truncated count)
- Add RedactedToken() to gitea/client.go to avoid exposing raw API tokens
- Add 30s timeout to http.DefaultClient usage in gitea/prs.go via package-level httpClient
- Fix stringsx.Fields (bufio 64KiB limit), Repeat (wrong for negative/zero), Replace (ignored n param) to match stdlib
- Fix fmtx.Println to use fmt.Sprintln so spaces appear between operands
- Fix filepathx.Abs to use path/filepath for OS-aware path handling; wrap Getwd error
- Fix stdio.Write to return io.ErrShortWrite on partial writes
- Add mutex lock to jobrunner.Journal.Query to prevent data race with Append
- Add sync.RWMutex to ScmProvider; protect p.index reads/writes in pkg/api/provider.go
- Fix cmd/scm/cmd_index.go: append dir to repoPaths only after ReadDir confirms existence
- Fix manifest/compile.go: copy manifest before applying version override to avoid mutating caller
- Fix forge/labels.go: use ListOrgLabelsIter/ListRepoLabelsIter names in iterator error logs
- Wrap single-segment validation error in syncutil.ParseRepoName with function context

Minor:
- Fix import ordering (stdlib → forge.lthn.ai → third-party) in cmd/forge, cmd/collect, repos, cmd/gitea files
- Add t.Setenv("HOME", t.TempDir()) to gitea testhelpers and forge/labels_test.go
- Add iterator yield guard in forge/orgs_test.go
- Convert syncutil/repo_name_test.go to table-driven tests
- Use json.Marshal in pkg/api/provider_test.go instead of string concatenation
- Fix test naming (redundant/conflicting _Good/_Bad suffixes) across 10 test files

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-04-07 09:25:42 +01:00
parent e5e6698662
commit f3dd8ca0f0
46 changed files with 227 additions and 189 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
cmd/collect/cmd.go
View file

@ -3,12 +3,12 @@
package collect
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"forge.lthn.ai/core/cli/pkg/cli"
"dappco.re/go/core/i18n"
"dappco.re/go/core/io"
"dappco.re/go/core/scm/collect"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
func init() {

5
cmd/collect/cmd_dispatch.go
View file

@ -3,12 +3,13 @@
package collect
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"time"
"forge.lthn.ai/core/cli/pkg/cli"
"dappco.re/go/core/i18n"
collectpkg "dappco.re/go/core/scm/collect"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
// addDispatchCommand adds the 'dispatch' subcommand to the collect parent.

5
cmd/collect/cmd_excavate.go
View file

@ -4,11 +4,12 @@ package collect
import (
"context"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"forge.lthn.ai/core/cli/pkg/cli"
"dappco.re/go/core/i18n"
"dappco.re/go/core/scm/collect"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
// Excavate command flags

4
cmd/forge/cmd_auth.go
View file

@ -3,10 +3,10 @@
package forge
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"forge.lthn.ai/core/cli/pkg/cli"
fg "dappco.re/go/core/scm/forge"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
// Auth command flags.

6
cmd/forge/cmd_issues.go
View file

@ -3,13 +3,13 @@
package forge
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
"forge.lthn.ai/core/cli/pkg/cli"
forgejo "codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
fg "dappco.re/go/core/scm/forge"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
)
// Issues command flags.

6
cmd/forge/cmd_prs.go
View file

@ -3,13 +3,13 @@
package forge
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
"forge.lthn.ai/core/cli/pkg/cli"
forgejo "codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
fg "dappco.re/go/core/scm/forge"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
)
// PRs command flags.

4
cmd/forge/cmd_repos.go
View file

@ -3,12 +3,12 @@
package forge
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"forge.lthn.ai/core/cli/pkg/cli"
forgejo "codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
fg "dappco.re/go/core/scm/forge"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
// Repos command flags.

4
cmd/forge/cmd_status.go
View file

@ -3,10 +3,10 @@
package forge
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
"forge.lthn.ai/core/cli/pkg/cli"
fg "dappco.re/go/core/scm/forge"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
)
// addStatusCommand adds the 'status' subcommand for instance info.

23
cmd/forge/cmd_sync.go
View file

@ -349,26 +349,3 @@ func syncCreateMainFromUpstream(client *fg.Client, org, repo string) error {
return nil
}
func syncRepoNameFromArg(arg string) (string, error) {
decoded, err := url.PathUnescape(arg)
if err != nil {
return "", coreerr.E("forge.syncRepoNameFromArg", "decode repo argument", err)
}
parts := strings.Split(decoded, "/")
switch len(parts) {
case 1:
return agentci.ValidatePathElement(parts[0])
case 2:
if _, err := agentci.ValidatePathElement(parts[0]); err != nil {
return "", coreerr.E("forge.syncRepoNameFromArg", "invalid repo owner", err)
}
name, err := agentci.ValidatePathElement(parts[1])
if err != nil {
return "", coreerr.E("forge.syncRepoNameFromArg", "invalid repo name", err)
}
return name, nil
default:
return "", coreerr.E("forge.syncRepoNameFromArg", "repo argument must be repo or owner/repo", nil)
}
}

7
cmd/gitea/cmd_mirror.go
View file

@ -3,12 +3,13 @@
package gitea
import (
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
"forge.lthn.ai/core/cli/pkg/cli"
exec "golang.org/x/sys/execabs"
gt "dappco.re/go/core/scm/gitea"
"forge.lthn.ai/core/cli/pkg/cli"
fmt "dappco.re/go/core/scm/internal/ax/fmtx"
strings "dappco.re/go/core/scm/internal/ax/stringsx"
)
// Mirror command flags.

23
cmd/gitea/cmd_sync.go
View file

@ -367,26 +367,3 @@ func createMainFromUpstream(client *gt.Client, org, repo string) error {
func strPtr(s string) *string { return &s }
func repoNameFromArg(arg string) (string, error) {
decoded, err := url.PathUnescape(arg)
if err != nil {
return "", coreerr.E("gitea.repoNameFromArg", "decode repo argument", err)
}
parts := strings.Split(decoded, "/")
switch len(parts) {
case 1:
return agentci.ValidatePathElement(parts[0])
case 2:
if _, err := agentci.ValidatePathElement(parts[0]); err != nil {
return "", coreerr.E("gitea.repoNameFromArg", "invalid repo owner", err)
}
name, err := agentci.ValidatePathElement(parts[1])
if err != nil {
return "", coreerr.E("gitea.repoNameFromArg", "invalid repo name", err)
}
return name, nil
default:
return "", coreerr.E("gitea.repoNameFromArg", "repo argument must be repo or owner/repo", nil)
}
}

6
cmd/internal/syncutil/repo_name.go
View file

@ -21,7 +21,11 @@ func ParseRepoName(arg string) (string, error) {
parts := strings.Split(decoded, "/")
switch len(parts) {
case 1:
return agentci.ValidatePathElement(parts[0])
name, err := agentci.ValidatePathElement(parts[0])
if err != nil {
return "", coreerr.E("syncutil.ParseRepoName", "invalid repo name", err)
}
return name, nil
case 2:
if _, err := agentci.ValidatePathElement(parts[0]); err != nil {
return "", coreerr.E("syncutil.ParseRepoName", "invalid repo owner", err)

49
cmd/internal/syncutil/repo_name_test.go
View file

@ -10,25 +10,38 @@ import (
)
func TestParseRepoName_Good(t *testing.T) {
name, err := ParseRepoName("core")
require.NoError(t, err)
assert.Equal(t, "core", name)
tests := []struct {
name string
input string
want string
}{
{name: "RepoOnly", input: "core", want: "core"},
{name: "OwnerRepo", input: "host-uk/core", want: "core"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseRepoName(tt.input)
require.NoError(t, err)
assert.Equal(t, tt.want, got)
})
}
}
func TestParseRepoName_Good_OwnerRepo(t *testing.T) {
name, err := ParseRepoName("host-uk/core")
require.NoError(t, err)
assert.Equal(t, "core", name)
}
func TestParseRepoName_Bad(t *testing.T) {
tests := []struct {
name string
input string
}{
{name: "PathTraversal", input: "../escape"},
{name: "PathTraversalEncoded", input: "host-uk%2F..%2Fescape"},
}
func TestParseRepoName_Bad_PathTraversal(t *testing.T) {
_, err := ParseRepoName("../escape")
require.Error(t, err)
assert.Contains(t, err.Error(), "syncutil.ParseRepoName")
}
func TestParseRepoName_Bad_PathTraversalEncoded(t *testing.T) {
_, err := ParseRepoName("host-uk%2F..%2Fescape")
require.Error(t, err)
assert.Contains(t, err.Error(), "syncutil.ParseRepoName")
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := ParseRepoName(tt.input)
require.Error(t, err)
assert.Contains(t, err.Error(), "syncutil.ParseRepoName")
})
}
}

4
cmd/scm/cmd_index.go
View file

@ -75,8 +75,6 @@ func expandIndexRepoPaths(dirs []string) ([]string, error) {
var repoPaths []string
for _, dir := range dirs {
repoPaths = append(repoPaths, dir)
entries, err := os.ReadDir(dir)
if err != nil {
if os.IsNotExist(err) {
@ -85,6 +83,8 @@ func expandIndexRepoPaths(dirs []string) ([]string, error) {
return nil, cli.WrapVerb(err, "read", dir)
}
repoPaths = append(repoPaths, dir)
for _, entry := range entries {
if !entry.IsDir() {
continue

18
collect/process_test.go
View file

@ -15,7 +15,7 @@ func TestProcessor_Name_Good(t *testing.T) {
assert.Equal(t, "process:github", p.Name())
}
func TestProcessor_Process_Bad_NoDir_Good(t *testing.T) {
func TestProcessor_Process_NoDir_Bad(t *testing.T) {
m := io.NewMockMedium()
cfg := NewConfigWithMedium(m, "/output")
@ -24,7 +24,7 @@ func TestProcessor_Process_Bad_NoDir_Good(t *testing.T) {
assert.Error(t, err)
}
func TestProcessor_Process_Good_DryRun_Good(t *testing.T) {
func TestProcessor_Process_DryRun_Good(t *testing.T) {
m := io.NewMockMedium()
cfg := NewConfigWithMedium(m, "/output")
cfg.DryRun = true
@ -36,7 +36,7 @@ func TestProcessor_Process_Good_DryRun_Good(t *testing.T) {
assert.Equal(t, 0, result.Items)
}
func TestProcessor_Process_Good_HTMLFiles_Good(t *testing.T) {
func TestProcessor_Process_HTMLFiles_Good(t *testing.T) {
m := io.NewMockMedium()
m.Dirs["/input"] = true
m.Files["/input/page.html"] = `<html><body><h1>Hello</h1><p>World</p></body></html>`
@ -57,7 +57,7 @@ func TestProcessor_Process_Good_HTMLFiles_Good(t *testing.T) {
assert.Contains(t, content, "World")
}
func TestProcessor_Process_Good_JSONFiles_Good(t *testing.T) {
func TestProcessor_Process_JSONFiles_Good(t *testing.T) {
m := io.NewMockMedium()
m.Dirs["/input"] = true
m.Files["/input/data.json"] = `{"name": "Bitcoin", "price": 42000}`
@ -77,7 +77,7 @@ func TestProcessor_Process_Good_JSONFiles_Good(t *testing.T) {
assert.Contains(t, content, "Bitcoin")
}
func TestProcessor_Process_Good_MarkdownPassthrough_Good(t *testing.T) {
func TestProcessor_Process_MarkdownPassthrough_Good(t *testing.T) {
m := io.NewMockMedium()
m.Dirs["/input"] = true
m.Files["/input/readme.md"] = "# Already Markdown\n\nThis is already formatted."
@ -96,7 +96,7 @@ func TestProcessor_Process_Good_MarkdownPassthrough_Good(t *testing.T) {
assert.Contains(t, content, "# Already Markdown")
}
func TestProcessor_Process_Good_SkipUnknownTypes_Good(t *testing.T) {
func TestProcessor_Process_SkipUnknownTypes_Good(t *testing.T) {
m := io.NewMockMedium()
m.Dirs["/input"] = true
m.Files["/input/image.png"] = "binary data"
@ -172,7 +172,7 @@ func TestHTMLToMarkdown_Good(t *testing.T) {
}
}
func TestHTMLToMarkdown_Good_StripsScripts_Good(t *testing.T) {
func TestHTMLToMarkdown_StripsScripts_Good(t *testing.T) {
input := `<html><head><script>alert('xss')</script></head><body><p>Clean</p></body></html>`
result, err := HTMLToMarkdown(input)
assert.NoError(t, err)
@ -190,14 +190,14 @@ func TestJSONToMarkdown_Good(t *testing.T) {
assert.Contains(t, result, "42")
}
func TestJSONToMarkdown_Good_Array_Good(t *testing.T) {
func TestJSONToMarkdown_Array_Good(t *testing.T) {
input := `[{"id": 1}, {"id": 2}]`
result, err := JSONToMarkdown(input)
assert.NoError(t, err)
assert.Contains(t, result, "# Data")
}
func TestJSONToMarkdown_Bad_InvalidJSON_Good(t *testing.T) {
func TestJSONToMarkdown_InvalidJSON_Bad(t *testing.T) {
_, err := JSONToMarkdown("not json")
assert.Error(t, err)
}

8
collect/state_test.go
View file

@ -75,7 +75,7 @@ func TestState_SaveLoad_Good(t *testing.T) {
assert.True(t, now.Equal(got.LastRun))
}
func TestState_Load_Good_NoFile_Good(t *testing.T) {
func TestState_Load_NoFile_Good(t *testing.T) {
m := io.NewMockMedium()
s := NewState(m, "/nonexistent.json")
@ -88,7 +88,7 @@ func TestState_Load_Good_NoFile_Good(t *testing.T) {
assert.False(t, ok)
}
func TestState_Load_Bad_InvalidJSON_Good(t *testing.T) {
func TestState_Load_InvalidJSON_Bad(t *testing.T) {
m := io.NewMockMedium()
m.Files["/state.json"] = "not valid json"
@ -97,7 +97,7 @@ func TestState_Load_Bad_InvalidJSON_Good(t *testing.T) {
assert.Error(t, err)
}
func TestState_SaveLoad_Good_MultipleEntries_Good(t *testing.T) {
func TestState_SaveLoad_MultipleEntries_Good(t *testing.T) {
m := io.NewMockMedium()
s := NewState(m, "/state.json")
@ -125,7 +125,7 @@ func TestState_SaveLoad_Good_MultipleEntries_Good(t *testing.T) {
assert.Equal(t, 30, c.Items)
}
func TestState_Set_Good_Overwrite_Good(t *testing.T) {
func TestState_Set_Overwrite_Good(t *testing.T) {
m := io.NewMockMedium()
s := NewState(m, "/state.json")

2
forge/config.go
View file

@ -6,7 +6,7 @@ import (
os "dappco.re/go/core/scm/internal/ax/osx"
"dappco.re/go/core/log"
"forge.lthn.ai/core/config"
"dappco.re/go/core/config"
)
const (

6
forge/labels.go
View file

@ -58,13 +58,13 @@ func (c *Client) ListOrgLabelsIter(org string) iter.Seq2[*forgejo.Label, error]
for repo, err := range c.ListOrgReposIter(org) {
if err != nil {
yield(nil, log.E("forge.ListOrgLabels", "failed to list org repos", err))
yield(nil, log.E("forge.ListOrgLabelsIter", "failed to list org repos", err))
return
}
for label, err := range c.ListRepoLabelsIter(repo.Owner.UserName, repo.Name) {
if err != nil {
yield(nil, log.E("forge.ListOrgLabels", "failed to list repo labels", err))
yield(nil, log.E("forge.ListOrgLabelsIter", "failed to list repo labels", err))
return
}
@ -118,7 +118,7 @@ func (c *Client) ListRepoLabelsIter(owner, repo string) iter.Seq2[*forgejo.Label
ListOptions: forgejo.ListOptions{Page: page, PageSize: 50},
})
if err != nil {
yield(nil, log.E("forge.ListRepoLabels", "failed to list repo labels", err))
yield(nil, log.E("forge.ListRepoLabelsIter", "failed to list repo labels", err))
return
}

2
forge/labels_test.go
View file

@ -38,6 +38,8 @@ func TestClient_ListRepoLabels_Bad_ServerError_Good(t *testing.T) {
}
func TestClient_ListRepoLabelsIter_Good_Paginates_Good(t *testing.T) {
t.Setenv("HOME", t.TempDir())
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/version", func(w http.ResponseWriter, r *http.Request) {
jsonResponse(w, map[string]string{"version": "1.21.0"})

2
forge/meta.go
View file

@ -78,7 +78,7 @@ func (c *Client) GetPRMeta(owner, repo string, pr int64) (*PRMeta, error) {
count := 0
for _, err := range c.ListIssueCommentsIter(owner, repo, pr) {
if err != nil {
break
return nil, log.E("forge.GetPRMeta", "list issue comments", err)
}
count++
}

3
forge/orgs_test.go
View file

@ -71,11 +71,14 @@ func TestClient_ListMyOrgsIter_Bad_ServerError_Good(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
seen := false
for _, err := range client.ListMyOrgsIter() {
seen = true
assert.Error(t, err)
assert.Contains(t, err.Error(), "failed to list orgs")
break
}
require.True(t, seen, "expected ListMyOrgsIter to yield an error")
}
func TestClient_GetOrg_Good(t *testing.T) {

12
gitea/client.go
View file

@ -42,10 +42,20 @@ func (c *Client) API() *gitea.Client { return c.api }
// Usage: URL(...)
func (c *Client) URL() string { return c.url }
// Token returns the Gitea API token.
// Token returns the Gitea API token for use in HTTP Authorization headers.
// The token is used internally and should not be logged or exposed externally.
// Usage: Token(...)
func (c *Client) Token() string { return c.token }
// RedactedToken returns a redacted representation of the API token for safe logging.
// Usage: RedactedToken(...)
func (c *Client) RedactedToken() string {
if len(c.token) <= 8 {
return "***"
}
return c.token[:4] + "****" + c.token[len(c.token)-4:]
}
// GetCurrentUser returns the authenticated user's information.
// Usage: GetCurrentUser(...)
func (c *Client) GetCurrentUser() (*gitea.User, error) {

2
gitea/config.go
View file

@ -6,7 +6,7 @@ import (
os "dappco.re/go/core/scm/internal/ax/osx"
"dappco.re/go/core/log"
"forge.lthn.ai/core/config"
"dappco.re/go/core/config"
)
const (

2
gitea/meta.go
View file

@ -78,7 +78,7 @@ func (c *Client) GetPRMeta(owner, repo string, pr int64) (*PRMeta, error) {
count := 0
for _, err := range c.ListIssueCommentsIter(owner, repo, pr) {
if err != nil {
break
return nil, log.E("gitea.GetPRMeta", "list issue comments", err)
}
count++
}

6
gitea/prs.go
View file

@ -8,6 +8,7 @@ import (
"net/http"
"net/url"
"strconv"
"time"
"code.gitea.io/sdk/gitea"
@ -16,6 +17,9 @@ import (
"dappco.re/go/core/scm/internal/ax/jsonx"
)
// httpClient is a package-level client with a timeout to avoid hanging indefinitely.
var httpClient = &http.Client{Timeout: 30 * time.Second}
// MergePullRequest merges a pull request with the given method ("squash", "rebase", "merge").
// Usage: MergePullRequest(...)
func (c *Client) MergePullRequest(owner, repo string, index int64, method string) error {
@ -74,7 +78,7 @@ func (c *Client) SetPRDraft(owner, repo string, index int64, draft bool) error {
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Authorization", "token "+c.Token())
resp, err := http.DefaultClient.Do(req)
resp, err := httpClient.Do(req)
if err != nil {
return log.E("gitea.SetPRDraft", "failed to update draft status", err)
}

16
gitea/repos_test.go
View file

@ -24,7 +24,7 @@ func TestClient_ListOrgRepos_Good(t *testing.T) {
assert.Equal(t, "org-repo", repos[0].Name)
}
func TestClient_ListOrgRepos_Bad_ServerError_Good(t *testing.T) {
func TestClient_ListOrgRepos_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
@ -44,7 +44,7 @@ func TestClient_ListUserRepos_Good(t *testing.T) {
assert.Equal(t, "repo-b", repos[1].Name)
}
func TestClient_ListUserRepos_Bad_ServerError_Good(t *testing.T) {
func TestClient_ListUserRepos_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
@ -62,7 +62,7 @@ func TestClient_GetRepo_Good(t *testing.T) {
assert.Equal(t, "org-repo", repo.Name)
}
func TestClient_GetRepo_Bad_ServerError_Good(t *testing.T) {
func TestClient_GetRepo_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
@ -71,7 +71,7 @@ func TestClient_GetRepo_Bad_ServerError_Good(t *testing.T) {
assert.Contains(t, err.Error(), "failed to get repo")
}
func TestClient_CreateMirror_Good_WithAuth_Good(t *testing.T) {
func TestClient_CreateMirror_WithAuth_Good(t *testing.T) {
client, srv := newTestClient(t)
defer srv.Close()
@ -81,7 +81,7 @@ func TestClient_CreateMirror_Good_WithAuth_Good(t *testing.T) {
assert.NotNil(t, repo)
}
func TestClient_CreateMirror_Bad_NoAuthToken_Good(t *testing.T) {
func TestClient_CreateMirror_NoAuthToken_Bad(t *testing.T) {
client, srv := newTestClient(t)
defer srv.Close()
@ -91,7 +91,7 @@ func TestClient_CreateMirror_Bad_NoAuthToken_Good(t *testing.T) {
assert.Contains(t, err.Error(), "failed to create mirror")
}
func TestClient_CreateMirror_Bad_ServerError_Good(t *testing.T) {
func TestClient_CreateMirror_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
@ -140,7 +140,7 @@ func TestClient_DeleteRepo_Good(t *testing.T) {
require.NoError(t, err)
}
func TestClient_DeleteRepo_Bad_ServerError_Good(t *testing.T) {
func TestClient_DeleteRepo_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()
@ -161,7 +161,7 @@ func TestClient_CreateOrgRepo_Good(t *testing.T) {
assert.NotNil(t, repo)
}
func TestClient_CreateOrgRepo_Bad_ServerError_Good(t *testing.T) {
func TestClient_CreateOrgRepo_ServerError_Bad(t *testing.T) {
client, srv := newErrorServer(t)
defer srv.Close()

2
gitea/testhelper_test.go
View file

@ -210,6 +210,7 @@ func jsonResponse(w http.ResponseWriter, data any) {
// newTestClient creates a Client backed by the mock server.
func newTestClient(t *testing.T) (*Client, *httptest.Server) {
t.Helper()
t.Setenv("HOME", t.TempDir())
srv := newMockGiteaServer(t)
client, err := New(srv.URL, "test-token")
@ -224,6 +225,7 @@ func newTestClient(t *testing.T) (*Client, *httptest.Server) {
// newErrorServer creates a mock server that returns errors for all API calls.
func newErrorServer(t *testing.T) (*Client, *httptest.Server) {
t.Helper()
t.Setenv("HOME", t.TempDir())
mux := http.NewServeMux()
mux.HandleFunc("/api/v1/version", func(w http.ResponseWriter, r *http.Request) {

10
internal/ax/filepathx/filepathx.go
View file

@ -3,7 +3,9 @@
package filepathx
import (
"fmt"
"path"
"path/filepath"
"syscall"
)
@ -13,14 +15,14 @@ const Separator = '/'
// Abs mirrors filepath.Abs for the paths used in this repo.
// Usage: Abs(...)
func Abs(p string) (string, error) {
if path.IsAbs(p) {
return path.Clean(p), nil
if filepath.IsAbs(p) {
return filepath.Clean(p), nil
}
cwd, err := syscall.Getwd()
if err != nil {
return "", err
return "", fmt.Errorf("filepathx.Abs: %w", err)
}
return path.Clean(path.Join(cwd, p)), nil
return filepath.Clean(filepath.Join(cwd, p)), nil
}
// Base mirrors filepath.Base.

11
internal/ax/fmtx/fmtx.go
View file

@ -3,6 +3,7 @@
package fmtx
import (
"fmt"
"io"
core "dappco.re/go/core"
@ -33,8 +34,14 @@ func Printf(format string, args ...any) (int, error) {
return Fprintf(stdio.Stdout, format, args...)
}
// Println mirrors fmt.Println.
// Sprintln mirrors fmt.Sprintln — spaces between operands, trailing newline.
// Usage: Sprintln(...)
func Sprintln(args ...any) string {
return fmt.Sprintln(args...)
}
// Println mirrors fmt.Println — spaces between operands, trailing newline.
// Usage: Println(...)
func Println(args ...any) (int, error) {
return io.WriteString(stdio.Stdout, Sprint(args...)+"\n")
return io.WriteString(stdio.Stdout, Sprintln(args...))
}

6
internal/ax/stdio/stdio.go
View file

@ -28,7 +28,11 @@ type fdWriter struct {
// Write implements io.Writer for stdout and stderr without importing os.
// Usage: Write(...)
func (w fdWriter) Write(p []byte) (int, error) {
return syscall.Write(w.fd, p)
n, err := syscall.Write(w.fd, p)
if n < len(p) && err == nil {
return n, io.ErrShortWrite
}
return n, err
}
// Stdin exposes process stdin without importing os.

21
internal/ax/stringsx/stringsx.go
View file

@ -3,9 +3,9 @@
package stringsx
import (
"bufio"
"bytes"
"iter"
"strings"
core "dappco.re/go/core"
)
@ -34,13 +34,7 @@ func EqualFold(s, t string) bool {
// Fields mirrors strings.Fields.
// Usage: Fields(...)
func Fields(s string) []string {
scanner := bufio.NewScanner(NewReader(s))
scanner.Split(bufio.ScanWords)
fields := make([]string, 0)
for scanner.Scan() {
fields = append(fields, scanner.Text())
}
return fields
return strings.Fields(s)
}
// HasPrefix mirrors strings.HasPrefix.
@ -76,10 +70,7 @@ func NewReader(s string) *bytes.Reader {
// Repeat mirrors strings.Repeat.
// Usage: Repeat(...)
func Repeat(s string, count int) string {
if count <= 0 {
return ""
}
return string(bytes.Repeat([]byte(s), count))
return strings.Repeat(s, count)
}
// ReplaceAll mirrors strings.ReplaceAll.
@ -88,10 +79,10 @@ func ReplaceAll(s, old, new string) string {
return core.Replace(s, old, new)
}
// Replace mirrors strings.Replace for replace-all call sites.
// Replace mirrors strings.Replace.
// Usage: Replace(...)
func Replace(s, old, new string, _ int) string {
return ReplaceAll(s, old, new)
func Replace(s, old, new string, n int) string {
return strings.Replace(s, old, new, n)
}
// Split mirrors strings.Split.

3
jobrunner/journal.go
View file

@ -206,6 +206,9 @@ func (j *Journal) Query(opts JournalQueryOptions) ([]JournalEntry, error) {
return nil, coreerr.E("jobrunner.Journal.Query", "journal is required", nil)
}
j.mu.Lock()
defer j.mu.Unlock()
ownerFilter, repoFilter, err := normaliseJournalQueryRepo(opts)
if err != nil {
return nil, coreerr.E("jobrunner.Journal.Query", "normalise repo filter", err)

6
jobrunner/poller_test.go
View file

@ -120,7 +120,7 @@ func TestPoller_RunOnce_Good(t *testing.T) {
assert.Equal(t, 1, p.Cycle())
}
func TestPoller_RunOnce_Good_NoSignals_Good(t *testing.T) {
func TestPoller_RunOnce_NoSignals_Good(t *testing.T) {
src := &mockSource{
name: "empty-source",
signals: nil,
@ -151,7 +151,7 @@ func TestPoller_RunOnce_Good_NoSignals_Good(t *testing.T) {
assert.Equal(t, 1, p.Cycle())
}
func TestPoller_RunOnce_Good_NoMatchingHandler_Good(t *testing.T) {
func TestPoller_RunOnce_NoMatchingHandler_Good(t *testing.T) {
sig := &PipelineSignal{
EpicNumber: 5,
ChildNumber: 8,
@ -192,7 +192,7 @@ func TestPoller_RunOnce_Good_NoMatchingHandler_Good(t *testing.T) {
assert.Empty(t, src.reports)
}
func TestPoller_RunOnce_Good_DryRun_Good(t *testing.T) {
func TestPoller_RunOnce_DryRun_Good(t *testing.T) {
sig := &PipelineSignal{
EpicNumber: 1,
ChildNumber: 3,

33
jobrunner/types_test.go
View file

@ -27,19 +27,32 @@ func TestPipelineSignal_HasUnresolvedThreads_Good(t *testing.T) {
assert.True(t, sig.HasUnresolvedThreads())
}
func TestPipelineSignal_HasUnresolvedThreads_Bad_AllResolved_Good(t *testing.T) {
sig := &PipelineSignal{
ThreadsTotal: 4,
ThreadsResolved: 4,
func TestPipelineSignal_HasUnresolvedThreads_Bad(t *testing.T) {
tests := []struct {
name string
sig *PipelineSignal
}{
{
name: "AllResolved",
sig: &PipelineSignal{
ThreadsTotal: 4,
ThreadsResolved: 4,
},
},
{
name: "ZeroThreads",
sig: &PipelineSignal{
ThreadsTotal: 0,
ThreadsResolved: 0,
},
},
}
assert.False(t, sig.HasUnresolvedThreads())
// Also verify zero threads is not unresolved.
sigZero := &PipelineSignal{
ThreadsTotal: 0,
ThreadsResolved: 0,
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.False(t, tt.sig.HasUnresolvedThreads())
})
}
assert.False(t, sigZero.HasUnresolvedThreads())
}
func TestActionResult_JSON_Good(t *testing.T) {

5
manifest/compile.go
View file

@ -48,9 +48,12 @@ func Compile(m *Manifest, opts CompileOptions) (*CompiledManifest, error) {
return nil, coreerr.E("manifest.Compile", "missing version", nil)
}
// Work on a copy to avoid mutating the caller's manifest.
mCopy := *m
if opts.Version != "" {
m.Version = opts.Version
mCopy.Version = opts.Version
}
m = &mCopy
// Sign if a key is supplied.
if opts.SignKey != nil {

2
marketplace/marketplace_test.go
View file

@ -60,7 +60,7 @@ func TestFind_Good(t *testing.T) {
assert.Equal(t, "XMRig", m.Name)
}
func TestFind_Bad_NotFound_Good(t *testing.T) {
func TestFind_NotFound_Good(t *testing.T) {
idx := &Index{}
_, ok := idx.Find("nope")
assert.False(t, ok)

33
pkg/api/provider.go
View file

@ -12,6 +12,7 @@ import (
"net/http"
"net/url"
"strings"
"sync"
"dappco.re/go/core/api"
"dappco.re/go/core/api/pkg/provider"
@ -28,6 +29,7 @@ import (
// as a service provider. It implements Provider, Streamable, Describable,
// and Renderable.
type ScmProvider struct {
mu sync.RWMutex
index *marketplace.Index
installer marketplaceInstaller
registry *repos.Registry
@ -230,7 +232,11 @@ func (p *ScmProvider) Describe() []api.RouteDescription {
// -- Marketplace Handlers -----------------------------------------------------
func (p *ScmProvider) listMarketplace(c *gin.Context) {
if p.index == nil {
p.mu.RLock()
idx := p.index
p.mu.RUnlock()
if idx == nil {
c.JSON(http.StatusOK, api.OK([]marketplace.Module{}))
return
}
@ -238,9 +244,9 @@ func (p *ScmProvider) listMarketplace(c *gin.Context) {
query := c.Query("q")
category := c.Query("category")
modules := p.index.Modules
modules := idx.Modules
if category != "" {
modules = p.index.ByCategory(category)
modules = idx.ByCategory(category)
}
if query != "" {
filtered := make([]marketplace.Module, 0, len(modules))
@ -259,7 +265,11 @@ func (p *ScmProvider) listMarketplace(c *gin.Context) {
}
func (p *ScmProvider) getMarketplaceItem(c *gin.Context) {
if p.index == nil {
p.mu.RLock()
idx := p.index
p.mu.RUnlock()
if idx == nil {
c.JSON(http.StatusNotFound, api.Fail("not_found", "marketplace index not loaded"))
return
}
@ -268,7 +278,7 @@ func (p *ScmProvider) getMarketplaceItem(c *gin.Context) {
if !ok {
return
}
mod, ok := p.index.Find(code)
mod, ok := idx.Find(code)
if !ok {
c.JSON(http.StatusNotFound, api.Fail("not_found", "provider not found in marketplace"))
return
@ -277,7 +287,12 @@ func (p *ScmProvider) getMarketplaceItem(c *gin.Context) {
}
func (p *ScmProvider) installItem(c *gin.Context) {
if p.index == nil || p.installer == nil {
p.mu.RLock()
idx := p.index
inst := p.installer
p.mu.RUnlock()
if idx == nil || inst == nil {
c.JSON(http.StatusServiceUnavailable, api.Fail("unavailable", "marketplace not configured"))
return
}
@ -286,13 +301,13 @@ func (p *ScmProvider) installItem(c *gin.Context) {
if !ok {
return
}
mod, ok := p.index.Find(code)
mod, ok := idx.Find(code)
if !ok {
c.JSON(http.StatusNotFound, api.Fail("not_found", "provider not found in marketplace"))
return
}
if err := p.installer.Install(context.Background(), mod); err != nil {
if err := inst.Install(context.Background(), mod); err != nil {
c.JSON(http.StatusInternalServerError, api.Fail("install_failed", err.Error()))
return
}
@ -356,7 +371,9 @@ func (p *ScmProvider) refreshMarketplace(c *gin.Context) {
return
}
p.mu.Lock()
p.index = idx
p.mu.Unlock()
p.emitEvent("scm.marketplace.refreshed", map[string]any{
"index_path": req.IndexPath,
"modules": len(idx.Modules),

5
pkg/api/provider_test.go
View file

@ -268,7 +268,8 @@ func TestScmProvider_RefreshMarketplace_Good(t *testing.T) {
r := setupRouter(p)
w := httptest.NewRecorder()
body := []byte(`{"index_path":"` + indexPath + `"}`)
body, err := json.Marshal(map[string]string{"index_path": indexPath})
require.NoError(t, err)
req, _ := http.NewRequest("POST", "/api/v1/scm/marketplace/refresh", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
r.ServeHTTP(w, req)
@ -276,7 +277,7 @@ func TestScmProvider_RefreshMarketplace_Good(t *testing.T) {
assert.Equal(t, http.StatusOK, w.Code)
var resp goapi.Response[map[string]any]
err := json.Unmarshal(w.Body.Bytes(), &resp)
err = json.Unmarshal(w.Body.Bytes(), &resp)
require.NoError(t, err)
assert.True(t, resp.Success)
assert.Equal(t, float64(1), resp.Data["modules"])

28
plugin/installer_test.go
View file

@ -25,7 +25,7 @@ func TestNewInstaller_Good(t *testing.T) {
// ── Install error paths ────────────────────────────────────────────
func TestInstall_Bad_InvalidSource_Good(t *testing.T) {
func TestInstall_InvalidSource_Bad(t *testing.T) {
m := io.NewMockMedium()
reg := NewRegistry(m, "/plugins")
inst := NewInstaller(m, reg)
@ -35,7 +35,7 @@ func TestInstall_Bad_InvalidSource_Good(t *testing.T) {
assert.Contains(t, err.Error(), "invalid source")
}
func TestInstall_Bad_AlreadyInstalled_Good(t *testing.T) {
func TestInstall_AlreadyInstalled_Bad(t *testing.T) {
m := io.NewMockMedium()
reg := NewRegistry(m, "/plugins")
_ = reg.Add(&PluginConfig{Name: "my-plugin", Version: "1.0.0"})
@ -80,7 +80,7 @@ func TestRemove_Good(t *testing.T) {
assert.False(t, m.Exists("/plugins/removable"))
}
func TestRemove_Good_DirAlreadyGone_Good(t *testing.T) {
func TestRemove_DirAlreadyGone_Good(t *testing.T) {
m := io.NewMockMedium()
reg := NewRegistry(m, "/plugins")
_ = reg.Add(&PluginConfig{Name: "ghost", Version: "1.0.0"})
@ -94,7 +94,7 @@ func TestRemove_Good_DirAlreadyGone_Good(t *testing.T) {
assert.False(t, ok)
}
func TestRemove_Bad_NotFound_Good(t *testing.T) {
func TestRemove_NotFound_Bad(t *testing.T) {
m := io.NewMockMedium()
reg := NewRegistry(m, "/plugins")
inst := NewInstaller(m, reg)
@ -119,7 +119,7 @@ func TestRemove_Bad_PathTraversalName(t *testing.T) {
// ── Update error paths ─────────────────────────────────────────────
func TestUpdate_Bad_NotFound_Good(t *testing.T) {
func TestUpdate_NotFound_Bad(t *testing.T) {
m := io.NewMockMedium()
reg := NewRegistry(m, "/plugins")
inst := NewInstaller(m, reg)
@ -131,7 +131,7 @@ func TestUpdate_Bad_NotFound_Good(t *testing.T) {
// ── ParseSource ────────────────────────────────────────────────────
func TestParseSource_Good_OrgRepo_Good(t *testing.T) {
func TestParseSource_OrgRepo_Good(t *testing.T) {
org, repo, version, err := ParseSource("host-uk/core-plugin")
assert.NoError(t, err)
assert.Equal(t, "host-uk", org)
@ -139,7 +139,7 @@ func TestParseSource_Good_OrgRepo_Good(t *testing.T) {
assert.Equal(t, "", version)
}
func TestParseSource_Good_OrgRepoVersion_Good(t *testing.T) {
func TestParseSource_OrgRepoVersion_Good(t *testing.T) {
org, repo, version, err := ParseSource("host-uk/core-plugin@v1.0.0")
assert.NoError(t, err)
assert.Equal(t, "host-uk", org)
@ -147,7 +147,7 @@ func TestParseSource_Good_OrgRepoVersion_Good(t *testing.T) {
assert.Equal(t, "v1.0.0", version)
}
func TestParseSource_Good_VersionWithoutPrefix_Good(t *testing.T) {
func TestParseSource_VersionWithoutPrefix_Good(t *testing.T) {
org, repo, version, err := ParseSource("org/repo@1.2.3")
assert.NoError(t, err)
assert.Equal(t, "org", org)
@ -155,37 +155,37 @@ func TestParseSource_Good_VersionWithoutPrefix_Good(t *testing.T) {
assert.Equal(t, "1.2.3", version)
}
func TestParseSource_Bad_Empty_Good(t *testing.T) {
func TestParseSource_Empty_Bad(t *testing.T) {
_, _, _, err := ParseSource("")
assert.Error(t, err)
assert.Contains(t, err.Error(), "source is empty")
}
func TestParseSource_Bad_NoSlash_Good(t *testing.T) {
func TestParseSource_NoSlash_Bad(t *testing.T) {
_, _, _, err := ParseSource("just-a-name")
assert.Error(t, err)
assert.Contains(t, err.Error(), "org/repo")
}
func TestParseSource_Bad_TooManySlashes_Good(t *testing.T) {
func TestParseSource_TooManySlashes_Bad(t *testing.T) {
_, _, _, err := ParseSource("a/b/c")
assert.Error(t, err)
assert.Contains(t, err.Error(), "org/repo")
}
func TestParseSource_Bad_EmptyOrg_Good(t *testing.T) {
func TestParseSource_EmptyOrg_Bad(t *testing.T) {
_, _, _, err := ParseSource("/repo")
assert.Error(t, err)
assert.Contains(t, err.Error(), "org/repo")
}
func TestParseSource_Bad_EmptyRepo_Good(t *testing.T) {
func TestParseSource_EmptyRepo_Bad(t *testing.T) {
_, _, _, err := ParseSource("org/")
assert.Error(t, err)
assert.Contains(t, err.Error(), "org/repo")
}
func TestParseSource_Bad_EmptyVersion_Good(t *testing.T) {
func TestParseSource_EmptyVersion_Bad(t *testing.T) {
_, _, _, err := ParseSource("org/repo@")
assert.Error(t, err)
assert.Contains(t, err.Error(), "version is empty")

4
plugin/loader_test.go
View file

@ -122,7 +122,7 @@ func TestLoader_LoadPlugin_Good(t *testing.T) {
assert.Equal(t, "1.0.0", manifest.Version)
}
func TestLoader_LoadPlugin_Bad_NotFound_Good(t *testing.T) {
func TestLoader_LoadPlugin_Bad_NotFound_Bad(t *testing.T) {
m := io.NewMockMedium()
loader := NewLoader(m, "/home/user/.core/plugins")
@ -131,7 +131,7 @@ func TestLoader_LoadPlugin_Bad_NotFound_Good(t *testing.T) {
assert.Contains(t, err.Error(), "failed to load plugin")
}
func TestLoader_LoadPlugin_Bad_InvalidManifest_Good(t *testing.T) {
func TestLoader_LoadPlugin_Bad_InvalidManifest_Bad(t *testing.T) {
m := io.NewMockMedium()
baseDir := "/home/user/.core/plugins"

4
plugin/plugin_test.go
View file

@ -24,7 +24,7 @@ func TestBasePlugin_Good(t *testing.T) {
assert.NoError(t, p.Stop(ctx))
}
func TestBasePlugin_Good_EmptyFields_Good(t *testing.T) {
func TestBasePlugin_EmptyFields_Good(t *testing.T) {
p := &BasePlugin{}
assert.Equal(t, "", p.Name())
@ -36,6 +36,6 @@ func TestBasePlugin_Good_EmptyFields_Good(t *testing.T) {
assert.NoError(t, p.Stop(ctx))
}
func TestBasePlugin_Good_ImplementsPlugin_Good(t *testing.T) {
func TestBasePlugin_ImplementsPlugin_Good(t *testing.T) {
var _ Plugin = &BasePlugin{}
}

3
repos/gitstate.go
View file

@ -3,9 +3,10 @@
package repos
import (
filepath "dappco.re/go/core/scm/internal/ax/filepathx"
"time"
filepath "dappco.re/go/core/scm/internal/ax/filepathx"
"dappco.re/go/core/io"
coreerr "dappco.re/go/core/log"
"gopkg.in/yaml.v3"

2
repos/gitstate_test.go
View file

@ -60,7 +60,7 @@ func TestGitState_Load_Good_NoFile_Good(t *testing.T) {
assert.Empty(t, gs.Agents)
}
func TestGitState_Load_Bad_InvalidYAML_Good(t *testing.T) {
func TestGitState_Load_Bad_InvalidYAML_Bad(t *testing.T) {
m := io.NewMockMedium()
_ = m.Write("/workspace/.core/git.yaml", "{{{{not yaml")

2
repos/kbconfig_test.go
View file

@ -75,7 +75,7 @@ search:
assert.Equal(t, "embeddinggemma", kb.Search.EmbedModel)
}
func TestKBConfig_Load_Bad_InvalidYAML_Good(t *testing.T) {
func TestKBConfig_Load_Bad_InvalidYAML_Bad(t *testing.T) {
m := io.NewMockMedium()
_ = m.Write("/workspace/.core/kb.yaml", "{{{{broken")

3
repos/workconfig.go
View file

@ -3,9 +3,10 @@
package repos
import (
filepath "dappco.re/go/core/scm/internal/ax/filepathx"
"time"
filepath "dappco.re/go/core/scm/internal/ax/filepathx"
"dappco.re/go/core/io"
coreerr "dappco.re/go/core/log"
"gopkg.in/yaml.v3"

4
repos/workconfig_test.go
View file

@ -78,7 +78,7 @@ sync:
assert.True(t, wc.Sync.CloneMissing)
}
func TestWorkConfig_Load_Bad_InvalidYAML_Good(t *testing.T) {
func TestWorkConfig_Load_Bad_InvalidYAML_Bad(t *testing.T) {
m := io.NewMockMedium()
_ = m.Write("/workspace/.core/work.yaml", "{{{{broken")
@ -96,7 +96,7 @@ func TestWorkConfig_HasTrigger_Good(t *testing.T) {
assert.True(t, wc.HasTrigger("scheduled"))
}
func TestWorkConfig_HasTrigger_Bad_NotFound_Good(t *testing.T) {
func TestWorkConfig_HasTrigger_NotFound_Bad(t *testing.T) {
wc := DefaultWorkConfig()
assert.False(t, wc.HasTrigger("on_deploy"))
}