style(cli): flatten trust workspace resolve errors with %s

[euxaristia]

Summary

Vasanth's review is right: this path already redacts via redaction.ErrorMessage on a string, and nothing unwraps the temporary fmt.Errorf, so %w vs %s was never a leak.

This PR is only a small readability tidy: trustCurrentDir / trustRemove flatten the workspace-resolve error with %s before that inline redaction. Removed .jules/sentinel.md (agent artifact, not on main, and the "avoid %w near redactors" guidance was wrong).

Changes

• internal/cli/trust.go: keep %s flatten on the two resolve-workspace error sites
• drop .jules/sentinel.md

Test plan

• Re-read call path: ErrorMessage returns a string; writeAppError takes a string; no error escapes trust.go
• go test ./internal/cli/ -run Trust (or full package) still green

Summary by CodeRabbit

• Bug Fixes
  • Improved the formatting of workspace resolution errors in trust management commands for clearer, more readable messages.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e612dfe-abd4-4183-803d-7fa1b2c3aaea

📥 Commits

Reviewing files that changed from the base of the PR and between 84661f6 and 6418121.

📒 Files selected for processing (1)

• internal/cli/trust.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/cli/trust.go

---

Walkthrough

The CLI now formats workspace-resolution errors with %s in trustCurrentDir and trustRemove instead of wrapping them with %w.

Changes

Error formatting

Layer / File(s)	Summary
Stringify workspace-resolution errors internal/cli/trust.go	trustCurrentDir and trustRemove format workspace-resolution errors as strings instead of wrapped errors.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes changing trust workspace resolve errors to use %s formatting.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Vasanthdev2004]

Thanks for looking at this, but I don't think there's a leak here to close, and I'd rather not land the change as written.

The %w -> %s swap is a no-op. redaction.ErrorMessage returns a string (redaction.go:238-240 -> RedactError(err, opts).Message, which is just RedactString(err.Error(), opts)). The fmt.Errorf value at trust.go:47 and trust.go:89 is a temporary consumed inline; writeAppError (app.go:1158) takes a string. No error object ever escapes trust.go, so there is no caller who could reach the wrapped error via errors.Unwrap.

I drove the real entry point with a getwd that fails carrying a secret. Both verbs produce identical stderr:

trust           code=1 stderr="[zero] resolve workspace: getcwd: password=[REDACTED]\n"
trust remove    code=1 stderr="[zero] resolve workspace: getcwd: password=[REDACTED]\n"

And comparing the redacted strings directly:

redacted(%w) = "resolve workspace: getcwd failed: api_key=[REDACTED] path=/home/u/.zero"
redacted(%s) = "resolve workspace: getcwd failed: api_key=[REDACTED] path=/home/u/.zero"

The only thing that differs anywhere is RedactedError.Name (*fmt.wrapError vs *errors.errorString), and ErrorMessage throws away Name, Fields and Stack. Redaction on this path was already working — it's applied to the flattened err.Error() of the whole chain, so wrapping never mattered.

.jules/sentinel.md has to come out. It's not in origin/main (git ls-tree origin/main --name-only .jules/ is empty), it adds a new top-level directory, and it isn't gitignored — the repo already ignores .superpowers/ for exactly this class of local agent artifact. It's also broken on its own terms: od -c shows the \n sequences are literal backslash-n, so the whole thing renders as one unbroken paragraph rather than the three bullets you intended.

More importantly, the guidance it commits to the tree is backwards. "Avoid %w when formatting errors that contain secrets prior to redacting" would push future contributors to break error chains for no security benefit. The correct pattern is already in the repo at internal/providermodeldiscovery/discovery.go:452-460:

return fmt.Errorf("%s", redaction.RedactString(err.Error(), redaction.Options{...}))

Note the ordering: redact first, then flatten. That flatten is load-bearing there because redactDiscoveryError returns the error to a caller who could otherwise unwrap to the unredacted original. In trust.go there's no caller and no return, which is why the swap changes nothing.

A few smaller things while I'm here:

• The description says the change matches "the pattern used elsewhere in this file." trust.go has no other fmt.Errorf call — the only other fmt uses are Fprintf/Fprintln.
• Both test-plan boxes are unchecked, and no test was added. The getwd-error path in trust.go has no coverage at all: I reverted both lines back to %w and ran the full ./internal/cli/ suite — green, except the known pre-existing TestBuildServeScopeKeepsLexicalPaths symlink-privilege failure on Windows. Nothing pins this in either direction.
• I checked the rest of the path for anything the change misses. The other three redaction.ErrorMessage call sites in trust.go (lines 50, 63, 101) hand the raw error straight in and are already redacted. The cwd/target/roots printed on the success paths (lines 52, 72, 103) are unredacted on purpose — printing the path is what the command is for. Repo-wide, grepping for redaction.ErrorMessage|RedactError|RedactString on the same line as %w returns zero hits, so there's no sibling instance of this shape anywhere else.

Where I'd land: drop .jules/sentinel.md entirely. If you still want the two-line change for consistency with redactDiscoveryError, I'll take it as a readability tidy — but please retitle it and drop the security framing from the body, since the release notes currently say we fixed a sensitive-info exposure and we didn't.

If you were working from a scanner hit that flagged %w near a redactor, that rule needs the extra condition "...and the wrapped error is returned rather than immediately stringified" — otherwise it'll keep firing on safe code.

I checked this one myself rather than relying on a read-through, because "your change is a no-op" is a claim worth being sure about: ErrorMessage is declared func ErrorMessage(err error, options Options) string at redaction.go:238, and both call sites hand the fmt.Errorf value straight into it, so the error is stringified and redacted inline and no error object survives the call. That's the whole reason the verb doesn't matter here.

Nothing else queued behind this — it's the complete review.

[euxaristia]

Addressed review:

1. Dropped .jules/sentinel.md entirely.
2. Dropped the security framing. Title/body now describe this as a readability tidy only: %w -> %s on the two trust resolve-workspace sites, matching the flatten style elsewhere, not a leak fix. Redaction was already correct on this path because ErrorMessage stringifies inline.

[Vasanthdev2004]

Approving. Re-checked on 8314acd5 rather than standing on the earlier round: one file, +2/-2, .jules/sentinel.md is gone, and the title and body now describe this as the readability tidy it is instead of a leak fix.

Thanks for taking the correction cleanly, and for rewriting the framing rather than just dropping the file — the summary now says the right thing about why %w vs %s never mattered here, which is more useful to the next person reading this code than the original claim would have been.

On the change itself: flattening with %s before an inline redaction.ErrorMessage is fine and matches the style elsewhere. Nothing unwraps the temporary, writeAppError takes a string, so behaviour is byte-identical either way — which is exactly why it can land as a style change.

One thought, not a request: if you want the %s form to stay put, a short comment at either site saying the error is deliberately flattened because ErrorMessage stringifies inline would stop someone "fixing" it back to %w on the reasonable instinct that wrapping is always better. Entirely optional.

[euxaristia]

Addressed the optional review note: short comment at both trust resolve-workspace sites explaining the intentional %s flatten (ErrorMessage stringifies inline, so %w would be a no-op).