fix(coderabbit): address review findings
1. forge/labels: add ErrLabelNotFound sentinel, EnsureLabel only creates on not-found 2. handlers/tick_parent: add comment explaining coreerr alias for go-log 3. repos/registry: wrap raw os errors with coreerr.E() in FindRegistry 4. go.mod: bump go 1.26.0 → 1.26.1 5. cmd/scm: pass locales.FS to cli.RegisterCommands 6. marketplace/builder: add Medium field, replace coreio.Local in loadFromDir/WriteIndex 7. marketplace/installer: capture and return DeleteAll error in Remove 8. cmd/scm/export: wrap manifest.Compile error with cli.WrapVerb 9. handlers/dispatch: add RepoOwner/RepoName/EpicNumber to early-success return 10. cmd/scm/compile: validate ed25519 key length before casting Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
9de597a8b0
commit
071eca4214
12 changed files with 63 additions and 23 deletions
|
|
@ -3,6 +3,7 @@ package scm
|
|||
import (
|
||||
"crypto/ed25519"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"os/exec"
|
||||
"strings"
|
||||
|
||||
|
|
@ -56,6 +57,12 @@ func runCompile(dir, signKeyHex, builtBy string) error {
|
|||
if err != nil {
|
||||
return cli.WrapVerb(err, "decode", "sign key")
|
||||
}
|
||||
if len(keyBytes) != ed25519.PrivateKeySize {
|
||||
return cli.WrapVerb(
|
||||
fmt.Errorf("expected %d bytes, got %d", ed25519.PrivateKeySize, len(keyBytes)),
|
||||
"validate", "sign key length",
|
||||
)
|
||||
}
|
||||
opts.SignKey = ed25519.PrivateKey(keyBytes)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ func runExport(dir string) error {
|
|||
BuiltBy: "core scm export",
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
return cli.WrapVerb(err, "compile", "manifest")
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import (
|
|||
"path/filepath"
|
||||
|
||||
"forge.lthn.ai/core/cli/pkg/cli"
|
||||
coreio "forge.lthn.ai/core/go-io"
|
||||
"forge.lthn.ai/core/go-scm/marketplace"
|
||||
)
|
||||
|
||||
|
|
@ -48,7 +49,7 @@ func runIndex(dirs []string, output, baseURL, org string) error {
|
|||
}
|
||||
|
||||
absOutput, _ := filepath.Abs(output)
|
||||
if err := marketplace.WriteIndex(absOutput, idx); err != nil {
|
||||
if err := marketplace.WriteIndex(coreio.Local, absOutput, idx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -9,10 +9,11 @@ package scm
|
|||
|
||||
import (
|
||||
"forge.lthn.ai/core/cli/pkg/cli"
|
||||
"forge.lthn.ai/core/go-scm/locales"
|
||||
)
|
||||
|
||||
func init() {
|
||||
cli.RegisterCommands(AddScmCommands)
|
||||
cli.RegisterCommands(AddScmCommands, locales.FS)
|
||||
}
|
||||
|
||||
// Style aliases from shared package.
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package forge
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
|
||||
forgejo "codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
|
||||
|
|
@ -8,6 +9,9 @@ import (
|
|||
"forge.lthn.ai/core/go-log"
|
||||
)
|
||||
|
||||
// ErrLabelNotFound is returned when a label cannot be found by name.
|
||||
var ErrLabelNotFound = errors.New("label not found")
|
||||
|
||||
// ListOrgLabels returns all labels for repos in the given organisation.
|
||||
// Note: The Forgejo SDK does not have a dedicated org-level labels endpoint.
|
||||
// This lists labels from the first repo found, which works when orgs use shared label sets.
|
||||
|
|
@ -74,15 +78,20 @@ func (c *Client) GetLabelByName(owner, repo, name string) (*forgejo.Label, error
|
|||
}
|
||||
}
|
||||
|
||||
return nil, log.E("forge.GetLabelByName", "label "+name+" not found in "+owner+"/"+repo, nil)
|
||||
return nil, log.E("forge.GetLabelByName", "label "+name+" not found in "+owner+"/"+repo, ErrLabelNotFound)
|
||||
}
|
||||
|
||||
// EnsureLabel checks if a label exists, and creates it if it doesn't.
|
||||
// Only creates a new label when the error is ErrLabelNotFound; other
|
||||
// errors (e.g. network failures) are returned immediately.
|
||||
func (c *Client) EnsureLabel(owner, repo, name, color string) (*forgejo.Label, error) {
|
||||
label, err := c.GetLabelByName(owner, repo, name)
|
||||
if err == nil {
|
||||
return label, nil
|
||||
}
|
||||
if !errors.Is(err, ErrLabelNotFound) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return c.CreateRepoLabel(owner, repo, forgejo.CreateLabelOption{
|
||||
Name: name,
|
||||
|
|
|
|||
2
go.mod
2
go.mod
|
|
@ -1,6 +1,6 @@
|
|||
module forge.lthn.ai/core/go-scm
|
||||
|
||||
go 1.26.0
|
||||
go 1.26.1
|
||||
|
||||
require (
|
||||
code.gitea.io/sdk/gitea v0.23.2
|
||||
|
|
|
|||
|
|
@ -109,10 +109,14 @@ func (h *DispatchHandler) Execute(ctx context.Context, signal *jobrunner.Pipelin
|
|||
if l.Name == LabelInProgress || l.Name == LabelAgentComplete {
|
||||
coreerr.Info("issue already processed, skipping", "issue", signal.ChildNumber, "label", l.Name)
|
||||
return &jobrunner.ActionResult{
|
||||
Action: "dispatch",
|
||||
Success: true,
|
||||
Timestamp: time.Now(),
|
||||
Duration: time.Since(start),
|
||||
Action: "dispatch",
|
||||
RepoOwner: safeOwner,
|
||||
RepoName: safeRepo,
|
||||
EpicNumber: signal.EpicNumber,
|
||||
ChildNumber: signal.ChildNumber,
|
||||
Success: true,
|
||||
Timestamp: time.Now(),
|
||||
Duration: time.Since(start),
|
||||
}, nil
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ import (
|
|||
|
||||
forgejosdk "codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v2"
|
||||
|
||||
coreerr "forge.lthn.ai/core/go-log"
|
||||
coreerr "forge.lthn.ai/core/go-log" // aliased coreerr: go-log exports E() for structured error creation
|
||||
"forge.lthn.ai/core/go-scm/forge"
|
||||
"forge.lthn.ai/core/go-scm/jobrunner"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -25,6 +25,18 @@ type Builder struct {
|
|||
|
||||
// Org is the default organisation used when constructing Repo URLs.
|
||||
Org string
|
||||
|
||||
// Medium is the filesystem abstraction used for reading and writing.
|
||||
// Falls back to coreio.Local if nil.
|
||||
Medium coreio.Medium
|
||||
}
|
||||
|
||||
// medium returns the builder's Medium or falls back to coreio.Local.
|
||||
func (b *Builder) medium() coreio.Medium {
|
||||
if b.Medium != nil {
|
||||
return b.Medium
|
||||
}
|
||||
return coreio.Local
|
||||
}
|
||||
|
||||
// BuildFromDirs scans each directory for subdirectories containing either
|
||||
|
|
@ -34,8 +46,9 @@ func (b *Builder) BuildFromDirs(dirs ...string) (*Index, error) {
|
|||
var modules []Module
|
||||
seen := make(map[string]bool)
|
||||
|
||||
m := b.medium()
|
||||
for _, dir := range dirs {
|
||||
entries, err := os.ReadDir(dir)
|
||||
entries, err := m.List(dir)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
continue
|
||||
|
|
@ -113,22 +126,24 @@ func BuildFromManifests(manifests []*manifest.Manifest) *Index {
|
|||
}
|
||||
|
||||
// WriteIndex serialises an Index to JSON and writes it to the given path.
|
||||
func WriteIndex(path string, idx *Index) error {
|
||||
if err := coreio.Local.EnsureDir(filepath.Dir(path)); err != nil {
|
||||
func WriteIndex(m coreio.Medium, path string, idx *Index) error {
|
||||
if err := m.EnsureDir(filepath.Dir(path)); err != nil {
|
||||
return coreerr.E("marketplace.WriteIndex", "mkdir failed", err)
|
||||
}
|
||||
data, err := json.MarshalIndent(idx, "", " ")
|
||||
if err != nil {
|
||||
return coreerr.E("marketplace.WriteIndex", "marshal failed", err)
|
||||
}
|
||||
return coreio.Local.Write(path, string(data))
|
||||
return m.Write(path, string(data))
|
||||
}
|
||||
|
||||
// loadFromDir tries core.json first, then falls back to .core/manifest.yaml.
|
||||
func (b *Builder) loadFromDir(dir string) (*manifest.Manifest, error) {
|
||||
m := b.medium()
|
||||
|
||||
// Prefer compiled manifest (core.json).
|
||||
coreJSON := filepath.Join(dir, "core.json")
|
||||
if raw, err := coreio.Local.Read(coreJSON); err == nil {
|
||||
if raw, err := m.Read(coreJSON); err == nil {
|
||||
cm, err := manifest.ParseCompiled([]byte(raw))
|
||||
if err != nil {
|
||||
return nil, coreerr.E("marketplace.Builder.loadFromDir", "parse core.json", err)
|
||||
|
|
@ -138,16 +153,16 @@ func (b *Builder) loadFromDir(dir string) (*manifest.Manifest, error) {
|
|||
|
||||
// Fall back to source manifest.
|
||||
manifestYAML := filepath.Join(dir, ".core", "manifest.yaml")
|
||||
raw, err := coreio.Local.Read(manifestYAML)
|
||||
raw, err := m.Read(manifestYAML)
|
||||
if err != nil {
|
||||
return nil, nil // No manifest — skip silently.
|
||||
}
|
||||
|
||||
m, err := manifest.Parse([]byte(raw))
|
||||
mf, err := manifest.Parse([]byte(raw))
|
||||
if err != nil {
|
||||
return nil, coreerr.E("marketplace.Builder.loadFromDir", "parse manifest.yaml", err)
|
||||
}
|
||||
return m, nil
|
||||
return mf, nil
|
||||
}
|
||||
|
||||
// repoURL constructs a module repository URL from the builder config.
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
coreio "forge.lthn.ai/core/go-io"
|
||||
"forge.lthn.ai/core/go-scm/manifest"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
|
@ -206,7 +207,7 @@ func TestWriteIndex_Good(t *testing.T) {
|
|||
},
|
||||
}
|
||||
|
||||
err := WriteIndex(path, idx)
|
||||
err := WriteIndex(coreio.Local, path, idx)
|
||||
require.NoError(t, err)
|
||||
|
||||
data, err := os.ReadFile(path)
|
||||
|
|
@ -231,7 +232,7 @@ func TestWriteIndex_Good_RoundTrip(t *testing.T) {
|
|||
idx, err := b.BuildFromDirs(root)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, WriteIndex(path, idx))
|
||||
require.NoError(t, WriteIndex(coreio.Local, path, idx))
|
||||
|
||||
data, err := os.ReadFile(path)
|
||||
require.NoError(t, err)
|
||||
|
|
|
|||
|
|
@ -110,7 +110,9 @@ func (i *Installer) Remove(code string) error {
|
|||
}
|
||||
|
||||
dest := filepath.Join(i.modulesDir, code)
|
||||
_ = i.medium.DeleteAll(dest)
|
||||
if err := i.medium.DeleteAll(dest); err != nil {
|
||||
return coreerr.E("marketplace.Installer.Remove", "delete "+code, err)
|
||||
}
|
||||
|
||||
return i.store.Delete(storeGroup, code)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -106,7 +106,7 @@ func FindRegistry(m io.Medium) (string, error) {
|
|||
// Check current directory and parents
|
||||
dir, err := os.Getwd()
|
||||
if err != nil {
|
||||
return "", err
|
||||
return "", coreerr.E("repos.FindRegistry", "failed to get working directory", err)
|
||||
}
|
||||
|
||||
for {
|
||||
|
|
@ -131,7 +131,7 @@ func FindRegistry(m io.Medium) (string, error) {
|
|||
// Check home directory common locations
|
||||
home, err := os.UserHomeDir()
|
||||
if err != nil {
|
||||
return "", err
|
||||
return "", coreerr.E("repos.FindRegistry", "failed to get home directory", err)
|
||||
}
|
||||
|
||||
commonPaths := []string{
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue