Conversation
Reference: core/api/LICENCE. Co-Authored-By: Cladius Maximus <cladius@lethean.io>
Drove banned-imports from 104 to 84 (-20) across 20 files.
Files converted (all backtick stdlib imports → core wrappers):
- 5 errors-only files: workconfig.go, kbconfig.go, gitstate.go, plugin/manifest.go, marketplace/builder.go (errors.New → core.E)
- 4 errors-only constructors: marketplace/installer.go (7 sites), gitea/client.go, forge/client.go, manifest/loader.go (errors.New → core.E)
- 1 errors-Is + errors.New: marketplace/index.go (errors.Is → core.Is, errors.New → core.E)
- 4 strings.Contains-only test files: collect/{bitcointalk,market,process}_test.go, plugin/installer_test.go (strings.Contains → core.Contains)
- 2 filepath.Join-only test files: marketplace/installer_test.go, agentci/config_test.go (filepath.Join → core.PathJoin)
- 1 filepath.Join + filepath.Base test file: agentci/agentci_test.go (filepath.Join → core.PathJoin, filepath.Base → core.PathBase)
- 3 json files: manifest_test.go (json.Unmarshal → core.JSONUnmarshal), jobrunner/types.go (json.Marshal/Unmarshal → core.JSONMarshal/Unmarshal with Result-shape adapters for MarshalJSON/UnmarshalJSON contracts), collect/state.go (json.Unmarshal/MarshalIndent → core.JSON*)
Verification:
- audit.sh: 104 → 84 findings (-20)
- GOWORK=off go build ./... clean
- GOWORK=off go test -count=1 ./... all packages pass
Refs tasks.lthn.sh/view.php?id=1319 (still open — more files to convert)
Filed-by: hephaestus
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drove banned-imports from 84 to 74 (-10) across 5 files. - manifest/sign.go: errors.New → core.E (2x), json.Marshal → core.JSONMarshal (with Result-shape adapter for `([]byte, error)` return) - plugin/loader.go: errors.New → core.E, filepath.Join → core.PathJoin - pkg/api/provider_test.go: json.Marshal → core.JSONMarshal (with Result-shape adapter), strings.Contains → core.Contains - repos/registry_test.go: os.WriteFile → core.WriteFile (Result), filepath.Join → core.PathJoin - scm_test.go: os.WriteFile → core.WriteFile (Result), filepath.Join → core.PathJoin Verification: - audit.sh: 84 → 74 findings (-10) - GOWORK=off go build ./... clean - GOWORK=off go test -count=1 ./... all packages pass Refs tasks.lthn.sh/view.php?id=1319 (still open — more files to convert) Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drove banned-imports from 74 to 64 (-10) across 5 test files. - forge/forge_test.go: os.MkdirAll/WriteFile/ReadFile + filepath.Join → core wrappers (Result-shape adapters) - gitea/gitea_test.go: same pattern - jobrunner/handlers/handlers_test.go: os.WriteFile + filepath.Join → core wrappers - jobrunner/poller_test.go: os.Stat + filepath.Join → core wrappers - marketplace/discovery_test.go: os.MkdirAll/WriteFile/Getwd/Chdir + filepath.Join/EvalSymlinks → core wrappers Note: NOT converting go/core/config/config_test.go — that subdir is a SEPARATE Go module (intentionally minimal, no dappco.re/go dep). Audit still flags its 2 banned imports; they're real but out-of-scope for #1319. Verification: - audit.sh: 74 → 64 findings (-10) - GOWORK=off go build ./... clean - GOWORK=off go test -count=1 ./forge/ ./gitea/ ./jobrunner/ ./marketplace/ all pass Refs tasks.lthn.sh/view.php?id=1319 (still open — more files to convert) Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drove banned-imports from 64 to 60 (-4) across 1 production-side test file. - jobrunner_test.go: removed errors/json/os/path-filepath imports; errors.New → core.E (test sentinel), json.Unmarshal → core.JSONUnmarshal (with Result-shape adapter), os.Stat + filepath.Join → core.Stat + core.PathJoin - core/config/config_test.go: imports left as-is (file lives in a separate go.mod sub-module that intentionally has no dappco.re/go dep). Audit carve-out documented. Verification: - audit.sh: 64 → 60 findings (-4) - GOWORK=off go build ./... clean - GOWORK=off go test -count=1 ./jobrunner/ pass Refs tasks.lthn.sh/view.php?id=1319 Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
…block) - git submodule update on external/* to current dev tips - go.work paths fixed for Phase 1 /go/ subtree layout where stale - go.work go-version bumped 1.26.0 → 1.26.2 to match submodule floor Workspace-mode build (`go build ./...`) is the verification path. Some repos may surface transitive dep issues (api/go.sum checksum drift, etc.) which are separate cascade tickets — not blocking this metadata refresh. Co-Authored-By: Cladius Maximus <cladius@lethean.io>
Co-Authored-By: Cladius Maximus <cladius@lethean.io>
Bumps external/go to dev@62aac07 (post-#1329 wrapper expansion +
tracked-artifacts audit dimension).
Adopts the new wrappers in the easy substitution set:
- forge/labels.go: strings.ToLower → core.Lower (3); strings.EqualFold
→ core.Lower(a)==core.Lower(b) (1); drops `strings` import
- forge/prs.go: bytes.NewReader → core.NewBufferReader (1); drops
`bytes` import
- collect/bitcointalk.go: bytes.Buffer → core.Buffer (1); drops
`bytes` import
- collect/process.go: bytes.Buffer → core.Buffer (5); json.Unmarshal
→ core.JSONUnmarshal (3 sites, Result-shape); json.NewEncoder +
enc.SetIndent + enc.Encode → core.JSONMarshalIndent + buf.Write +
buf.WriteByte('\n') (preserves trailing-newline behaviour); drops
`bytes` and `encoding/json` imports
Banned-imports: 60 → 49 (-11) for go-scm.
Build green (workspace mode).
Refs Mantis #1319
Filed-by: hephaestus
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Replaces banned stdlib imports with core/go primitives in production code: - forge/config.go: errors+os+path/filepath → core.E + core.Getenv + core.UserHomeDir().Value.(string) + core.PathJoin + core.PathDir + core.ReadFile + core.MkdirAll + core.WriteFile (all Result-shape) - gitea/config.go: same pattern as forge/config.go - agentci/config.go: errors+fmt+strings → core.E + core.Sprintf + core.Lower (replaces strings.EqualFold via Lower-comparison) + core.Contains - jobrunner/forgejo/forgejo.go: fmt+strings → core.Sprintf + core.E + core.Trim + core.Lower (3 EqualFold sites converted via Lower==const) + core.Upper + core.Split - plugin/installer.go: errors+strings → core.E + core.SplitN; adds package-local lastIndexAt helper for strings.LastIndex (no core wrapper exists; helper is 7 lines, no banned imports) All conversions preserve original semantics. The Result-unwrap pattern is verbose but matches the canonical core/go shape consumers should adopt. Banned-imports: 49 → 36 (-13) for go-scm. Build green (workspace mode). Refs Mantis #1319 Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
The local go/core/config/ module was a pre-cascade-resolution defensive copy intended to avoid downstream dependency on dappco.re/go/config. It is: - Not imported by any consumer (verified with grep across all .go, yaml, toml, Taskfile, and shell files) - Not wired into the go.work workspace - A separate module (dappco.re/go/core/config) that doesn't collide with the upstream dappco.re/go/config but offers no functionality beyond it - The source of 4 banned-imports findings (errors, os, path/filepath, strings) that were only justified by the now-stale "local replacement" rationale Removing it clears 4 banned-imports findings and 5 unused files (config.go, config_test.go, config_example_test.go, go.mod, go.sum). All consumers already use dappco.re/go/config from the upstream external/config/ submodule (verified at agentci/config.go and agentci/agentci_test.go). Refs Mantis #1319 Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Replaces banned stdlib imports in cmd/{compile,pkg,sign,verify} test
files with core/go primitives:
- strings.Contains → core.Contains (4 sites)
- os.MkdirAll → core.MkdirAll(...).OK Result-shape unwrap (2 sites)
- os.WriteFile → core.WriteFile(...).OK Result-shape unwrap (4 sites)
- os.ReadFile → core.ReadFile(...).OK + .Value.([]byte) (3 sites)
- filepath.Join → core.PathJoin (9 sites)
Drops `path/filepath` and `strings` imports from all 4 files.
The `os` import is retained in each file because captureStdout test
helper swaps `os.Stdout` and uses `os.Pipe()` for stdout-capture
testing — neither has a core/go equivalent (no core.Pipe, no
core.OSStdoutCapture wrapper). This is a test-helper-shaped wrapper
gap worth surfacing separately; for now `os` remains for that one
use site per file.
All cmd/* tests pass (compile, pkg, sign, verify packages).
Banned-imports: 36 → 24 (-12) for go-scm.
Refs Mantis #1319
Filed-by: hephaestus
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drops `os` + `path/filepath` imports; replaces with core wrappers: - filepath.Join → core.PathJoin (~12 sites) - os.MkdirAll → core.MkdirAll(...).OK Result-shape via local requireR test helper (2 sites) - os.WriteFile → core.WriteFile(...).OK via requireR (4 sites) - os.ReadFile → core.ReadFile + .Value.([]byte) unwrap (3 sites) - core.RequireNoError(t, os.X(...)) → requireR(t, core.X(...)) pattern, since core.X returns Result not error Adds package-local requireR(t *testing.T, r core.Result) helper because core's RequireNoError takes error, but core.WriteFile returns Result. Could be promoted to core/go later if the pattern recurs. All marketplace tests pass. Banned-imports: 24 → 22 (-2) for go-scm. Refs Mantis #1319 Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drops `encoding/json` + `os` + `path/filepath` imports; replaces with core wrappers: - filepath.Join → core.PathJoin (1 site) - os.ReadFile → core.ReadFile + .Value.([]byte) unwrap (1 site) - json.Marshal → core.JSONMarshal + .Value.([]byte) (1 site) - json.Unmarshal → core.JSONUnmarshal Result-shape (3 sites) All jobrunner tests pass. Banned-imports: 22 → 19 (-3) for go-scm. Refs Mantis #1319 Filed-by: hephaestus Co-authored-by: Hephaestus <hephaestus@lthn.ai>
Drops `errors` + `os` + `path/filepath` imports; replaces with core
wrappers:
- filepath.Join → core.PathJoin (1 site)
- os.WriteFile → core.WriteFile via local requireR helper (1 site)
- core.RequireNoError(t, os.X) → requireR(t, core.X) Result-shape
- errors.New → core.E("collect.test", msg, nil) (2 sites — emit-error
fixture and BitcoinTalk fetcher mock)
Adds package-local requireR(t *core.T, r core.Result) helper to
bridge Result-shape returns into the existing test fail pattern.
All collect tests pass.
Banned-imports: 19 → 16 (-3) for go-scm.
Refs Mantis #1319
Filed-by: hephaestus
Co-authored-by: Hephaestus <hephaestus@lthn.ai>
…/ratelimit.go (Mantis #1336)
audit: 20 -> 15.
- cmd/{verify,compile,sign,pkg}/cmd_*_test.go: captureStdout helper
refactored to use cli.SetStdout(core.NewBuilder()) instead of
os.Pipe + os.Stdout reassignment. Drops io+os imports.
- git/git.go: runGit() switched from exec.CommandContext to
process.RunWithOptions. Drops os/exec import.
- collect/ratelimit.go: gh CLI rate-limit probe switched to
process.RunWithOptions. Drops os/exec import.
New deps: dappco.re/go/cli (test stdout-capture), dappco.re/go/process
(subprocess execution). Both added as external/ submodules + go.work
+ go.mod indirect requires.
Remaining 15: 4 in repos/{service,repos}_test.go, 4 in
jobrunner/handlers/handlers.go, 4 in git/git_test.go (3 of those 4
files are test-only refactors).
Co-Authored-By: Cladius <noreply@anthropic.com>
…ureSSHArgs (Mantis #1336) audit: 14 -> 12. handlers.go: 4 imports dropped (encoding/json, fmt, os/exec, strings) - fmt.Errorf → core.E(op, msg, cause): preserves op as structured field - fmt.Sprintf → core.Sprintf - strings.EqualFold → core.Lower(x) == "literal" (explicit case-fold) - strings.Trim/Split/Contains/Replace/Join → core.Trim/Split/Contains/Replace/Join - json.MarshalIndent → core.JSONMarshalIndent (Result-unwrapping) - exec.Cmd return type from buildDispatchSSHCommand → (host, shellCmd string) caller invokes via process.RunWithOptions + agentci.SecureSSHArgs agentci/security.go: extracted SecureSSHArgs(host, cmd) []string helper so callers can spawn via process.RunWithOptions without depending on *exec.Cmd construction. SecureSSHCommandContext kept for backward compat. Co-Authored-By: Cladius <noreply@anthropic.com>
…336)
audit: 14 -> 0 / verdict COMPLIANT.
go/git/git_test.go:
- runGitCmd / testGitCommand: exec.Command → process.RunWithOptions
- os.WriteFile / ReadFile → core.WriteFile / ReadFile (Result-shape)
- filepath.Join / Base → core.PathJoin / PathBase (1:1 rename)
- errors.New("X") → core.E("test", "X", nil) (1:1 with op slot)
go/repos/repos_test.go + service_test.go:
- Same patterns; testReposGitCommand / runGitCmd refactored.
- writeRegistry helper returns core.WriteFile/MkdirAll Result unwrapped.
go/agentci/security_test.go + security_example_test.go:
- TestSecurity_SecureSSHArgs_{Good,Bad,Ugly} + ExampleSecureSSHArgs
added for the new SecureSSHArgs helper introduced in caa6ca6.
Co-Authored-By: Cladius <noreply@anthropic.com>
…#1336)
go-scm now exposes the canonical service-registration shape every other
go-* repo provides per #1336:
scm.NewService(scm.Options{...}) → factory for core.WithService
scm.Register(c) → imperative Register with defaults
Implementation lives in service.go as thin aliases over the existing
NewCoreService — no behaviour change, just the canonical names so
consumers (CoreGUI, IDE, lthn.ai server, tools) can pull go-scm in
through the same plumbing as go-store, go-process, go-i18n, etc.
NewCoreService is kept as a deprecated alias for one cycle so the five
downstream pinned consumers (api, ide, go-ai, lint, go-devops) don't
break before they bump submodule pins:
api/external/go-scm — pinned, will get NewService at next bump
ide/external/scm — pinned, same
go-ai/external/go-scm — pinned, same
lint/external/go-scm — pinned, same
go-devops/external/go-scm — pinned, same
cmd/scm/main.go updated to use NewService inline as the model usage
example for downstream consumers.
Smoke verified:
- go vet ./... clean
- go build ./cmd/scm clean
- TestRegister_NewServiceWiresSCMandGit (smoke) — PASS:
c := core.New(core.WithService(NewService(Options{Root: tmpdir})))
ServiceStartup → c.Service("scm").OK + c.Service("git").OK
- TestRegister_ImperativeRegister (smoke) — PASS:
c := core.New(); Register(c) returns Ok
- Pre-existing tests still pass (TestScm_NewCoreService_* untouched —
they test the alias path which stays identical)
Co-Authored-By: Virgil <virgil@lethean.io>
…s (S4144, S1186, S100, S1871, S1940, S1135, Python+YAML lint)
Co-Authored-By: Virgil <virgil@lethean.io> Co-Authored-By: Hephaestus <hephaestus@lthn.ai>
The external/go submodule was checked out at a v0.3.3-era commit (6c4bcf0) while go/go.mod pins dappco.re/go v0.10.3. Under the repo's go.work (the canonical dev test environment) this failed to compile: undefined core.Buffer and core.NewBufferReader, used by collect/process.go, collect/bitcointalk.go and forge/prs.go. Aligning the submodule with the declared go.mod version restores a clean `go build ./...` from a fresh checkout under go.work. No source in external/go is modified — only the submodule pin. Co-Authored-By: Virgil <virgil@lethean.io>
- Add main_test.go TestMain to initialise the global process service
(git helpers call process.RunWithOptions, which v0.10.3 requires be
initialised via process.Init first).
- Add service_handlers_test.go with Good/Bad/Ugly triplets for the
previously-untested service surface: runPush/runPull, the multiple
variants, validatePath/validatePaths, handleQuery, handleTaskMessage,
and the parseStatus/parseBranchLine/parseCount helpers.
- Fix stale TestGit_GitError_Error_Bad expectation: v0.10.3 core.E
renders the Operation prefix ("test: failed"), not the bare message.
Coverage: git 53.5% -> 86.5%.
Co-Authored-By: Virgil <virgil@lethean.io>
- Add main_test.go TestMain to initialise the global process service (registry sync paths shell out via process.RunWithOptions, which v0.10.3 requires be initialised through process.Init first). - Add service_sync_test.go with a seedSyncedRepo helper plus Good/Bad/ Ugly triplets for syncAll, handleRepoSyncAll, syncRepo (by-path) and the syncNamedRepo registry-not-loaded path. Coverage: repos 71.8% -> 89.0%. Co-Authored-By: Virgil <virgil@lethean.io>
- handlers: add main_test.go TestMain (dispatch handler shells out via process.RunWithOptions, which v0.10.3 requires be initialised through process.Init); add comments_test.go covering completionComment, fixCommandComment and tickCheckbox with Good/Bad/Ugly triplets. - forgejo: add helpers_test.go covering the pure SDK-mapping helpers splitRepoRef, parseChildIssueNumbers, pullState, mergeableState, issueAssignee, pullCommitSHA, pullCommitTime, plus the signalsForEpic nil/empty-body/error-fetch branches. Coverage: jobrunner/handlers 70.8% -> 77.5%; jobrunner/forgejo 18.9% -> 61.4%. Co-Authored-By: Virgil <virgil@lethean.io>
The rate-limiter shells out to `gh` via process.RunWithOptions, which v0.10.3 requires be initialised through process.Init first. Without it the CheckGitHubRateLimit Good cases failed with a "service not initialized" error. Coverage: collect 72.3% -> 74.2%. Co-Authored-By: Virgil <virgil@lethean.io>
The compile/sign/pkg/verify commands emit usage and result lines via core.Print(nil, ...), which writes to core.Stdout() (os.Stdout) directly. v0.10.3 routes that to the OS stream, no longer to the cli package writer, so the previous cli.SetStdout-based captureStdout helper saw empty output and TestRegisterHelp / TestVerifySignedCoreJSON failed. Replace each captureStdout helper with an os.Pipe redirect of os.Stdout, which faithfully captures core.Print output under v0.10.3. Test-only change; production output behaviour is unchanged. Co-Authored-By: Virgil <virgil@lethean.io>
- cmd/pkg: add helpers_test.go with Good/Bad/Ugly triplets for uniqueCategories, splitList, trimRightSlash, sortIndex, applyRepoDefaults, failed and resultError. - cmd/verify: add helpers_test.go covering canonicalManifestBytes, publicKey (incl. wrong-size key), failed, resultError and readFile. Coverage: cmd/pkg 54.2% -> 77.1%; cmd/verify 56.5% -> 72.9%. Co-Authored-By: Virgil <virgil@lethean.io>
Add handlers_test.go exercising the error/edge branches of the SCM provider routes that the happy-path test left at 50%: nil index/registry/ installer, module-not-found, repo-not-found, and the populated getRepo success path. Coverage: pkg/api 61.6% -> 78.8%. Co-Authored-By: Virgil <virgil@lethean.io>
Add helpers_test.go with Good/Bad/Ugly triplets for privateKey (incl. wrong-size key), canonicalManifestBytes, compiledManifest (manifest-path and missing-file fallbacks), failed, resultError and readFile. Coverage: cmd/sign 64.6% -> 81.8%. Co-Authored-By: Virgil <virgil@lethean.io>
Add meta_helpers_test.go with Good/Bad/Ugly triplets for hasNextForgePage, hasMoreForgeItems (generic short-page / full-page-no-metadata cases) and appendUniqueLabels (case-insensitive dedup). Coverage: forge 70.7% -> 74.1%. Co-Authored-By: Virgil <virgil@lethean.io>
Advance external/go workspace submodule to v0.10.4 so dev (GOWORK on) and standalone (GOWORK=off) builds resolve the same core/go. Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (90)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several infrastructure and dependency updates to the project, primarily focused on improved Go module management, new external dependencies, and licensing. The most significant changes include adding new submodules, updating references to external packages, and introducing a new project license.
Dependency and Submodule Management:
external/cliandexternal/processand updated submodule commits forexternal/config,external/go,external/io,external/log, andexternal/wsto their latest versions. [1] [2] [3] [4] [5] [6] [7] [8]go.workto include new and reorganized module paths for the added and existing external dependencies, reflecting the new submodule structure.go.work.sumfor a leaner and more accurate dependency graph. [1] [2] [3] [4]Project Configuration and Licensing:
.core/go.yamlfile to define project metadata and Go module configuration, improving project structure and automation compatibility.LICENCEfile with the European Union Public Licence v. 1.2, clarifying the project's licensing terms.Code Maintenance:
go/agentci/agentci_test.goto usecore.PathJoininstead offilepath.Joinfor path handling, aligning with internal utilities. [1] [2]