Skip to content

fix(canister): make the --with-icp recovery command paste in every shell - #829

Closed
raymondk wants to merge 1 commit into
mainfrom
cycles-buy/05-paste-flags
Closed

raymondk wants to merge 1 commit into
mainfrom
cycles-buy/05-paste-flags

Conversation

@raymondk

Copy link
Copy Markdown
Collaborator

Cherry-pick of 11a85d4, the first commit of the closed #823, by Josh. The rest of #823 (the deploy-time shortfall hint) is not wanted; this part is, because the next PRs in the stack print pasteable follow-up commands and need the flag builder.

When icp canister create --with-icp stops before the cycles minting canister confirms the creation, it prints the command that finishes it; the ICP is already spent, so that command must paste as printed. Its flags single-quoted every value POSIX-style, which PowerShell reads differently, and a name starting with - broke it in every shell. The flags now come from one builder, commands::args::selection_flags (PasteFlags, shell_word): short flags where there are some, --flag=value for a value starting with -, bare / single-quoted / double-quoted values as each shell icp-cli completes for needs, and a placeholder plus a note for anything no quoting carries.

Unit tests pin every quoted word and the recovery command for -dash, o'neil and a name no quoting carries.


Part of restacking #824 into small PRs, one fix each, stacked on main in this order: cycles-buy/01-json-implies-yes → 02-arg-group → 03-gateway → 04-qr-colours → 05-paste-flags → then the wait, its follow-up text and the --json fields. Each PR targets the one below it; after the one below merges, retarget to main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Placeholder fallback loses the required equals syntax for dash-prefixed names, leaving recovery commands unusable after substitution.

1 open issue
What changed in this PR

Improves cross-shell recovery commands for icp canister create --with-icp and introduces a reusable flag builder for future follow-up commands.

Changes:

  • Adds shared quoting and selection-flag formatting.
  • Adds placeholder notes and regression tests for recovery commands.
  • Documents the behavior in the changelog.
File Description
crates/​icp-cli/​src/​commands/​canister/​create.rs Uses shared flags and adds recovery notes and tests.
crates/​icp-cli/​src/​commands/​args.rs Adds flag building, shell quoting, placeholders, and tests.
CHANGELOG.md Documents cross-shell recovery improvements.

Review effort: Balanced

match word {
Some(word) => flags.text.push_str(&format!(" {word}")),
None => {
flags.text.push_str(&format!(" {short} {blank}"));
@raymondk
raymondk added this pull request to stack #835 October 20, 2026 15:44
@raymondk
raymondk force-pushed the cycles-buy/05-paste-flags branch 2 times, most recently from 58e021f to 68c3333 Compare October 20, 2026 17:45
Base automatically changed from cycles-buy/04-qr-colours to main October 20, 2026 18:48
When `icp canister create --with-icp` stops before the cycles minting
canister confirms the creation, it prints the command that finishes it:
the ICP is already spent, so that command must paste as printed. Its
flags single-quoted every value POSIX-style, which PowerShell reads
differently (`'o'\''neil'`), and a name starting with `-` broke it in
every shell: `--identity '-dash'` reaches the CLI as two flags.

The flags now come from one builder, `commands::args::selection_flags`,
which the shortfall hint will share:

- short flags where there are some (`-n`, `-k`, `-e`), and `--identity`;
- `--flag=value` for a value starting with `-` (`--identity=-dash`);
- a value bare when nothing in it is special to any shell icp-cli
  completes for (the POSIX shells, fish, elvish and PowerShell), else in
  single quotes, else, for a name with `'`, in double quotes when nothing
  else in it is special there (`"o'neil"`);
- anything else (a control character, a curly quote, a backslash, or a
  `'` with `$`, a backtick, `"` or `!`) left as a placeholder such as
  `<your identity>`, and the recovery message then ends with a line
  asking to fill it in.

Unit tests pin every quoted word and flag string, and the recovery
command for `-dash`, `o'neil` and a name no quoting carries. Each
quoted form was run through the released 1.7.0 in zsh, bash 3.2 and 5,
dash, ksh, fish, elvish and PowerShell.

(cherry picked from commit 11a85d4)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Twxau844tMTwyHpQJEbmBo
@raymondk
raymondk force-pushed the cycles-buy/05-paste-flags branch from 68c3333 to 27d76b6 Compare October 20, 2026 18:48
raymondk pushed a commit that referenced this pull request Oct 20, 2026
…how to look in on it

When the wait ended with the order unsettled (Ctrl-C, the hour, the
gateway out of reach) the command said one line ("Order co-1 stays
payable until ...", "Order co-1 is paid, delivering.") and a fixed hint
naming `icp cycles balance` without the flags the run used, then ended
with a short error of its own; --no-wait said the same hint. An order
an operator handles (a refund owed, or one needing review) said only
"Keep the order id".

Now the command says once, as the error it ends with, why the wait
ended (stopped; an hour passed; the gateway stopped answering), whether
the order stays payable until its deadline, is paid and being delivered,
or is in some other state, where its cycles land, and the
`icp cycles balance` command that shows them, on a line of its own with
the identity (the default named, since only the identity that made an
order can read it), network and environment the run used. --no-wait
says the same. A refund owed and an order needing review come with the
`icp canister call <gateway> order_status --query ... '("<id>")'`
command that reads the order back. Paragraphs are set apart by a blank
line, commands are indented and never wrapped, and a value no quoting
carries in every shell is left as <your identity> with a note. -q keeps
the status alone, as before.

The flags come from a builder this commit adds,
commands::args::selection_flags (PasteFlags, shell_word): the short
flag where there is one, `--flag=value` for a value starting with `-`,
which would otherwise read as a flag, and a value bare when nothing in
it is special to any of the POSIX shells, fish, elvish or PowerShell,
else single-quoted, else (for a name with `'`) double-quoted when
nothing else in it is special there; anything else is a placeholder,
`--flag=<placeholder>` when the value starts with `-`. Unit tests pin
every quoted word and every flag form. The builder was the first commit
of #823 and then #829, where it rewrote the recovery command
`icp canister create --with-icp` prints; that path is rare enough to be
left as it is.

The stand-in gateway's tests buy as a named identity, `buyer`, so the
printed flags can be asserted.

Extracted from PR #824.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015aem4tPs4TYVZb8k4LMbJJ
@raymondk

Copy link
Copy Markdown
Collaborator Author

Closing without merging. The --with-icp recovery path is used only by icp canister create, and only on restricted system subnets when the CMC stalls after the ICP transfer; nobody meets it often enough for its quoting to carry a fix of its own. The flag builder is what the stack needs, so it moves into #831, the first PR that prints a pasteable command, with the placeholder fix the review here asked for (--identity=<your identity> for a name starting with -). The recovery command keeps its current quoting.

@raymondk raymondk closed this Oct 20, 2026
@raymondk
raymondk deleted the cycles-buy/05-paste-flags branch October 20, 2026 19:35
@raymondk
raymondk restored the cycles-buy/05-paste-flags branch October 20, 2026 19:35
@raymondk
raymondk removed this pull request from stack #835 October 20, 2026 19:40
@raymondk
raymondk deleted the cycles-buy/05-paste-flags branch October 20, 2026 19:40
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