Skip to content

fix: substitute known NextAction params in human output#41

Merged
jpage-godaddy merged 3 commits into
mainfrom
fix-nextactions-subs
Jul 8, 2026
Merged

fix: substitute known NextAction params in human output#41
jpage-godaddy merged 3 commits into
mainfrom
fix-nextactions-subs

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • append_next_actions echoed a NextAction's command template verbatim in human output, so a <placeholder> never got replaced even when the caller had already set a concrete value via NextActionParam::value(...).
  • Adds substitute_known_params: fills <key> in the command string with params[key].value when present, leaving it as literal placeholder text when the param is only a required()/enum hint with no known value. Returns Cow<str> to avoid allocating when nothing has a known value.
  • Aligned envelope.rs's doc comment and two test fixtures with the <key> angle-bracket placeholder convention actually used everywhere (they showed a stale {{key}} mustache-style example predating this feature).

Test plan

  • cargo test — new cases human_output_substitutes_known_next_action_params and human_output_leaves_placeholder_without_a_known_value, plus existing human_output_appends_next_steps_footer (no params → no substitution) still pass
  • cargo clippy -- -D warnings, cargo fmt --check
  • Verified end-to-end against the godaddy-cli binary (gddy domain quote) via a temporary [patch.crates-io] path override — the real quote token now substitutes into the printed "Next steps" line instead of a literal <token>

Human output's "Next steps" footer echoed a NextAction's command template
verbatim, so any concrete value set via NextActionParam::value (e.g. a
freshly issued quote token) never replaced its <placeholder>, leaving
users to copy-paste a literal <token> that doesn't exist.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Improves cli-engine human output rendering for NextAction follow-ups by substituting any known NextActionParam::value(...) entries into <placeholder> command templates, while leaving required-only hints as literal placeholders for the user.

Changes:

  • Human output “Next steps” now substitutes <key> placeholders with params[key].value when present.
  • Added unit tests covering substitution vs. leaving placeholders untouched when no concrete value exists.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/output/human.rs
Comment thread src/output/human.rs Outdated
- Update the append_next_actions doc comment, which still described the
  pre-substitution behavior of showing placeholders verbatim.
- Return Cow<str> from substitute_known_params instead of always allocating
  a new String, skipping the allocation when no param has a known value.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread src/output/human.rs
The struct doc comment and two envelope tests still used a {{key}} mustache-
style example for NextAction::command, but the only real placeholder
convention in use — the human-output substitution added in this branch, and
every NextAction call site in the downstream CLI — is <key> (angle
brackets). Update the doc comment and test fixtures to match so they don't
mislead a future caller into using a syntax substitution never recognizes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@jpage-godaddy jpage-godaddy merged commit 86b4bd7 into main Jul 8, 2026
4 of 5 checks passed
@jpage-godaddy jpage-godaddy deleted the fix-nextactions-subs branch July 8, 2026 15:51
jpage-godaddy pushed a commit that referenced this pull request Jul 8, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.4.1](cli-engine-v0.4.0...cli-engine-v0.4.1)
(2026-07-08)


### Bug Fixes

* substitute known NextAction params in human output
([#41](#41))
([86b4bd7](86b4bd7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants