chore(dev→main): routine PR cadence for CodeRabbit review

[Snider]

Routine dev→main PR opened by Hephaestus PR-cadence task.

This PR exists to:

1. Trigger CodeRabbit auto-review of accumulated dev work
2. Surface any blocking review feedback before merge
3. Keep main current with dev once approved

ahead_by: 21 commit(s) (per gh api compare)

If CodeRabbit clears with no blocking comments, this PR is eligible for gh pr merge --merge (real merge commit, no force-push). Conflicts and review feedback should be addressed on the dev branch before merge.

Co-authored-by: Hephaestus hephaestus@cladius

Summary by CodeRabbit

Release Notes

• New Features

  • Added encrypted file storage with authenticated encryption support.
  • Introduced multiple storage backend integrations: GitHub, SFTP, WebDAV, and S3.
  • Added workspace management for organizing and switching between file collections.
  • Introduced REST API endpoints for file operations and workspace management.
  • Added mock storage implementation for testing.

• Chores

  • Updated module path and dependency versions for infrastructure alignment.

[coderabbitai]

Warning

Rate limit exceeded

@Snider has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 16 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a0e453f-019e-43bc-9196-7dc66a530c4e

📥 Commits

Reviewing files that changed from the base of the PR and between 72ea438 and 0891b7b.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (28)

• actions.go
• actions_test.go
• cube/actions.go
• cube/cube.go
• cube/cube_test.go
• go.mod
• internal/fsutil/direntry.go
• io.go
• mock.go
• node/node.go
• pkg/api/handlers.go
• pkg/api/handlers_test.go
• pkg/api/provider.go
• pkg/api/provider_test.go
• pkg/medium/github/github.go
• pkg/medium/sftp/sftp.go
• pkg/medium/webdav/webdav.go
• s3/actions.go
• s3/s3.go
• sigil/crypto_sigil_test.go
• sigil/sigil_test.go
• sigil/sigils.go
• store/medium.go
• store/store.go
• tests/cli/io/Taskfile.yaml
• workspace/service.go
• workspace/workspace.go
• workspace/workspace_test.go

📝 Walkthrough

Walkthrough

The pull request restructures the module from dappco.re/go/core/io to dappco.re/go/io, introduces comprehensive action handlers for local filesystem, in-memory, S3, SFTP, GitHub, and PWA storage operations, adds encrypted cube archive support with ChaCha20Poly1305, implements workspace management with encryption, adds HTTP API handlers for workspace and medium operations, and updates sigil/crypto handling.

Changes

Cohort / File(s)	Summary
Module Restructuring and Core Actions go.mod, io.go, actions.go, actions_test.go	Module renamed to dappco.re/go/io, adds 13 exported action name constants (local, memory, GitHub, PWA, SFTP, S3, Cube, copy operations), implements RegisterActions and ResetMemoryActionStore, removes standard library path helpers in favour of core utilities, refactors directory listing with custom insertion sort.
Cube Encrypted Archive Storage cube/cube.go, cube/cube_test.go, cube/actions.go, cube/actions_test.go	Implements encrypted file storage using ChaChaPolyAEAD sigil, wrapping inner coreio.Medium with transparent encryption/decryption, tar-based packing/unpacking to/from disk .cube files, and action handlers for read/write/pack/unpack operations with comprehensive test coverage.
Medium Implementations—GitHub pkg/medium/github/github.go, pkg/medium/github/github_test.go, pkg/medium/github/register.go	Read-only GitHub REST API medium for fetching repository content, supports ref/branch targeting and OAuth2 authentication, exposes read, list, and clone actions, returns ErrReadOnly for all write operations.
Medium Implementations—PWA pkg/medium/pwa/pwa.go, pkg/medium/pwa/pwa_test.go, pkg/medium/pwa/register.go	Stubbed PWA medium returning ErrNotImplemented for all operations, exposes scrape, read, list, and write action handlers.
Medium Implementations—SFTP pkg/medium/sftp/sftp.go, pkg/medium/sftp/sftp_test.go, pkg/medium/sftp/register.go	SFTP-backed medium supporting dial via SSH with password/key auth or injected client, implements full filesystem operations including streaming, exposes read and write actions.
Medium Implementations—WebDAV pkg/medium/webdav/webdav.go, pkg/medium/webdav/webdav_test.go, pkg/medium/webdav/register.go	WebDAV-backed medium with HTTP client configuration and basic auth support, implements all medium operations via WebDAV verbs (PROPFIND, MKCOL, PUT, MOVE, DELETE), exposes read and write actions.
Medium Implementations—S3 s3/s3.go, s3/s3_test.go, s3/actions.go, s3/actions_test.go	S3-backed medium with pluggable client, updates path handling to use core.CleanPath/core.PathBase, refactors body reading to use core.ReadAll, exposes read and write actions.
Local and Memory Mediums local/medium.go, mock.go	Updates isSymlink signature for mode type safety, adds new MockMedium for testing with thread-safe in-memory file storage, metadata tracking, and compile-time interface assertions.
Node/Archive Support node/node.go, node/node_test.go	Refactors tar serialisation to avoid bytes helpers via nodeArchiveBuffer and core utilities, replaces bytes.Reader with custom dataFileReader, updates ReadDir sorting to insertion sort, changes import path to dappco.re/go/io.
Cryptographic Sigils sigil/crypto_sigil.go, sigil/crypto_sigil_test.go, sigil/sigils.go, sigil/sigil_test.go	Changes NewChaChaPolySigil signature to accept nonce any with runtime validation rejecting fixed []byte nonces, adds InvalidNonceError, adds Nonce() method, refactors gzip/hash internals to avoid bytes/io via custom writer adapters and core utilities, rejects chacha20poly1305 scheme in NewSigil.
Workspace Management workspace/workspace.go, workspace/workspace_test.go, workspace/command.go, workspace/command_test.go, workspace/service.go, workspace/service_test.go	Implements encrypted workspace service managing named directories with file operations, renames Workspace interface to EncryptedWorkspace, adds WorkspaceCommand dispatch contract, uses core.SHA256Hex for ID derivation, adds custom workspaceSHA256Hash adapter for HKDF, supports legacy action identifiers.
HTTP API Layer pkg/api/provider.go, pkg/api/provider_test.go, pkg/api/handlers.go, pkg/api/handlers_test.go	Introduces IOProvider adapting go-io actions to HTTP via Gin, registers workspace and medium/file operation endpoints under /v1, dispatches RFC §15 actions through configurable core registry, supports memory and local media types, returns standardised JSON responses with content/entries conditionally included.
Import Updates and Refactoring datanode/medium.go, datanode/medium_test.go, store/medium.go, store/store.go, sqlite/sqlite.go	Updates coreio import paths from dappco.re/go/core/io to dappco.re/go/io, refactors path handling to use core.CleanPath/core.PathBase, changes NotFoundError to use core.E instead of errors.New, removes bytes.Reader wrapping in favour of custom readers.
Testing Infrastructure tests/cli/io/Taskfile.yaml	Adds build/test/vet task definitions, configures unit and integration test filtering with build tags, includes S3 endpoint environment variable guards.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title describes a routine PR cadence task for CodeRabbit review, which is tangentially related to the actual changeset but does not capture the primary or substantial changes (new crypto Medium, actions registry, multiple medium implementations, workspace service, API layer).	Update the title to reflect the major features introduced, such as 'feat: add Cube encryption Medium, action dispatch, and multi-protocol mediums (SFTP, WebDAV, GitHub, PWA)' or summarise the primary objective more clearly.
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.70% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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.}

[Snider]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 5

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

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

⚠️ Outside diff range comments (2)

store/store.go (1)

12-16: ⚠️ Potential issue | 🟠 Major

Keep NotFoundError as a plain sentinel.

NotFoundError should remain an errors.New(...) value; using core.E(...) changes the sentinel shape and makes the doc comment inaccurate. As per coding guidelines, sentinel errors should use standard errors.New() — only inline error returns in functions should use coreerr.E().

🔧 Proposed fix

 import (
 	"database/sql"
+	"errors"
 	"io/fs"
 	"text/template" // Note: AX-6 intrinsic - structural for KeyValueStore.Render templating; core exposes no template primitive.
@@
-var NotFoundError = core.E("store", "key not found", nil)
+var NotFoundError = errors.New("key not found")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@store/store.go` around lines 12 - 16, The NotFoundError sentinel should be a
plain errors.New value rather than core.E(...) so callers can reliably use
errors.Is identity checks; replace the current var NotFoundError =
core.E("store", "key not found", nil) with var NotFoundError =
errors.New("store: key not found"), update imports to include the standard
"errors" package and remove the use of core.E, and keep the existing doc comment
referencing errors.Is and the NotFoundError symbol.

s3/s3.go (1)

472-479: ⚠️ Potential issue | 🟠 Major

Don't treat body-read failures as an empty object.

If GetObject succeeds but reading out.Body fails, existing stays empty and the eventual close overwrites the object with only the appended suffix. Bubble the read error up instead of silently dropping the original content.

🔧 Proposed fix

 	if err == nil {
-		if content, readErr := readAllString(out.Body); readErr == nil {
-			existing = []byte(content)
-		}
+		content, readErr := readAllString(out.Body)
+		if readErr != nil {
+			return nil, core.E("s3.Append", core.Concat("failed to read existing object: ", key), readErr)
+		}
+		existing = []byte(content)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@s3/s3.go` around lines 472 - 479, The code currently treats a failure reading
out.Body as if the object were empty (leaving existing as an empty slice);
instead, when medium.client.GetObject succeeds but readAllString(out.Body)
returns an error, propagate that read error (do not overwrite existing), e.g.
set/return the readErr so the caller/Write flow fails rather than silently using
empty existing; locate the GetObject call and the readAllString usage around
medium.client.GetObject and out.Body, ensure out.Body is closed and that read
errors are returned or assigned to err (not ignored), so existing is only
populated on a successful read.

🟠 Major comments (17)

mock.go-109-118 (1)

109-118: ⚠️ Potential issue | 🟠 Major

Rename drops file metadata.

m.meta is not moved from oldPath to newPath, so mode/modtime become zeroed for the renamed file.

Proposed fix

 func (m *MockMedium) Rename(oldPath, newPath string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	f, ok := m.Files[oldPath]
 	if !ok {
 		return fs.ErrNotExist
 	}
 	m.Files[newPath] = f
 	delete(m.Files, oldPath)
+	if metadata, ok := m.meta[oldPath]; ok {
+		m.meta[newPath] = metadata
+		delete(m.meta, oldPath)
+	}
 	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mock.go` around lines 109 - 118, The Rename method on MockMedium is copying
the file data but not moving its metadata, causing mode/modtime to be lost;
update MockMedium.Rename to also move the entry in m.meta (e.g., set
m.meta[newPath] = m.meta[oldPath] and delete m.meta[oldPath]) inside the
existing mutex-protected block, and handle the case where no metadata exists (do
nothing) so metadata is preserved after renames.

mock.go-90-95 (1)

90-95: ⚠️ Potential issue | 🟠 Major

Delete silently succeeds for missing paths.

Returning nil when nothing was removed hides caller mistakes and diverges from the rest of the Medium implementations.

Proposed fix

 func (m *MockMedium) Delete(path string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	delete(m.Files, path)
-	return nil
+	if _, ok := m.Files[path]; ok {
+		delete(m.Files, path)
+		delete(m.meta, path)
+		return nil
+	}
+	if _, ok := m.dirs[path]; ok {
+		delete(m.dirs, path)
+		return nil
+	}
+	return fs.ErrNotExist
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mock.go` around lines 90 - 95, MockMedium.Delete currently always returns nil
even if the path wasn't present, which hides caller errors and diverges from
other Medium implementations; update MockMedium.Delete (the method on type
MockMedium that uses m.mu, m.Files) to check if the key exists before deleting
and return a not-found error (e.g., os.ErrNotExist or a clear fmt.Errorf) when
the path is missing, otherwise perform the delete and return nil.

mock.go-97-106 (1)

97-106: ⚠️ Potential issue | 🟠 Major

DeleteAll leaves stale directory/meta state and never reports missing paths.

It removes file bodies only, but not corresponding metadata or nested directory entries, and always returns nil.

Proposed fix

 func (m *MockMedium) DeleteAll(path string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
+	found := false
 	for k := range m.Files {
 		if k == path || len(k) > len(path) && k[:len(path)+1] == path+"/" {
 			delete(m.Files, k)
+			delete(m.meta, k)
+			found = true
 		}
 	}
-	delete(m.dirs, path)
-	return nil
+	for d := range m.dirs {
+		if d == path || len(d) > len(path) && d[:len(path)+1] == path+"/" {
+			delete(m.dirs, d)
+			found = true
+		}
+	}
+	if !found {
+		return fs.ErrNotExist
+	}
+	return nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mock.go` around lines 97 - 106, The DeleteAll implementation only deletes
file bodies and always returns nil, leaving stale directory entries and not
signaling missing paths; update MockMedium.DeleteAll to (1) check whether the
target path exists in either m.Files or m.dirs and return os.ErrNotExist if it
does not, (2) delete all file keys that equal path or start with path+"/", (3)
remove any directory entries in m.dirs that equal or are prefixed by path+"/",
and (4) also remove any associated metadata maps (e.g., m.meta or similar
metadata fields) for those files/dirs; keep the existing m.mu lock/unlock and
return nil only on successful deletion.

pkg/medium/webdav/webdav.go-67-70 (1)

67-70: ⚠️ Potential issue | 🟠 Major

Add timeout to HTTP client to prevent indefinite hangs.

The fallback HTTP client has no timeout, which causes operations to hang indefinitely under network failures or unresponsive servers. This affects all operations: Read, Write, Delete, Stat, List, and Rename.

Proposed fix

 	client := options.Client
 	if client == nil {
-		client = http.DefaultClient
+		client = &http.Client{Timeout: 30 * time.Second}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/medium/webdav/webdav.go` around lines 67 - 70, The current fallback
assignment uses http.DefaultClient (client := options.Client; if client == nil {
client = http.DefaultClient }) which has no timeout and can hang; change the
fallback to construct a dedicated *http.Client with a sensible Timeout (e.g.
15–30s) and assign that to client when options.Client is nil so all webdav
operations (Read, Write, Delete, Stat, List, Rename) use a timed client; ensure
you only replace the nil-case (options.Client == nil) and reuse the provided
options.Client unchanged.

s3/s3.go-72-85 (1)

72-85: ⚠️ Potential issue | 🟠 Major

Close the S3 body in readAllString and handle read errors in Append.

The readAllString helper (lines 72–85) reads from GetObject responses but never closes the reader. This causes HTTP connection leaks across Read, Open, and Append calls. AWS SDK for Go v2 requires callers to close io.ReadCloser bodies to avoid keeping connections open.

Additionally, in the Append method (lines 477–479), a read failure from the existing object is silently ignored—if readAllString fails, existing remains empty and the method proceeds to overwrite the object with only the appended content, causing data loss.

Fix readAllString to accept and close the reader:

🔧 Proposed fix for readAllString

-func readAllString(reader any) (string, error) {
+func readAllString(reader goio.ReadCloser) (string, error) {
+	defer reader.Close()
+
 	result := core.ReadAll(reader)
 	if !result.OK {
 		if err, ok := result.Value.(error); ok {
 			return "", err

Also fix Append to propagate read errors instead of swallowing them, so failed reads don't silently lose data.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@s3/s3.go` around lines 72 - 85, readAllString currently takes any and uses
core.ReadAll without closing the underlying body, leaking HTTP connections;
change readAllString to accept an io.ReadCloser (or io.Reader plus optional
closer), call core.ReadAll to get bytes/strings, and ensure you always close the
ReadCloser (defer rc.Close()) before returning, propagating any read errors as
returned errors (use fs.ErrInvalid only for type assertions). In the Append
method, stop swallowing read errors from readAllString: call readAllString on
the GetObject body, check the returned error and return that error (or wrap it
with context) instead of treating existing as empty, so a failed read does not
overwrite object data.

s3/actions.go-42-67 (1)

42-67: ⚠️ Potential issue | 🟠 Major

Guard against a nil medium after the type assertion.

A typed nil *Medium will pass the assertion and then panic on Read/Write. Treat nil as invalid in both handlers.

🔧 Suggested fix

 func readAction(_ context.Context, opts core.Options) core.Result {
 	medium, ok := opts.Get("medium").Value.(*Medium)
-	if !ok {
+	if !ok || medium == nil {
 		return core.Result{}.New(core.E("s3.readAction", "medium is required", fs.ErrInvalid))
 	}
 	content, err := medium.Read(opts.String("path"))
 	if err != nil {
@@
 func writeAction(_ context.Context, opts core.Options) core.Result {
 	medium, ok := opts.Get("medium").Value.(*Medium)
-	if !ok {
+	if !ok || medium == nil {
 		return core.Result{}.New(core.E("s3.writeAction", "medium is required", fs.ErrInvalid))
 	}
 	if err := medium.Write(opts.String("path"), opts.String("content")); err != nil {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@s3/actions.go` around lines 42 - 67, The readAction and writeAction handlers
currently only check the type assertion for opts.Get("medium").Value.(*Medium)
but don't guard against a typed nil Medium, which will panic when calling
Read/Write; after the type assertion in both readAction and writeAction, add an
explicit nil check (e.g., if medium == nil) and return a core.Result error using
core.E("s3.readAction", "medium is required", fs.ErrInvalid) for readAction and
core.E("s3.writeAction", "medium is required", fs.ErrInvalid) for writeAction so
that a nil *Medium is treated as invalid rather than causing a panic.

cube/actions.go-39-42 (1)

39-42: ⚠️ Potential issue | 🟠 Major

Don't overwrite the existing cube read/write contract.

These actions reuse core.io.cube.read and core.io.cube.write, but the new handlers only accept inner + key. The existing registry exercised in actions_test.go, Lines 400-429, already accepts a prebuilt medium. If both registrars are used on the same *core.Core, the later call silently narrows the contract and breaks current callers. Please either reuse the shared handler implementation or support both option shapes here.

Also applies to: 50-92

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cube/actions.go` around lines 39 - 42, The new registrations for
ActionRead/ActionWrite (and similarly for pack/unpack) replace the existing
core.io.cube.read/write contract by only accepting {inner, key}; update the
handlers (readAction, writeAction, packAction, unpackAction) to accept both the
original options shape (including a prebuilt medium) and the new {inner,key}
shape or delegate to the shared existing implementation used by the registry in
actions_test.go so the contract is not narrowed; locate the registrations
c.Action(ActionRead, readAction) / c.Action(ActionWrite, writeAction) and adjust
the corresponding handler functions to detect and handle both option forms (or
call the prior common handler) and preserve backward compatibility.

pkg/api/provider.go-27-31 (1)

27-31: ⚠️ Potential issue | 🟠 Major

Describe() is documenting an API that no longer exists.

The comments and route descriptions still say "seven currently wired" actions and 501 Not Implemented workspace endpoints, but RegisterRoutes mounts live handlers and pkg/api/handlers_test.go, Lines 24-174, already expects 200/400/404 behaviour. The request schema is stale as well: for example, the create-workspace tests post {"workspace":"alice"}, while the description still requires identifier and password. If Describe() feeds generated docs or clients, they'll be wrong out of the box.

Also applies to: 80-177

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/api/provider.go` around lines 27 - 31, Update Describe() so its text and
route/schema descriptions match the actual behaviour implemented by
RegisterRoutes and exercised by pkg/api/handlers_test.go: remove the stale
"seven currently wired" wording and any references to 501 endpoints, change
response codes to reflect 200/400/404 where appropriate, and update request body
schemas (e.g., accept "workspace" as used in tests instead of "identifier" and
"password"); ensure all route descriptions between the Describe() start and the
affected block (around the previous 80-177 region) are consistent with the live
handlers and tests.

actions_test.go-150-154 (1)

150-154: ⚠️ Potential issue | 🟠 Major

Please don't bake an unsandboxed "/" default into the local-action tests.

This assertion effectively blesses listing an arbitrary host path with no explicit root, which is the opposite of the filesystem boundary this package is supposed to enforce. Please either pass an explicit temp-rooted sandbox here or change the expectation so a missing root fails instead. As per coding guidelines, **/*_test.go: Use io.NewMemoryMedium() or io.NewSandboxed(t.TempDir()) in unit tests. Never hit real S3/SQLite unless performing integration testing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@actions_test.go` around lines 150 - 154, The test
`c.Action(coreio.ActionLocalList).Run(...)` is currently allowing an unsandboxed
root (defaults to "/"); change the test to use a sandboxed medium instead of
relying on the implicit root. Update the call that builds options (the
`core.NewOptions` / `core.Option{Key: "path", Value: tempDir}` setup) to provide
a sandboxed medium (e.g., `io.NewSandboxed(t.TempDir())` or
`io.NewMemoryMedium()`) or pass an explicit sandbox-rooted path, and adjust the
assertion so that listing without an explicit `root` fails if you choose to
enforce that; ensure you reference and modify the test around the
`ActionLocalList` invocation to supply the sandboxed medium per testing
guidelines.

workspace/service.go-270-286 (1)

270-286: ⚠️ Potential issue | 🟠 Major

Use the package's single workspace-name resolver here.

This fallback logic no longer matches WorkspaceCommand.workspaceName() in workspace/command.go, Lines 32-40, which prefers Workspace → WorkspaceID → Identifier. It also lets the create path fall through to CreateWorkspace(""), which deterministically hashes the empty identifier and creates or collides on the same workspace every time. Please reuse command.workspaceName() and reject blank names before calling CreateWorkspace or SwitchWorkspace.

Suggested fix

 	switch command.Action {
 	case WorkspaceCreateAction, legacyWorkspaceCreateAction:
-		passphrase := command.Password
-		identifier := command.Identifier
-		if identifier == "" {
-			identifier = command.Workspace
-		}
-		workspaceID, err := service.CreateWorkspace(identifier, passphrase)
+		identifier := command.workspaceName()
+		if identifier == "" {
+			return core.Result{}.New(core.E("workspace.HandleWorkspaceCommand", "workspace identifier is required", fs.ErrInvalid))
+		}
+		workspaceID, err := service.CreateWorkspace(identifier, command.Password)
 		if err != nil {
 			return core.Result{}.New(err)
 		}
 		return core.Result{Value: workspaceID, OK: true}
 	case WorkspaceSwitchAction, legacyWorkspaceSwitchAction:
-		workspaceID := command.WorkspaceID
-		if workspaceID == "" {
-			workspaceID = command.Workspace
-		}
+		workspaceID := command.workspaceName()
+		if workspaceID == "" {
+			return core.Result{}.New(core.E("workspace.HandleWorkspaceCommand", "workspace id is required", fs.ErrInvalid))
+		}
 		if err := service.SwitchWorkspace(workspaceID); err != nil {
 			return core.Result{}.New(err)
 		}
 		return core.Result{OK: true}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@workspace/service.go` around lines 270 - 286, The create/switch action
branches currently use ad-hoc fallback ordering and can call
CreateWorkspace(""); replace those fallbacks by calling the
command.workspaceName() helper to resolve the workspace name, validate the
returned name is non-empty and return an error/result for blank names, then pass
that validated name into service.CreateWorkspace(identifier, passphrase) for
WorkspaceCreateAction/legacyWorkspaceCreateAction and into
service.SwitchWorkspace(workspaceID) for
WorkspaceSwitchAction/legacyWorkspaceSwitchAction; reference the functions
command.workspaceName(), CreateWorkspace, and SwitchWorkspace when making the
change.

actions.go-82-85 (1)

82-85: ⚠️ Potential issue | 🟠 Major

Avoid double-registration of core.io.cube.* handlers.

These action names are also registered in cube/actions.go; registering both makes the active handler order-dependent and can silently change behaviour.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@actions.go` around lines 82 - 85, This file double-registers the core.io.cube
handlers: remove the redundant registrations in actions.go by deleting or
disabling the c.Action calls for
ActionCubeRead/ActionCubeWrite/ActionCubePack/ActionCubeUnpack (the lines that
register cubeReadAction, cubeWriteAction, cubePackAction, cubeUnpackAction) so
that the single authoritative registration in cube/actions.go remains; if you
need a runtime guard instead, wrap these calls in a clear conditional or lookup
that only registers when no existing handler for those ActionCube* names exists.

workspace/workspace.go-24-29 (1)

24-29: ⚠️ Potential issue | 🟠 Major

Avoid implicit fallback to global local storage.

Defaulting nil medium to coreio.Local can unintentionally route workspace data to host filesystem. Require explicit medium injection (or an explicitly sandboxed default).

As per coding guidelines, Use io.NewSandboxed(root) to define filesystem boundaries; the directory becomes an immutable root where all paths are relative. Never attempt to escape the sandbox root.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@workspace/workspace.go` around lines 24 - 29, The code currently silently
falls back to coreio.Local when medium is nil (in NewWorkspace/clipboard where
medium variable is set), which risks routing data to the host FS; remove that
implicit fallback and require an explicit medium: if medium == nil return an
error (use core.E with context like "workspace.NewWorkspace" and fs.ErrInvalid)
or accept a sandboxed default by requiring the caller to pass
io.NewSandboxed(root) — update the initialization around the medium variable to
enforce non-nil and document that callers must pass io.NewSandboxed(...) when a
filesystem medium is intended.

pkg/medium/github/github.go-93-108 (1)

93-108: ⚠️ Potential issue | 🟠 Major

Token file reads bypass the io.Medium abstraction.

This path uses direct os/filepath file access instead of the project’s Medium boundary model.

As per coding guidelines, All data access in the CoreGO ecosystem MUST go through the io.Medium interface. Never use raw os, filepath, or ioutil calls directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/medium/github/github.go` around lines 93 - 108, tokenFromEnvironment
currently uses os.Getenv, os.UserHomeDir, filepath.Join and os.ReadFile which
bypass the io.Medium abstraction; change tokenFromEnvironment to obtain
environment and filesystem data through the project's io.Medium (either by
adding an io.Medium parameter or using the existing Medium instance) and replace
os.Getenv/os.UserHomeDir/filepath.Join/os.ReadFile calls with the corresponding
io.Medium methods (e.g., Medium.Getenv, Medium.UserHomeDir, Medium.JoinPath or
PathJoin, and Medium.ReadFile) so all env and file access goes through the
Medium interface.

actions.go-505-515 (1)

505-515: ⚠️ Potential issue | 🟠 Major

Reject traversal paths from tar headers before writing.

Current extraction accepts names like ../x after only trimming /, which can break destination path guarantees on permissive media.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@actions.go` around lines 505 - 515, Reject tar header names that attempt
directory traversal before calling destination.WriteMode: use path.Clean (from
the path package, since tar headers use forward slashes) on header.Name (after
trimming leading "/"), then reject any cleaned name that is empty, equals "." or
starts with ".." or contains "/.." (this covers names like "../x" or
"a/../../b"), and also ensure absolute paths are not allowed; keep the existing
checks for trailing "/" (core.HasSuffix(name, "/")) and default mode handling,
and only call destination.WriteMode(name, string(content), mode) for names that
pass the traversal checks.

cube/cube.go-363-366 (1)

363-366: ⚠️ Potential issue | 🟠 Major

Do not suppress source listing errors in archive walks.

Line 365 returns success on List failure, which can silently produce incomplete cube archives.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cube/cube.go` around lines 363 - 366, The code is suppressing errors from
source.List by returning nil on failure; change the handling so listing errors
are propagated instead of treated as "nothing to archive." Locate the entries,
err := source.List(path) call and replace the early return with returning the
error (or wrapping it with context via fmt.Errorf or errors.Wrap) so the caller
receives the failure; ensure the surrounding function signature (the function
that performs the archive walk) returns an error and update callers if needed to
handle the propagated error.

cube/cube.go-426-435 (1)

426-435: ⚠️ Potential issue | 🟠 Major

Sanitise tar entry paths before writing them to destination.

extractTarToMedium does not reject traversal segments (e.g. ../), so malicious archives can violate path expectations on permissive media.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cube/cube.go` around lines 426 - 435, Sanitise the tar entry path before
writing: after computing name := core.TrimPrefix(header.Name, "/") and before
the HasSuffix check or destination.WriteMode call in extractTarToMedium, reject
any absolute paths, any paths containing traversal segments (e.g. ".."
components) or cleaned paths that escape the intended root (use path.Clean and
verify it does not start with ".." or contain "/../"), and return a
core.E("cube.extract", core.Concat("invalid tar entry path: ", name), nil) (or
similar) when rejected; only proceed to call destination.WriteMode(name, ...)
for validated, non-empty, non-directory-safe names.

actions.go-446-449 (1)

446-449: ⚠️ Potential issue | 🟠 Major

Propagate listing failures during archive walk.

Line 448 returns nil on List error, which can produce partial archives while reporting success.

Suggested fix

 func walkAndArchive(source Medium, archivePath string, tarWriter *tar.Writer) error {
 	entries, err := source.List(archivePath)
 	if err != nil {
-		return nil
+		return core.E("io.cube.archive", core.Concat("failed to list: ", archivePath), err)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@actions.go` around lines 446 - 449, The current archive walk swallows errors
from source.List(archivePath) by returning nil, causing partial archives to be
treated as success; change the error handling so that when entries, err :=
source.List(archivePath) returns a non-nil err you propagate that error (e.g.,
return err or wrap and return it) instead of returning nil. Locate the block
that calls source.List with variables entries and archivePath and replace the
nil return with an appropriate error return from the surrounding function so
failures during listing fail the overall operation.

🟡 Minor comments (3)

mock.go-165-165 (1)

165-165: ⚠️ Potential issue | 🟡 Minor

fs.FileInfo.Name() should be the base name, not the full path.

Using full paths here breaks expected fs.FileInfo semantics for callers.

Proposed fix

-		return NewFileInfo(path, int64(len(content)), mt.mode, mt.modTime, false), nil
+		name := path
+		if idx := strings.LastIndex(name, "/"); idx >= 0 {
+			name = name[idx+1:]
+		}
+		return NewFileInfo(name, int64(len(content)), mt.mode, mt.modTime, false), nil
@@
-	return &MockFile{Reader: bytes.NewReader([]byte(content)), info: NewFileInfo(path, int64(len(content)), mt.mode, mt.modTime, false)}, nil
+	name := path
+	if idx := strings.LastIndex(name, "/"); idx >= 0 {
+		name = name[idx+1:]
+	}
+	return &MockFile{Reader: bytes.NewReader([]byte(content)), info: NewFileInfo(name, int64(len(content)), mt.mode, mt.modTime, false)}, nil

Also applies to: 181-181

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mock.go` at line 165, The fs.FileInfo returned by NewFileInfo is being
created with the full path (e.g., NewFileInfo(path,...)) so fs.FileInfo.Name()
will return the full path instead of just the base name; update the NewFileInfo
calls to pass the base name (use filepath.Base(path) or pathpkg.Base(path)) so
Name() returns only the file's base name—change the occurrences where
NewFileInfo(path, int64(len(content)), mt.mode, mt.modTime, false) is used (and
the similar call at the other location) to NewFileInfo(filepath.Base(path),
int64(len(content)), mt.mode, mt.modTime, false).

sigil/crypto_sigil_test.go-191-193 (1)

191-193: ⚠️ Potential issue | 🟡 Minor

Use require before dereferencing err.

Both negative cases call err.Error() after assert.ErrorIs. If NewChaChaPolySigil ever returns nil unexpectedly, the test panics instead of failing cleanly.

🧪 Proposed fix

 			_, err := NewChaChaPolySigil(key, nonce)
-			assert.ErrorIs(t, err, InvalidNonceError)
+			require.ErrorIs(t, err, InvalidNonceError)
 			assert.Contains(t, err.Error(), "fixed-nonce []byte path removed; use PreObfuscator or nil")
@@
 	_, err := NewChaChaPolySigil(key, "fixed nonce")
-	assert.ErrorIs(t, err, InvalidNonceError)
+	require.ErrorIs(t, err, InvalidNonceError)
 	assert.Contains(t, err.Error(), "nonce must be PreObfuscator or nil")

Also applies to: 202-204

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sigil/crypto_sigil_test.go` around lines 191 - 193, The test dereferences err
with err.Error() after using assert.ErrorIs which can panic if err is nil;
update the checks around NewChaChaPolySigil so you first assert the error is
non-nil using require (e.g., require.ErrorIs(t, err, InvalidNonceError) or
require.NotNil(t, err)) before calling err.Error(), and apply the same change to
the analogous assertions around lines 202-204; reference NewChaChaPolySigil,
InvalidNonceError and the err variable when making the replacement.

sigil/sigil_test.go-293-297 (1)

293-297: ⚠️ Potential issue | 🟡 Minor

Fail fast before reading err.Error().

assert.Error does not stop the test. If this path unexpectedly returns nil, Line 296 panics and hides the real regression. Use a require assertion before checking the message.

🧪 Proposed fix

 func TestSigil_NewSigil_ChaChaPoly1305RequiresKey_Bad(t *testing.T) {
 	_, err := NewSigil("chacha20poly1305")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Contains(t, err.Error(), "scheme requires key material; use NewChaChaPolySigil")
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@sigil/sigil_test.go` around lines 293 - 297, In
TestSigil_NewSigil_ChaChaPoly1305RequiresKey_Bad, change the non-fatal assertion
to a fatal one: replace the assert.Error usage that checks the presence of an
error from NewSigil("chacha20poly1305") with require.Error so the test fails
immediately if err is nil before calling err.Error(); then keep the subsequent
check that the error message contains "scheme requires key material; use
NewChaChaPolySigil".

🧹 Nitpick comments (3)

tests/cli/io/Taskfile.yaml (1)

24-32: Move -tags integration before the package pattern for idiomatic Go style.

Both flag orderings work correctly in Go, but the build flag should precede the package pattern ./... to follow the standard convention documented in go test usage. Placing build flags after packages is valid but non-idiomatic.

Suggested fix

-        go test -count=1 ./... -run Integration -tags integration
+        go test -count=1 -tags integration ./... -run Integration

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/cli/io/Taskfile.yaml` around lines 24 - 32, Update the go test command
in the test-integration Taskfile target so the build tag flag appears before the
package pattern: modify the command using go test (in the test-integration
block) to place -tags integration directly before ./... (i.e., change the
ordering so "-tags integration" precedes the package pattern) to follow
idiomatic Go flag ordering.

node/node.go (1)

352-362: Consider de-duplicating sortDirEntriesByName.

This helper is now duplicated in multiple packages with the same logic. A shared internal helper would reduce drift risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@node/node.go` around lines 352 - 362, Duplicate insertion-sort helper
sortDirEntriesByName should be consolidated into a single shared internal
utility: extract the function (the insertion-sort logic that takes []fs.DirEntry
and sorts by Name()) into a common internal package (e.g., internal/fsutils or
pkg/util) and export a well-named function (e.g., SortDirEntriesByName); then
update all files that currently define their own sortDirEntriesByName to import
and call the shared SortDirEntriesByName and remove the duplicate
implementations so there is one canonical implementation to maintain.

cube/cube_test.go (1)

249-261: Use the sandboxed medium helper for consistency with test patterns.

This test uses local.New(tempDir) directly, which is the only instance in the test suite. The codebase already standardises on coreio.NewSandboxed(t.TempDir()) elsewhere (as seen in medium_test.go), and the coding guidelines explicitly mandate this pattern for unit tests. Swapping to coreio.NewSandboxed(tempDir) aligns this test with established conventions.

♻️ Suggested refactor

-	sandbox, err := local.New(tempDir)
+	sandbox, err := coreio.NewSandboxed(tempDir)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cube/cube_test.go` around lines 249 - 261, TestCube_Pack_Good uses
local.New(tempDir) directly instead of the project-standard sandbox helper;
replace the local.New call with coreio.NewSandboxed(tempDir) (keep the tempDir
usage and variable name sandbox) so the test creates a sandboxed medium
consistent with other tests, update error handling if needed to match the
NewSandboxed return signature, and ensure subsequent calls (sandbox.Exists)
continue to work with the returned sandbox type.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@actions.go`:
- Around line 398-402: The code currently constructs a host-root filesystem via
local.New("/") (creating localMedium) which bypasses sandboxing; replace
local.New("/") with io.NewSandboxed(sandboxRoot) and use that sandboxed medium
for all file operations (e.g., the localMedium used for WriteMode on outputPath
and the analogous read/write in the 410-417 block). Ensure sandboxRoot is the
intended immutable root (for example the action/workspace directory passed into
the routine), make all paths relative to that root and reject/normalize attempts
to escape (no ".." escapes), and keep existing error handling (return
core.E(...)) when creating the sandboxed medium fails.
- Around line 313-317: Remove the implicit default of "/" for the local action
root and require callers to provide a root; replace the call to local.New(root)
with io.NewSandboxed(root) (or the appropriate sandbox constructor) so the
filesystem is always sandboxed and paths are relative to that immutable root;
ensure you validate that opts.String("root") is non-empty and return an error
(or fail early) when it is missing instead of falling back to "/".

In `@cube/cube.go`:
- Around line 269-273: The Pack/Unpack/Open logic currently creates a filesystem
at host root via local.New("/") which allows escaping the sandbox; update these
to create a sandboxed filesystem using io.NewSandboxed(root) (where root is the
configured sandbox base) and use that returned medium for all file operations
(e.g. replace local.New("/") calls in Pack, Unpack, and Open with
io.NewSandboxed(root)), ensure paths like outputPath (and any input paths in the
other blocks) are treated as relative to the sandbox and do not attempt to join
or resolve outside the sandbox, and keep error handling the same but reference
the sandboxed medium variable (e.g. localMedium) for WriteMode/Read/Open calls.

In `@pkg/api/handlers.go`:
- Around line 229-233: The handler currently accepts client-supplied req.Root
and passes it to coreio.NewSandboxed, which allows callers to pick arbitrary
filesystem roots; replace this by sourcing the sandbox root from trusted server
configuration or an allow-list (e.g., a configured ServerConfig.SandboxRoot or
AllowedRoots map) instead of using req.Root, remove or ignore the
strings.TrimSpace(req.Root) path-check and the call
coreio.NewSandboxed(req.Root), validate that the configured/allowed root exists
and is safe, call coreio.NewSandboxed with that trusted root, and return a clear
error if no configured root is available or the requested allowed identifier is
not permitted.

In `@pkg/api/provider.go`:
- Around line 9-13: The build fails because the imported module alias goapi
(dappco.re/go/api v0.8.0-alpha.1) referenced in pkg/api/provider.go has no
checksum in go.sum; run module tooling from the repository root to populate
go.sum (e.g. run go get dappco.re/go/api@v0.8.0-alpha.1 or simply go mod tidy)
so the checksum entry for dappco.re/go/api is added, then commit the updated
go.sum so imports like the goapi alias in pkg/api/provider.go resolve during CI
and local builds.

---

Outside diff comments:
In `@s3/s3.go`:
- Around line 472-479: The code currently treats a failure reading out.Body as
if the object were empty (leaving existing as an empty slice); instead, when
medium.client.GetObject succeeds but readAllString(out.Body) returns an error,
propagate that read error (do not overwrite existing), e.g. set/return the
readErr so the caller/Write flow fails rather than silently using empty
existing; locate the GetObject call and the readAllString usage around
medium.client.GetObject and out.Body, ensure out.Body is closed and that read
errors are returned or assigned to err (not ignored), so existing is only
populated on a successful read.

In `@store/store.go`:
- Around line 12-16: The NotFoundError sentinel should be a plain errors.New
value rather than core.E(...) so callers can reliably use errors.Is identity
checks; replace the current var NotFoundError = core.E("store", "key not found",
nil) with var NotFoundError = errors.New("store: key not found"), update imports
to include the standard "errors" package and remove the use of core.E, and keep
the existing doc comment referencing errors.Is and the NotFoundError symbol.

---

Major comments:
In `@actions_test.go`:
- Around line 150-154: The test `c.Action(coreio.ActionLocalList).Run(...)` is
currently allowing an unsandboxed root (defaults to "/"); change the test to use
a sandboxed medium instead of relying on the implicit root. Update the call that
builds options (the `core.NewOptions` / `core.Option{Key: "path", Value:
tempDir}` setup) to provide a sandboxed medium (e.g.,
`io.NewSandboxed(t.TempDir())` or `io.NewMemoryMedium()`) or pass an explicit
sandbox-rooted path, and adjust the assertion so that listing without an
explicit `root` fails if you choose to enforce that; ensure you reference and
modify the test around the `ActionLocalList` invocation to supply the sandboxed
medium per testing guidelines.

In `@actions.go`:
- Around line 82-85: This file double-registers the core.io.cube handlers:
remove the redundant registrations in actions.go by deleting or disabling the
c.Action calls for
ActionCubeRead/ActionCubeWrite/ActionCubePack/ActionCubeUnpack (the lines that
register cubeReadAction, cubeWriteAction, cubePackAction, cubeUnpackAction) so
that the single authoritative registration in cube/actions.go remains; if you
need a runtime guard instead, wrap these calls in a clear conditional or lookup
that only registers when no existing handler for those ActionCube* names exists.
- Around line 505-515: Reject tar header names that attempt directory traversal
before calling destination.WriteMode: use path.Clean (from the path package,
since tar headers use forward slashes) on header.Name (after trimming leading
"/"), then reject any cleaned name that is empty, equals "." or starts with ".."
or contains "/.." (this covers names like "../x" or "a/../../b"), and also
ensure absolute paths are not allowed; keep the existing checks for trailing "/"
(core.HasSuffix(name, "/")) and default mode handling, and only call
destination.WriteMode(name, string(content), mode) for names that pass the
traversal checks.
- Around line 446-449: The current archive walk swallows errors from
source.List(archivePath) by returning nil, causing partial archives to be
treated as success; change the error handling so that when entries, err :=
source.List(archivePath) returns a non-nil err you propagate that error (e.g.,
return err or wrap and return it) instead of returning nil. Locate the block
that calls source.List with variables entries and archivePath and replace the
nil return with an appropriate error return from the surrounding function so
failures during listing fail the overall operation.

In `@cube/actions.go`:
- Around line 39-42: The new registrations for ActionRead/ActionWrite (and
similarly for pack/unpack) replace the existing core.io.cube.read/write contract
by only accepting {inner, key}; update the handlers (readAction, writeAction,
packAction, unpackAction) to accept both the original options shape (including a
prebuilt medium) and the new {inner,key} shape or delegate to the shared
existing implementation used by the registry in actions_test.go so the contract
is not narrowed; locate the registrations c.Action(ActionRead, readAction) /
c.Action(ActionWrite, writeAction) and adjust the corresponding handler
functions to detect and handle both option forms (or call the prior common
handler) and preserve backward compatibility.

In `@cube/cube.go`:
- Around line 363-366: The code is suppressing errors from source.List by
returning nil on failure; change the handling so listing errors are propagated
instead of treated as "nothing to archive." Locate the entries, err :=
source.List(path) call and replace the early return with returning the error (or
wrapping it with context via fmt.Errorf or errors.Wrap) so the caller receives
the failure; ensure the surrounding function signature (the function that
performs the archive walk) returns an error and update callers if needed to
handle the propagated error.
- Around line 426-435: Sanitise the tar entry path before writing: after
computing name := core.TrimPrefix(header.Name, "/") and before the HasSuffix
check or destination.WriteMode call in extractTarToMedium, reject any absolute
paths, any paths containing traversal segments (e.g. ".." components) or cleaned
paths that escape the intended root (use path.Clean and verify it does not start
with ".." or contain "/../"), and return a core.E("cube.extract",
core.Concat("invalid tar entry path: ", name), nil) (or similar) when rejected;
only proceed to call destination.WriteMode(name, ...) for validated, non-empty,
non-directory-safe names.

In `@mock.go`:
- Around line 109-118: The Rename method on MockMedium is copying the file data
but not moving its metadata, causing mode/modtime to be lost; update
MockMedium.Rename to also move the entry in m.meta (e.g., set m.meta[newPath] =
m.meta[oldPath] and delete m.meta[oldPath]) inside the existing mutex-protected
block, and handle the case where no metadata exists (do nothing) so metadata is
preserved after renames.
- Around line 90-95: MockMedium.Delete currently always returns nil even if the
path wasn't present, which hides caller errors and diverges from other Medium
implementations; update MockMedium.Delete (the method on type MockMedium that
uses m.mu, m.Files) to check if the key exists before deleting and return a
not-found error (e.g., os.ErrNotExist or a clear fmt.Errorf) when the path is
missing, otherwise perform the delete and return nil.
- Around line 97-106: The DeleteAll implementation only deletes file bodies and
always returns nil, leaving stale directory entries and not signaling missing
paths; update MockMedium.DeleteAll to (1) check whether the target path exists
in either m.Files or m.dirs and return os.ErrNotExist if it does not, (2) delete
all file keys that equal path or start with path+"/", (3) remove any directory
entries in m.dirs that equal or are prefixed by path+"/", and (4) also remove
any associated metadata maps (e.g., m.meta or similar metadata fields) for those
files/dirs; keep the existing m.mu lock/unlock and return nil only on successful
deletion.

In `@pkg/api/provider.go`:
- Around line 27-31: Update Describe() so its text and route/schema descriptions
match the actual behaviour implemented by RegisterRoutes and exercised by
pkg/api/handlers_test.go: remove the stale "seven currently wired" wording and
any references to 501 endpoints, change response codes to reflect 200/400/404
where appropriate, and update request body schemas (e.g., accept "workspace" as
used in tests instead of "identifier" and "password"); ensure all route
descriptions between the Describe() start and the affected block (around the
previous 80-177 region) are consistent with the live handlers and tests.

In `@pkg/medium/github/github.go`:
- Around line 93-108: tokenFromEnvironment currently uses os.Getenv,
os.UserHomeDir, filepath.Join and os.ReadFile which bypass the io.Medium
abstraction; change tokenFromEnvironment to obtain environment and filesystem
data through the project's io.Medium (either by adding an io.Medium parameter or
using the existing Medium instance) and replace
os.Getenv/os.UserHomeDir/filepath.Join/os.ReadFile calls with the corresponding
io.Medium methods (e.g., Medium.Getenv, Medium.UserHomeDir, Medium.JoinPath or
PathJoin, and Medium.ReadFile) so all env and file access goes through the
Medium interface.

In `@pkg/medium/webdav/webdav.go`:
- Around line 67-70: The current fallback assignment uses http.DefaultClient
(client := options.Client; if client == nil { client = http.DefaultClient })
which has no timeout and can hang; change the fallback to construct a dedicated
*http.Client with a sensible Timeout (e.g. 15–30s) and assign that to client
when options.Client is nil so all webdav operations (Read, Write, Delete, Stat,
List, Rename) use a timed client; ensure you only replace the nil-case
(options.Client == nil) and reuse the provided options.Client unchanged.

In `@s3/actions.go`:
- Around line 42-67: The readAction and writeAction handlers currently only
check the type assertion for opts.Get("medium").Value.(*Medium) but don't guard
against a typed nil Medium, which will panic when calling Read/Write; after the
type assertion in both readAction and writeAction, add an explicit nil check
(e.g., if medium == nil) and return a core.Result error using
core.E("s3.readAction", "medium is required", fs.ErrInvalid) for readAction and
core.E("s3.writeAction", "medium is required", fs.ErrInvalid) for writeAction so
that a nil *Medium is treated as invalid rather than causing a panic.

In `@s3/s3.go`:
- Around line 72-85: readAllString currently takes any and uses core.ReadAll
without closing the underlying body, leaking HTTP connections; change
readAllString to accept an io.ReadCloser (or io.Reader plus optional closer),
call core.ReadAll to get bytes/strings, and ensure you always close the
ReadCloser (defer rc.Close()) before returning, propagating any read errors as
returned errors (use fs.ErrInvalid only for type assertions). In the Append
method, stop swallowing read errors from readAllString: call readAllString on
the GetObject body, check the returned error and return that error (or wrap it
with context) instead of treating existing as empty, so a failed read does not
overwrite object data.

In `@workspace/service.go`:
- Around line 270-286: The create/switch action branches currently use ad-hoc
fallback ordering and can call CreateWorkspace(""); replace those fallbacks by
calling the command.workspaceName() helper to resolve the workspace name,
validate the returned name is non-empty and return an error/result for blank
names, then pass that validated name into service.CreateWorkspace(identifier,
passphrase) for WorkspaceCreateAction/legacyWorkspaceCreateAction and into
service.SwitchWorkspace(workspaceID) for
WorkspaceSwitchAction/legacyWorkspaceSwitchAction; reference the functions
command.workspaceName(), CreateWorkspace, and SwitchWorkspace when making the
change.

In `@workspace/workspace.go`:
- Around line 24-29: The code currently silently falls back to coreio.Local when
medium is nil (in NewWorkspace/clipboard where medium variable is set), which
risks routing data to the host FS; remove that implicit fallback and require an
explicit medium: if medium == nil return an error (use core.E with context like
"workspace.NewWorkspace" and fs.ErrInvalid) or accept a sandboxed default by
requiring the caller to pass io.NewSandboxed(root) — update the initialization
around the medium variable to enforce non-nil and document that callers must
pass io.NewSandboxed(...) when a filesystem medium is intended.

---

Minor comments:
In `@mock.go`:
- Line 165: The fs.FileInfo returned by NewFileInfo is being created with the
full path (e.g., NewFileInfo(path,...)) so fs.FileInfo.Name() will return the
full path instead of just the base name; update the NewFileInfo calls to pass
the base name (use filepath.Base(path) or pathpkg.Base(path)) so Name() returns
only the file's base name—change the occurrences where NewFileInfo(path,
int64(len(content)), mt.mode, mt.modTime, false) is used (and the similar call
at the other location) to NewFileInfo(filepath.Base(path), int64(len(content)),
mt.mode, mt.modTime, false).

In `@sigil/crypto_sigil_test.go`:
- Around line 191-193: The test dereferences err with err.Error() after using
assert.ErrorIs which can panic if err is nil; update the checks around
NewChaChaPolySigil so you first assert the error is non-nil using require (e.g.,
require.ErrorIs(t, err, InvalidNonceError) or require.NotNil(t, err)) before
calling err.Error(), and apply the same change to the analogous assertions
around lines 202-204; reference NewChaChaPolySigil, InvalidNonceError and the
err variable when making the replacement.

In `@sigil/sigil_test.go`:
- Around line 293-297: In TestSigil_NewSigil_ChaChaPoly1305RequiresKey_Bad,
change the non-fatal assertion to a fatal one: replace the assert.Error usage
that checks the presence of an error from NewSigil("chacha20poly1305") with
require.Error so the test fails immediately if err is nil before calling
err.Error(); then keep the subsequent check that the error message contains
"scheme requires key material; use NewChaChaPolySigil".

---

Nitpick comments:
In `@cube/cube_test.go`:
- Around line 249-261: TestCube_Pack_Good uses local.New(tempDir) directly
instead of the project-standard sandbox helper; replace the local.New call with
coreio.NewSandboxed(tempDir) (keep the tempDir usage and variable name sandbox)
so the test creates a sandboxed medium consistent with other tests, update error
handling if needed to match the NewSandboxed return signature, and ensure
subsequent calls (sandbox.Exists) continue to work with the returned sandbox
type.

In `@node/node.go`:
- Around line 352-362: Duplicate insertion-sort helper sortDirEntriesByName
should be consolidated into a single shared internal utility: extract the
function (the insertion-sort logic that takes []fs.DirEntry and sorts by Name())
into a common internal package (e.g., internal/fsutils or pkg/util) and export a
well-named function (e.g., SortDirEntriesByName); then update all files that
currently define their own sortDirEntriesByName to import and call the shared
SortDirEntriesByName and remove the duplicate implementations so there is one
canonical implementation to maintain.

In `@tests/cli/io/Taskfile.yaml`:
- Around line 24-32: Update the go test command in the test-integration Taskfile
target so the build tag flag appears before the package pattern: modify the
command using go test (in the test-integration block) to place -tags integration
directly before ./... (i.e., change the ordering so "-tags integration" precedes
the package pattern) to follow idiomatic Go flag ordering.

🪄 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: 57a62775-c9da-4bc7-8151-a00805f6614b

📥 Commits

Reviewing files that changed from the base of the PR and between fba13d5 and 72ea438.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (48)

• actions.go
• actions_test.go
• cube/actions.go
• cube/actions_test.go
• cube/cube.go
• cube/cube_test.go
• datanode/medium.go
• datanode/medium_test.go
• go.mod
• io.go
• local/medium.go
• mock.go
• node/node.go
• node/node_test.go
• pkg/api/handlers.go
• pkg/api/handlers_test.go
• pkg/api/provider.go
• pkg/api/provider_test.go
• pkg/medium/github/github.go
• pkg/medium/github/github_test.go
• pkg/medium/github/register.go
• pkg/medium/pwa/pwa.go
• pkg/medium/pwa/pwa_test.go
• pkg/medium/pwa/register.go
• pkg/medium/sftp/register.go
• pkg/medium/sftp/sftp.go
• pkg/medium/sftp/sftp_test.go
• pkg/medium/webdav/register.go
• pkg/medium/webdav/webdav.go
• pkg/medium/webdav/webdav_test.go
• s3/actions.go
• s3/actions_test.go
• s3/s3.go
• s3/s3_test.go
• sigil/crypto_sigil.go
• sigil/crypto_sigil_test.go
• sigil/sigil_test.go
• sigil/sigils.go
• sqlite/sqlite.go
• store/medium.go
• store/store.go
• tests/cli/io/Taskfile.yaml
• workspace/command.go
• workspace/command_test.go
• workspace/service.go
• workspace/service_test.go
• workspace/workspace.go
• workspace/workspace_test.go

[sonarqubecloud]

Quality Gate failed

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

See analysis details on SonarQube Cloud