fix(debug): drop redundant output hint (Fixes #1732)

[deepujain]

fix(debug): drop redundant --output hint after tarball creation (Fixes #1732)

Summary

Fixes #1732.

nemoclaw debug --output ... already tells the user where the tarball was written, warns about redaction, and says to attach that file to the issue. The command then printed a second message telling the user to run the same command with --output, which was redundant and confusing.

This change makes the completion text depend on whether a tarball was already written. When --output is present, the command now ends with a short confirmation instead of repeating the --output example.

Changes

• src/lib/debug.ts: add a small helper for the final debug messages and use it from runDebug().
• src/lib/debug.test.ts: add coverage for the completion message with and without --output.

Testing

• npm run build:cli
• npm run typecheck:cli
• npm test -- src/lib/debug.test.ts src/lib/debug-command.test.ts
• npm test

Evidence it works

• The new helper test verifies that nemoclaw debug --output ... no longer prints the redundant suggestion to rerun with --output.
• The touched-path build and unit tests pass locally.
• Full npm test still hits unrelated pre-existing suite failures in this environment, including:
  • test/install-preflight.test.ts
  • test/legacy-path-guard.test.ts
  • src/lib/preflight.test.ts
  • src/lib/sandbox-version.test.ts

Signed-off-by: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

• Tests

  • Added test coverage verifying completion guidance is emitted when no output path is provided and suppressed when an output path is present.

• Refactor

  • Made completion guidance generation dynamic so final instructions are conditionally shown based on whether an output path was specified, replacing previous hardcoded messaging.

[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: 822f3ef1-5eee-4922-b085-a1a874563685

📥 Commits

Reviewing files that changed from the base of the PR and between 19da3f4 and d93353f.

📒 Files selected for processing (2)

• src/lib/debug.test.ts
• src/lib/debug.ts

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

• src/lib/debug.ts
• src/lib/debug.test.ts

---

📝 Walkthrough

Walkthrough

Extracted completion-message logic into a new exported helper getDebugCompletionMessages(output?: string): string[] and updated runDebug to print those messages conditionally: when no tarball output is provided it suggests --output; when an output path is present it suppresses that suggestion.

Changes

Cohort / File(s)	Summary
Completion Message Handler src/lib/debug.ts, src/lib/debug.test.ts	Added exported getDebugCompletionMessages(output?: string): string[]. Refactored runDebug to iterate and info(...) each message from the helper. Added tests covering both no-output and with-output cases to assert conditional message presence/absence.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped through code with a twitch and a cheer,
I trimmed the repeat that was showing up near.
Now hints appear only when they matter,
No extra lines making support chatter.
A tidy debug, and the tarball is clear.

🚥 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 summarizes the main change: removing a redundant hint when --output is already provided.
Linked Issues check	✅ Passed	The PR successfully addresses issue #1732 by adding a helper function that suppresses the redundant --output suggestion when output is already specified.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the redundant output hint issue; no unrelated modifications were introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/lib/debug.test.ts (1)

54-67: Consider one integration-level assertion on final CLI output.

These helper tests are solid, but a runDebug({ output: ... }) output assertion would better lock the user-visible behavior and catch future regressions from message sources outside this helper.

🤖 Prompt for AI Agents

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

In `@src/lib/debug.test.ts` around lines 54 - 67, Add an integration-style
assertion that verifies the final CLI output from runDebug matches the
user-visible message from getDebugCompletionMessages; specifically, add a new
test that calls runDebug({ output: "/tmp/nemoclaw-debug.tar.gz" }) (or the
existing run helper) and asserts the stdout/stderr contains "Done. Tarball is
ready to attach to your issue." to ensure message composition outside
getDebugCompletionMessages doesn't regress. Locate the test suite around
getDebugCompletionMessages and add a descriptive it("prints final tarball-ready
message when output provided") that invokes runDebug with an output path and
checks the process output for the exact string returned by
getDebugCompletionMessages("/tmp/nemoclaw-debug.tar.gz").

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@src/lib/debug.ts`:
- Around line 461-464: The function getDebugCompletionMessages currently returns
the "Done. Tarball is ready to attach to your issue." confirmation even when an
`output` path is provided, causing a duplicate attachment confirmation; update
getDebugCompletionMessages(output?: string) so that when `output` is truthy it
does not include the attach confirmation (return an empty array or only
non-attachment messages), and only return the attach confirmation when `output`
is falsy—modify the function body accordingly to prevent the duplicate message.

---

Nitpick comments:
In `@src/lib/debug.test.ts`:
- Around line 54-67: Add an integration-style assertion that verifies the final
CLI output from runDebug matches the user-visible message from
getDebugCompletionMessages; specifically, add a new test that calls runDebug({
output: "/tmp/nemoclaw-debug.tar.gz" }) (or the existing run helper) and asserts
the stdout/stderr contains "Done. Tarball is ready to attach to your issue." to
ensure message composition outside getDebugCompletionMessages doesn't regress.
Locate the test suite around getDebugCompletionMessages and add a descriptive
it("prints final tarball-ready message when output provided") that invokes
runDebug with an output path and checks the process output for the exact string
returned by getDebugCompletionMessages("/tmp/nemoclaw-debug.tar.gz").

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4a78e628-60c4-48c3-b6be-055bc90e65b1

📥 Commits

Reviewing files that changed from the base of the PR and between bb5fdb4 and 19da3f4.

📒 Files selected for processing (2)

• src/lib/debug.test.ts
• src/lib/debug.ts

[deepujain]

Trimmed the extra post-tarball guidance and added docstrings on the touched debug helpers. Build, CLI typecheck, and the focused debug tests all pass locally.