Skip to content

v0.9.0 compliance: full upgrade against core/go reference#5

Open
Snider wants to merge 2 commits intomainfrom
dev
Open

v0.9.0 compliance: full upgrade against core/go reference#5
Snider wants to merge 2 commits intomainfrom
dev

Conversation

@Snider
Copy link
Copy Markdown
Contributor

@Snider Snider commented Apr 28, 2026

Brings this repo to verdict: COMPLIANT against the v0.9.0 audit.

🤖 Generated with Claude Code + Codex
Co-Authored-By: Codex noreply@openai.com
Co-Authored-By: Virgil virgil@lethean.io

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved error handling for asynchronous task execution; errors are now properly captured and handled.
    • Enhanced context cancellation handling in SSH operations with better error reporting.
    • Fixed YAML parsing errors that could leave inconsistent state when loading role configuration.
  • Tests

    • Extensive new end-to-end test suite covering public APIs, SSH client operations, and playbook execution.
    • Migrated test framework to improve consistency and reliability.
  • Chores

    • Updated testing dependencies for enhanced code quality assurance.

Snider and others added 2 commits April 27, 2026 17:34
Round 2 verification on PR #4. CodeRabbit re-flagged previously-fixed
items; verified each is still present from d7d6e33:
- async_features.go: timeout context lifecycle (line 25)
- async_features.go: cached clients recreated not shared (line 79)
- executor.go: unresolved operands fail closed (line 2883)
- executor.go: symbolic operators with spaces (line 2914)
- parser.go: sync.RWMutex on medium access (line 24)
- parser.go: non-local wildcard vars rejected with coreerr.E (line 421)
- template_features.go: bare registered-result lookup (line 55)
- local_client.go: errors wrapped with coreerr.E (line 111)
- go.mod / go.sum: replacements produce replacement-path sums

Only diff this pass: gofmt on cmd/ansible/ansible.go (single line).

Verification: gofmt -l clean, GOWORK=off go vet + go test -count=1 ./...
pass with explicit cache paths.

Co-authored-by: Codex <noreply@openai.com>
bash /tmp/v090/audit.sh . → verdict: COMPLIANT (all 7 dimensions zero).
go test -count=1 ./... → all green.

Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Virgil <virgil@lethean.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

📝 Walkthrough

Walkthrough

This pull request migrates the test framework across the codebase from Go's standard testing.T with testify packages to a custom dappco.re/go framework using core.T. It updates related import paths, refines error handling in several core functions, and updates dependencies in go.mod.

Changes

Cohort / File(s) Summary
Test Framework Migration
ax7_public_test.go, cmd/ansible/ansible_test.go, cmd/ansible/ax7_register_test.go, executor_become_test.go, executor_extra_test.go, executor_test.go, local_client_test.go, mock_ssh_test.go, modules_cmd_test.go, modules_file_test.go, modules_infra_test.go, modules_svc_test.go, parser_test.go, ssh_test.go, types_test.go
Replaces testing.T with core.T from dappco.re/go; migrates all testify/assert and testify/require calls to corresponding core.Assert* and core.Require* helpers. Adds comprehensive end-to-end tests in ax7_public_test.go covering SSH, inventory, playbooks, and executor lifecycle.
Error Handling & Logic Improvements
async_features.go, executor.go, parser.go, ssh.go
Improves error handling: async task launcher now checks runTaskOnHost errors and exits early on failure; executor.go centralises client closure with explicit error handling; parser.go validates YAML unmarshal results; ssh.go checks context cancellation signal errors before returning.
Import Path Updates
cmd/ansible/cmd.go, cmd/ansible/core_primitives.go, core_primitives.go
Changes core package import from dappco.re/go/core to dappco.re/go; updates usage examples and import statements accordingly.
Command Registration Updates
cmd/ansible/ansible.go
Switches result creation from direct core.Result{...} to standardised core.Fail(err) and core.Ok(nil) helpers; updates import to dappco.re/go; improves SSH client cleanup error handling.
Dependency Management
go.mod
Replaces direct dependency on dappco.re/go/core v0.8.0-alpha.1 with dappco.re/go v0.9.0; removes github.com/stretchr/testify v1.11.1 and transitive test utility dependencies.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main objective of the pull request: upgrading the codebase to v0.9.0 compliance against the core/go reference package.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
3.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ax7_public_test.go (1)

14-1257: ⚠️ Potential issue | 🟠 Major

Restore testify-based assertions in this file.

This test suite uses core.Assert* and core.Require* from dappco.re/go, but the repository coding guidelines require testify/assert for soft assertions and testify/require for hard assertions in all *_test.go files. Please refactor the approximately 300+ assertions across this file to use the standard testify framework.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ax7_public_test.go` around lines 14 - 1257, This file still uses the
project's custom core.Require*/core.Assert* helpers (e.g. calls inside
TestAX7_Client_BecomeState_Good, TestAX7_SSHClient_Run_Good,
TestAX7_Executor_TemplateFile_Good, ax7WriteFile, ax7Inventory,
ax7ConnectedSSHClient, and all other TestAX7_* functions); replace those with
testify's require.* for hard assertions (formerly core.Require*) and assert.*
for soft assertions (formerly core.Assert*), import
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
and update each call signature accordingly (e.g. require.NoError(t, err),
assert.Equal(t, expected, actual), etc.) while preserving test semantics and
keeping helper t.Helper() usage and existing function names unchanged.
🧹 Nitpick comments (4)
cmd/ansible/ansible.go (1)

486-490: Simplify the deferred close block; current error branch is a no-op.

At Line 486-Line 490, the if err := client.Close(); err != nil { return } branch silently discards errors and has no side effect. A direct defer is clearer.

Proposed cleanup
-	defer func() {
-		if err := client.Close(); err != nil {
-			return
-		}
-	}()
+	defer client.Close()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/ansible/ansible.go` around lines 486 - 490, The deferred anonymous
function that calls if err := client.Close(); err != nil { return } should be
simplified to a direct defer of the Close call; replace the defer func block
with defer client.Close() so errors are not silently checked with a no-op branch
and the intent is clearer (look for the deferred anonymous closure that invokes
client.Close()).
ax7_public_test.go (1)

50-55: Use at least a 2048-bit RSA host key in the SSH test helper.

The in-process SSH server is generating a 1024-bit RSA key here. That is already being flagged by static analysis and can break under stricter crypto policies in CI, even for test code.

Possible fix
-	key, err := rsa.GenerateKey(rand.Reader, 1024)
+	key, err := rsa.GenerateKey(rand.Reader, 2048)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ax7_public_test.go` around lines 50 - 55, The SSH test helper currently
generates a 1024-bit RSA key (rsa.GenerateKey(rand.Reader, 1024)) which is too
weak; update the call to rsa.GenerateKey to use at least 2048 bits (e.g., 2048
or 4096) so the generated key (variable key) and subsequent signer creation via
ssh.NewSignerFromKey remain compatible with stricter crypto policies; ensure
serverConfig.AddHostKey(signer) continues to receive the new stronger key.
cmd/ansible/ax7_register_test.go (1)

13-19: Avoid locking a nil-pointer panic into Register's contract.

Register is a public entrypoint in cmd/ansible/cmd.go, and this test currently makes a nil dereference the expected behaviour. That will fight any later hardening; it would be better for the test to follow a safe nil guard once the implementation is updated.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/ansible/ax7_register_test.go` around lines 13 - 19, The test
TestAX7_Register_Bad should stop asserting a nil-pointer panic from Register;
update the test to call Register(app) with app == nil but assert it does NOT
panic and that app remains nil afterwards. Replace the core.AssertPanics(t,
func(){ Register(app) }) usage with a non-panicking assert (e.g.
core.AssertNotPanics or wrap Register in a recover-based check) and keep
core.AssertNil(t, app) so the test verifies safe nil-guarding in Register rather
than expecting a panic.
async_features.go (1)

43-45: Surface detached async failures for diagnostics.

This goroutine still discards runTaskOnHost errors and exits silently. Please log or record the failure so async task issues are diagnosable in CI and local runs.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@async_features.go` around lines 43 - 45, The goroutine is swallowing errors
from clone.runTaskOnHost (called with asyncCtx, host, cloneHosts, &cloneTask,
clonePlay) and returning silently; update the goroutine to capture the returned
error, and instead of just returning, log or record the failure with contextual
fields (host, cloneTask ID/name, clonePlay, error) using the existing logger or
a failure channel so async failures are visible in CI/local runs; ensure the
error handling path includes both an informative log entry and, if present in
surrounding code, sending the error to the task-status/failure reporting
mechanism rather than discarding it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ax7_public_test.go`:
- Around line 48-119: The helper ax7ConnectedSSHClient currently leaves a
running server goroutine and open connections; register a deterministic teardown
by calling t.Cleanup(...) before returning to close the returned *SSHClient and
the listener (and any raw clientConn/conn if necessary) so the server goroutine
unblocks and exits. Capture listener and the SSHClient value in the cleanup
closure and invoke their Close methods (or SSHClient.Close) to ensure the TCP
listener and SSH client connection are closed when the test finishes.

In `@cmd/ansible/ax7_register_test.go`:
- Around line 3-28: Change the test functions to use the standard testing.T
signature and testify assertions: update imports to include "testing",
"github.com/stretchr/testify/assert", and "github.com/stretchr/testify/require";
change each test signature from func TestAX7_Register_*(t *core.T) to func
TestAX7_Register_*(t *testing.T); replace core.AssertTrue with assert.True(t,
...), core.AssertNotNil with assert.NotNil(t, ...), core.AssertPanics with
require.Panics(t, func() { Register(app) }), and core.AssertNil with
assert.Nil(t, app); keep the same test names and calls to Register, core.New(),
and app.Command("ansible"/"ansible/test") so behavior is unchanged.

In `@executor_become_test.go`:
- Line 5: Remove the core testing framework import (core "dappco.re/go") and
switch the tests in executor_become_test.go to use testify: add imports
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
then replace the core assertions used in the become tests (the assertion blocks
previously at lines around the two failing sections) with the appropriate
testify calls—use require.* for hard (fatal) checks (e.g., require.NoError,
require.NotNil) and assert.* for soft checks (e.g., assert.Equal, assert.True)
so each core.Assert/core.Require usage is converted to the corresponding assert
or require function from testify.

In `@executor_extra_test.go`:
- Line 5: Replace the import of core ("dappco.re/go") in executor_extra_test.go
with the testify packages and update all assertion usages: change the import to
include "github.com/stretchr/testify/assert" and
"github.com/stretchr/testify/require", replace every call to core.Assert* with
assert.* and every core.Require* with require.* (e.g., core.AssertEqual →
assert.Equal, core.RequireNoError → require.NoError) ensuring function names map
to their testify equivalents throughout the test file.

In `@executor_test.go`:
- Line 5: Replace the custom assertion import core "dappco.re/go" with the
testify packages and update all assertion calls: import
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require"
and change core.X calls to assert.X (for non-fatal checks) or require.X (for
fatal checks) as appropriate — e.g., replace core.Equal -> require.Equal or
assert.Equal based on whether the test should abort, replace core.NoError ->
require.NoError when error must stop the test, etc.; ensure each assertion call
uses the *testing.T variable as the first argument (t) to match testify
signatures and remove the core import.

In `@modules_cmd_test.go`:
- Line 5: The test import change replaced testify with core ("dappco.re/go")
which violates the repo standard; restore testify usage in modules_cmd_test.go
by re-adding imports for "github.com/stretchr/testify/assert" and
"github.com/stretchr/testify/require" (keeping any other necessary imports like
the core package if used) so tests use assert for soft assertions and require
for hard assertions as required by CLAUDE.md.

In `@modules_file_test.go`:
- Line 7: Replace the project-specific test assertions by importing testify's
packages and switching all core assertion calls to testify equivalents: remove
the import alias core "dappco.re/go" and add
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require";
replace usages like core.AssertEqual, core.AssertTrue, core.AssertNil with
assert.Equal, assert.True, assert.Nil (soft asserts) and replace core.Require*
calls (e.g., core.RequireNoError, core.RequireNotNil) with require.NoError,
require.NotNil (hard asserts) throughout modules_file_test.go, preserving the
same argument order and test.T usage.

In `@modules_infra_test.go`:
- Line 5: The tests use core.Assert*/core.Require* from dappco.re/go; replace
these with testify equivalents: convert soft assertions like core.AssertEqual,
core.AssertTrue, core.AssertNil, core.AssertNotNil, etc. to assert.Equal,
assert.True, assert.Nil, assert.NotNil and convert hard assertions like
core.RequireNoError, core.RequireNil, core.RequireEqual, etc. to
require.NoError, require.Nil, require.Equal. Add the testify imports
(github.com/stretchr/testify/assert and github.com/stretchr/testify/require) and
remove or stop using core.* assertion imports; update any helper calls (e.g.,
TestXxx functions that call core.Assert* or core.Require*) to use the
assert/require variables and adjust call order (require before further checks)
so tests compile and follow project standards.

In `@modules_svc_test.go`:
- Line 4: The file currently imports core "dappco.re/go" and uses
core.Assert*/core.Require* assertion helpers; replace that import with the
testify packages (github.com/stretchr/testify/assert and
github.com/stretchr/testify/require) and change every core.AssertX(...) call to
assert.X(...) and core.RequireX(...) to require.X(...). Update any aliased usage
(e.g., core.AssertEqual -> assert.Equal, core.RequireNoError -> require.NoError)
and ensure the testify packages are added to imports; keep function signatures
and test logic unchanged.

In `@parser_test.go`:
- Line 4: Replace all usages of core's test helpers with testify's equivalents:
remove the core test helper import and add imports for
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
then update calls such that core.AssertEqual -> assert.Equal, core.RequireEqual
-> require.Equal, core.RequireNoError/core.AssertNoError ->
require.NoError/assert.NoError as appropriate, core.RequireTrue/core.AssertTrue
-> require.True/assert.True, etc., preserving the same argument order and
passing t (the *testing.T) as the first argument; ensure table-driven tests and
helper functions use assert/require consistently (use require for fatal
assertions that must stop the test and assert for non-fatal checks) across the
parser_test.go file where core helpers were used.

In `@ssh_test.go`:
- Line 4: Replace all usages of the custom core.Assert* helpers in ssh_test.go
with testify's assert and require calls: add imports
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require"
and remove (or stop using) core.Assert*; use require.* for hard assertions that
should stop the test (e.g., checks for non-nil errors or missing setup) and
assert.* for soft assertions (e.g., value equality or state checks that can
continue). Update each call site (the previous
core.AssertEqual/AssertTrue/AssertNil/AssertNotNil/etc.) to the corresponding
testify form that accepts t as the first arg (e.g., assert.Equal(t, expected,
actual), require.NoError(t, err), require.NotNil(t, obj), assert.True(t, cond)),
ensuring all tests compile and the import list reflects testify usage.

In `@types_test.go`:
- Line 4: The tests import and use dappco.re/go's core.Assert*/core.Require*
helpers; replace that import with "github.com/stretchr/testify/assert" and
"github.com/stretchr/testify/require", update the test functions to accept t
*testing.T if needed, and change all core.AssertX(...) calls to assert.X(t, ...)
and core.RequireX(...) calls to require.X(t, ...). Ensure you remove the core
import alias, adjust any call argument ordering to match testify signatures, and
run the test suite to verify compile-time changes.

---

Outside diff comments:
In `@ax7_public_test.go`:
- Around line 14-1257: This file still uses the project's custom
core.Require*/core.Assert* helpers (e.g. calls inside
TestAX7_Client_BecomeState_Good, TestAX7_SSHClient_Run_Good,
TestAX7_Executor_TemplateFile_Good, ax7WriteFile, ax7Inventory,
ax7ConnectedSSHClient, and all other TestAX7_* functions); replace those with
testify's require.* for hard assertions (formerly core.Require*) and assert.*
for soft assertions (formerly core.Assert*), import
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
and update each call signature accordingly (e.g. require.NoError(t, err),
assert.Equal(t, expected, actual), etc.) while preserving test semantics and
keeping helper t.Helper() usage and existing function names unchanged.

---

Nitpick comments:
In `@async_features.go`:
- Around line 43-45: The goroutine is swallowing errors from clone.runTaskOnHost
(called with asyncCtx, host, cloneHosts, &cloneTask, clonePlay) and returning
silently; update the goroutine to capture the returned error, and instead of
just returning, log or record the failure with contextual fields (host,
cloneTask ID/name, clonePlay, error) using the existing logger or a failure
channel so async failures are visible in CI/local runs; ensure the error
handling path includes both an informative log entry and, if present in
surrounding code, sending the error to the task-status/failure reporting
mechanism rather than discarding it.

In `@ax7_public_test.go`:
- Around line 50-55: The SSH test helper currently generates a 1024-bit RSA key
(rsa.GenerateKey(rand.Reader, 1024)) which is too weak; update the call to
rsa.GenerateKey to use at least 2048 bits (e.g., 2048 or 4096) so the generated
key (variable key) and subsequent signer creation via ssh.NewSignerFromKey
remain compatible with stricter crypto policies; ensure
serverConfig.AddHostKey(signer) continues to receive the new stronger key.

In `@cmd/ansible/ansible.go`:
- Around line 486-490: The deferred anonymous function that calls if err :=
client.Close(); err != nil { return } should be simplified to a direct defer of
the Close call; replace the defer func block with defer client.Close() so errors
are not silently checked with a no-op branch and the intent is clearer (look for
the deferred anonymous closure that invokes client.Close()).

In `@cmd/ansible/ax7_register_test.go`:
- Around line 13-19: The test TestAX7_Register_Bad should stop asserting a
nil-pointer panic from Register; update the test to call Register(app) with app
== nil but assert it does NOT panic and that app remains nil afterwards. Replace
the core.AssertPanics(t, func(){ Register(app) }) usage with a non-panicking
assert (e.g. core.AssertNotPanics or wrap Register in a recover-based check) and
keep core.AssertNil(t, app) so the test verifies safe nil-guarding in Register
rather than expecting a panic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0fd50c3c-ad82-4650-8750-75e7628524e4

📥 Commits

Reviewing files that changed from the base of the PR and between 712d4b5 and 6473705.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (25)
  • async_features.go
  • ax7_public_test.go
  • cmd/ansible/ansible.go
  • cmd/ansible/ansible_test.go
  • cmd/ansible/ax7_register_test.go
  • cmd/ansible/cmd.go
  • cmd/ansible/core_primitives.go
  • core_primitives.go
  • executor.go
  • executor_become_test.go
  • executor_extra_test.go
  • executor_test.go
  • go.mod
  • local_client_test.go
  • mock_ssh_test.go
  • modules_adv_test.go
  • modules_cmd_test.go
  • modules_file_test.go
  • modules_infra_test.go
  • modules_svc_test.go
  • parser.go
  • parser_test.go
  • ssh.go
  • ssh_test.go
  • types_test.go

Comment thread ax7_public_test.go
Comment on lines +48 to +119
func ax7ConnectedSSHClient(t *core.T, responses map[string]string) *SSHClient {
t.Helper()
serverConfig := &ssh.ServerConfig{NoClientAuth: true}
key, err := rsa.GenerateKey(rand.Reader, 1024)
core.RequireNoError(t, err)
signer, err := ssh.NewSignerFromKey(key)
core.RequireNoError(t, err)
serverConfig.AddHostKey(signer)

listener, err := net.Listen("tcp", "127.0.0.1:0")
core.RequireNoError(t, err)
go func() {
defer listener.Close()
serverConn, err := listener.Accept()
if err != nil {
return
}
conn, chans, reqs, err := ssh.NewServerConn(serverConn, serverConfig)
if err != nil {
return
}
defer conn.Close()
go ssh.DiscardRequests(reqs)
for next := range chans {
if next.ChannelType() != "session" {
_ = next.Reject(ssh.UnknownChannelType, "session required")
continue
}
channel, requests, err := next.Accept()
if err != nil {
continue
}
go func() {
defer channel.Close()
for req := range requests {
if req.Type != "exec" {
_ = req.Reply(false, nil)
continue
}
var payload struct{ Command string }
ssh.Unmarshal(req.Payload, &payload)
if req.WantReply {
_ = req.Reply(true, nil)
}
out := responses[payload.Command]
if out == "" {
for key, value := range responses {
if strings.Contains(payload.Command, key) {
out = value
break
}
}
}
_, _ = io.WriteString(channel, out)
_, _ = channel.SendRequest("exit-status", false, ssh.Marshal(struct{ Status uint32 }{0}))
return
}
}()
}
}()
clientConn, err := net.Dial("tcp", listener.Addr().String())
core.RequireNoError(t, err)

clientConfig := &ssh.ClientConfig{
User: "tester",
HostKeyCallback: ssh.InsecureIgnoreHostKey(),
Timeout: time.Second,
}
conn, chans, reqs, err := ssh.NewClientConn(clientConn, "pipe", clientConfig)
core.RequireNoError(t, err)
return &SSHClient{client: ssh.NewClient(conn, chans, reqs), host: "pipe", port: 22, user: "tester"}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add automatic teardown to ax7ConnectedSSHClient.

This helper returns a live SSHClient, but most callers in this file never close it. That leaves the background server goroutine and TCP connection running past the end of each test, which can accumulate across this suite and make later failures harder to reason about. Please register cleanup here so every caller gets deterministic teardown.

🧰 Tools
🪛 ast-grep (0.42.1)

[warning] 50-50: RSA keys should be at least 2048 bits.
Context: 1024
Note: [CWE-326] Inadequate Encryption Strength. [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#algorithms

(use-of-weak-rsa-key-go)

🪛 GitHub Check: SonarCloud Code Analysis

[failure] 48-48: Refactor this method to reduce its Cognitive Complexity from 44 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=dAppCore_go-ansible&issues=AZ3Vg2XVnCBQP4QQLx4E&open=AZ3Vg2XVnCBQP4QQLx4E&pullRequest=5

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ax7_public_test.go` around lines 48 - 119, The helper ax7ConnectedSSHClient
currently leaves a running server goroutine and open connections; register a
deterministic teardown by calling t.Cleanup(...) before returning to close the
returned *SSHClient and the listener (and any raw clientConn/conn if necessary)
so the server goroutine unblocks and exits. Capture listener and the SSHClient
value in the cleanup closure and invoke their Close methods (or SSHClient.Close)
to ensure the TCP listener and SSH client connection are closed when the test
finishes.

Comment on lines +3 to +28
import core "dappco.re/go"

func TestAX7_Register_Good(t *core.T) {
app := core.New()
Register(app)
ansible := app.Command("ansible")
core.AssertTrue(t, ansible.OK)
core.AssertNotNil(t, ansible.Value)
}

func TestAX7_Register_Bad(t *core.T) {
var app *core.Core
core.AssertPanics(t, func() {
Register(app)
})
core.AssertNil(t, app)
}

func TestAX7_Register_Ugly(t *core.T) {
app := core.New()
Register(app)
Register(app)
testCommand := app.Command("ansible/test")
core.AssertTrue(t, testCommand.OK)
core.AssertNotNil(t, testCommand.Value)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check the actual file content
cat -n cmd/ansible/ax7_register_test.go | head -30

Repository: dAppCore/go-ansible

Length of output: 837


🏁 Script executed:

# Search for testify usage in the codebase to understand the expected pattern
rg "testify" --type go -c | head -20

Repository: dAppCore/go-ansible

Length of output: 45


🏁 Script executed:

# Check other test files in cmd/ansible to see the pattern
find cmd/ansible -name "*_test.go" -type f | head -5

Repository: dAppCore/go-ansible

Length of output: 124


🏁 Script executed:

# Look at an existing test file to see the assertion pattern
rg "assert\.|require\." cmd/ansible --type go -A 2 -B 2 | head -30

Repository: dAppCore/go-ansible

Length of output: 45


🏁 Script executed:

# Check for CLAUDE.md or similar guidelines file
find . -name "CLAUDE.md" -o -name "guidelines*" -o -name "RULES*" | head -5

Repository: dAppCore/go-ansible

Length of output: 75


🏁 Script executed:

# Check CLAUDE.md for the test guidelines
cat CLAUDE.md

Repository: dAppCore/go-ansible

Length of output: 4820


🏁 Script executed:

# Check if testify is actually imported/used in other test files
rg "testify" --type go | head -20

Repository: dAppCore/go-ansible

Length of output: 45


🏁 Script executed:

# Look at the other test file in cmd/ansible to see what pattern it uses
cat -n cmd/ansible/ansible_test.go | head -50

Repository: dAppCore/go-ansible

Length of output: 1536


Migrate to testify assertions and testing.T signature.

The test naming convention (_Good, _Bad, _Ugly) is correct and should be preserved. However, replace *core.T and core.Assert* calls with the standard *testing.T, testify/assert for soft assertions, and testify/require for hard assertions. This aligns with the repository's coding standards.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmd/ansible/ax7_register_test.go` around lines 3 - 28, Change the test
functions to use the standard testing.T signature and testify assertions: update
imports to include "testing", "github.com/stretchr/testify/assert", and
"github.com/stretchr/testify/require"; change each test signature from func
TestAX7_Register_*(t *core.T) to func TestAX7_Register_*(t *testing.T); replace
core.AssertTrue with assert.True(t, ...), core.AssertNotNil with
assert.NotNil(t, ...), core.AssertPanics with require.Panics(t, func() {
Register(app) }), and core.AssertNil with assert.Nil(t, app); keep the same test
names and calls to Register, core.New(), and
app.Command("ansible"/"ansible/test") so behavior is unchanged.

Comment thread executor_become_test.go

import (
"context"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in $(fd -a '^executor_become_test\.go$'); do
  echo "== $f =="
  rg -n 'core\.(Assert|Require)|dappco\.re/go"' "$f" || true
  rg -n 'testify/(assert|require)|\b(assert|require)\.' "$f" || true
done

Repository: dAppCore/go-ansible

Length of output: 795


Migrate become tests from core assertions back to testify.

Tests in executor_become_test.go use the core framework assertions (lines 97–102 and 129–133) instead of testify/assert and testify/require. Per coding guidelines for **/*_test.go, tests must use testify/assert for soft assertions and testify/require for hard assertions. Remove the core import at line 5 and replace all assertions with the appropriate testify functions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@executor_become_test.go` at line 5, Remove the core testing framework import
(core "dappco.re/go") and switch the tests in executor_become_test.go to use
testify: add imports "github.com/stretchr/testify/assert" and
"github.com/stretchr/testify/require", then replace the core assertions used in
the become tests (the assertion blocks previously at lines around the two
failing sections) with the appropriate testify calls—use require.* for hard
(fatal) checks (e.g., require.NoError, require.NotNil) and assert.* for soft
checks (e.g., assert.Equal, assert.True) so each core.Assert/core.Require usage
is converted to the corresponding assert or require function from testify.

Comment thread executor_extra_test.go

import (
"context"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in $(fd -a '^executor_extra_test\.go$'); do
  echo "== $f =="
  echo "-- core assertions --"
  rg -n 'core\.(Assert|Require)|dappco\.re/go"' "$f" || true
  echo "-- testify imports/usages --"
  rg -n 'testify/(assert|require)|\b(assert|require)\.' "$f" || true
done

Repository: dAppCore/go-ansible

Length of output: 21801


Replace core assertions with testify assertions throughout the test file.

The file imports and uses core.Require* and core.Assert* from dappco.re/go (line 5) across all test functions, but repository standards require testify/assert for soft assertions and testify/require for hard assertions. Update the import and replace all core.Assert* with assert.* and all core.Require* with require.* from the testify package.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@executor_extra_test.go` at line 5, Replace the import of core
("dappco.re/go") in executor_extra_test.go with the testify packages and update
all assertion usages: change the import to include
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
replace every call to core.Assert* with assert.* and every core.Require* with
require.* (e.g., core.AssertEqual → assert.Equal, core.RequireNoError →
require.NoError) ensuring function names map to their testify equivalents
throughout the test file.

Comment thread executor_test.go

import (
"context"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "executor_test.go" -type f

Repository: dAppCore/go-ansible

Length of output: 82


🏁 Script executed:

git ls-files | grep -E "(executor|test)" | head -20

Repository: dAppCore/go-ansible

Length of output: 457


🏁 Script executed:

fd executor_test.go

Repository: dAppCore/go-ansible

Length of output: 80


🏁 Script executed:

head -50 executor_test.go

Repository: dAppCore/go-ansible

Length of output: 1080


🏁 Script executed:

wc -l executor_test.go

Repository: dAppCore/go-ansible

Length of output: 85


🏁 Script executed:

rg "testify" executor_test.go

Repository: dAppCore/go-ansible

Length of output: 45


🏁 Script executed:

rg "core\.Assert|core\.Require" executor_test.go | head -30

Repository: dAppCore/go-ansible

Length of output: 1316


🏁 Script executed:

cat CLAUDE.md

Repository: dAppCore/go-ansible

Length of output: 4820


Replace dappco.re/go assertions with testify/assert and testify/require.

This file has been migrated to core "dappco.re/go" for assertions, but the repository standard (per CLAUDE.md) requires testify/assert for soft assertions and testify/require for hard assertions in all *_test.go files. Revert the assertion calls throughout the file to use testify.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@executor_test.go` at line 5, Replace the custom assertion import core
"dappco.re/go" with the testify packages and update all assertion calls: import
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require"
and change core.X calls to assert.X (for non-fatal checks) or require.X (for
fatal checks) as appropriate — e.g., replace core.Equal -> require.Equal or
assert.Equal based on whether the test should abort, replace core.NoError ->
require.NoError when error must stop the test, etc.; ensure each assertion call
uses the *testing.T variable as the first argument (t) to match testify
signatures and remove the core import.

Comment thread modules_infra_test.go
import (
"context"
"testing"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "modules_infra_test.go" -type f

Repository: dAppCore/go-ansible

Length of output: 87


🏁 Script executed:

fd "modules_infra_test.go" -type f

Repository: dAppCore/go-ansible

Length of output: 234


🏁 Script executed:

head -50 modules_infra_test.go 2>/dev/null || fd "modules_infra_test.go" -exec head -50 {} \;

Repository: dAppCore/go-ansible

Length of output: 1272


🏁 Script executed:

rg "testify|assert|require" modules_infra_test.go 2>/dev/null || fd "modules_infra_test.go" -exec rg "testify|assert|require" {} \;

Repository: dAppCore/go-ansible

Length of output: 319


🏁 Script executed:

cat -n modules_infra_test.go | head -100

Repository: dAppCore/go-ansible

Length of output: 3207


🏁 Script executed:

rg "import\s*\(" modules_infra_test.go -A 10

Repository: dAppCore/go-ansible

Length of output: 280


🏁 Script executed:

rg "core\.(Assert|Require)" modules_infra_test.go | head -20

Repository: dAppCore/go-ansible

Length of output: 907


🏁 Script executed:

rg "testify" modules_infra_test.go

Repository: dAppCore/go-ansible

Length of output: 45


Restore testify assertions to comply with project standards.

Tests in this file are using core.Assert* methods from dappco.re/go instead of the mandated testify/assert and testify/require packages. Per project guidelines for **/*_test.go, soft assertions must use testify/assert and hard assertions must use testify/require. Replace all core.Assert* / core.Require* calls with appropriate testify equivalents throughout the file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules_infra_test.go` at line 5, The tests use core.Assert*/core.Require*
from dappco.re/go; replace these with testify equivalents: convert soft
assertions like core.AssertEqual, core.AssertTrue, core.AssertNil,
core.AssertNotNil, etc. to assert.Equal, assert.True, assert.Nil, assert.NotNil
and convert hard assertions like core.RequireNoError, core.RequireNil,
core.RequireEqual, etc. to require.NoError, require.Nil, require.Equal. Add the
testify imports (github.com/stretchr/testify/assert and
github.com/stretchr/testify/require) and remove or stop using core.* assertion
imports; update any helper calls (e.g., TestXxx functions that call core.Assert*
or core.Require*) to use the assert/require variables and adjust call order
(require before further checks) so tests compile and follow project standards.

Comment thread modules_svc_test.go

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

fd modules_svc_test.go

Repository: dAppCore/go-ansible

Length of output: 83


🏁 Script executed:

wc -l modules_svc_test.go

Repository: dAppCore/go-ansible

Length of output: 88


🏁 Script executed:

head -50 modules_svc_test.go

Repository: dAppCore/go-ansible

Length of output: 1564


🏁 Script executed:

grep -n "core\." modules_svc_test.go | head -20

Repository: dAppCore/go-ansible

Length of output: 997


Revert to testify for test assertions in this file.

This file uses core from dappco.re/go for assertions instead of testify/assert and testify/require, which violates the test standard for this repository. Swap all core.Assert* and core.Require* calls back to the appropriate testify equivalents, and change the import from core "dappco.re/go" to use testify packages directly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@modules_svc_test.go` at line 4, The file currently imports core
"dappco.re/go" and uses core.Assert*/core.Require* assertion helpers; replace
that import with the testify packages (github.com/stretchr/testify/assert and
github.com/stretchr/testify/require) and change every core.AssertX(...) call to
assert.X(...) and core.RequireX(...) to require.X(...). Update any aliased usage
(e.g., core.AssertEqual -> assert.Equal, core.RequireNoError -> require.NoError)
and ensure the testify packages are added to imports; keep function signatures
and test logic unchanged.

Comment thread parser_test.go
package ansible

import (
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in $(fd -a '^parser_test\.go$'); do
  echo "== $f =="
  rg -n 'core\.(Assert|Require)|dappco\.re/go"' "$f" || true
  rg -n 'testify/(assert|require)|\b(assert|require)\.' "$f" || true
done

Repository: dAppCore/go-ansible

Length of output: 17710


Replace core assertion helpers with testify assertions throughout.

Tests in this file use core.AssertEqual, core.RequireNoError, and similar helpers from the core package rather than testify/assert and testify/require. The coding guidelines require test files to use the testify framework for all assertions. Refactor the assertions across lines 4 and 26–1221 to use testify methods instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@parser_test.go` at line 4, Replace all usages of core's test helpers with
testify's equivalents: remove the core test helper import and add imports for
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
then update calls such that core.AssertEqual -> assert.Equal, core.RequireEqual
-> require.Equal, core.RequireNoError/core.AssertNoError ->
require.NoError/assert.NoError as appropriate, core.RequireTrue/core.AssertTrue
-> require.True/assert.True, etc., preserving the same argument order and
passing t (the *testing.T) as the first argument; ensure table-driven tests and
helper functions use assert/require consistently (use require for fatal
assertions that must stop the test and assert for non-fatal checks) across the
parser_test.go file where core helpers were used.

Comment thread ssh_test.go

import (
"testing"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in $(fd -a '^ssh_test\.go$'); do
  echo "== $f =="
  rg -n 'core\.(Assert|Require)|dappco\.re/go"' "$f" || true
  rg -n 'testify/(assert|require)|\b(assert|require)\.' "$f" || true
done

Repository: dAppCore/go-ansible

Length of output: 764


Replace core.Assert* calls with testify/assert and testify/require.

Tests in ssh_test.go use core.Assert* methods (imported at line 4, used throughout lines 16–49), which violates the test assertion guideline requiring testify/assert for soft assertions and testify/require for hard assertions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ssh_test.go` at line 4, Replace all usages of the custom core.Assert* helpers
in ssh_test.go with testify's assert and require calls: add imports
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require"
and remove (or stop using) core.Assert*; use require.* for hard assertions that
should stop the test (e.g., checks for non-nil errors or missing setup) and
assert.* for soft assertions (e.g., value equality or state checks that can
continue). Update each call site (the previous
core.AssertEqual/AssertTrue/AssertNil/AssertNotNil/etc.) to the corresponding
testify form that accepts t as the first arg (e.g., assert.Equal(t, expected,
actual), require.NoError(t, err), require.NotNil(t, obj), assert.True(t, cond)),
ensuring all tests compile and the import list reflects testify usage.

Comment thread types_test.go

import (
"testing"
core "dappco.re/go"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for f in $(fd -a '^types_test\.go$'); do
  echo "== $f =="
  rg -n 'core\.(Assert|Require)|dappco\.re/go"' "$f" || true
  rg -n 'testify/(assert|require)|\b(assert|require)\.' "$f" || true
done

Repository: dAppCore/go-ansible

Length of output: 10583


Migrate assertions to testify/assert and testify/require.

The test file uses core.Assert* and core.Require* methods, which does not align with the repository test framework policy. Replace the dappco.re/go assertion methods with testify/assert (soft assertions) and testify/require (hard assertions) throughout.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@types_test.go` at line 4, The tests import and use dappco.re/go's
core.Assert*/core.Require* helpers; replace that import with
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require",
update the test functions to accept t *testing.T if needed, and change all
core.AssertX(...) calls to assert.X(t, ...) and core.RequireX(...) calls to
require.X(t, ...). Ensure you remove the core import alias, adjust any call
argument ordering to match testify signatures, and run the test suite to verify
compile-time changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant