fix: recompute command path lengths when a subtree is reparented

[r0h1tb]

Fixes #2463

Problem

AddCommand caches len(x.CommandPath()) on the parent at the moment a subcommand is attached. CommandPath() is derived by walking the parent chain, so attaching a subtree somewhere new lengthens the path of every command beneath it — invalidating anything cached earlier.

Two identical trees therefore render different help text depending only on construction order:

// bottom-up: sub has no parent yet, so this caches "remote authentication" (21)
sub.AddCommand(t1); sub.AddCommand(t2); root.AddCommand(sub)

// top-down: caches "app remote authentication" (25)
root.AddCommand(sub); sub.AddCommand(t1); sub.AddCommand(t2)

CommandPathPadding() reads that cached value, so the "Additional help topics" column comes out misaligned in the bottom-up case.

Fix

AddCommand now refreshes cached path lengths for the attached command and all of its descendants, via a small recomputeCommandPathLengths helper.

Two notes on scope:

• Only commandsMaxCommandPathLen is recomputed. commandsMaxUseLen and commandsMaxNameLen derive from Use and Name, which reparenting doesn't affect.
• The recursion is needed rather than a single level: a three-deep tree attached at the root has stale caches at every intermediate node, which the second test covers.

RemoveCommand already performs an equivalent recompute for its own children, so this brings AddCommand in line with it.

Tests

TestCommandPathPaddingIndependentOfAddCommandOrder builds the issue's exact tree both ways and asserts the cached length matches, that it equals len("app remote authentication"), and that the rendered help output is byte-identical.

TestCommandPathPaddingRecomputedForDeepSubtree covers the multi-level case.

On main both fail:

max command path length = 21, want 25 (must not depend on AddCommand order)
nested max command path length = 11, want 19

With this change go test ./... is green across cobra and cobra/doc. gofmt and go vet are clean.

[CLAassistant]

All committers have signed the CLA.

[eugene-panin]

Thanks for the recursive refresh — I found the matching invalidation when a subtree is detached.

main passes this sequence, but this PR makes it fail because AddCommand refreshes the subtree with the longer path and RemoveCommand only clears parent:

remote.AddCommand(authentication, tls)
root.AddCommand(remote)
root.RemoveCommand(remote)

// stale with this PR: "app remote authentication" length
// expected: len("remote authentication")
if got, want := tls.CommandPathPadding(), len(authentication.CommandPath()); got != want {
    t.Fatalf("padding = %d, want %d", got, want)
}

The minimal follow-up appears to be refreshing the detached subtree immediately after clearing its parent:

command.parent = nil
command.recomputeCommandPathLengths()

I verified this in Docker against the PR branch: the reproduction fails before that line and passes after it, along with go test ./..., go test -race ./..., go vet ./..., and formatting. A regression test for the detach path would make the cache invalidation symmetric with the attach path.

[r0h1tb]

Good catch — thanks @eugene-panin, this reproduces exactly as you describe and the suggested line is the right fix. Pushed in 4a5f267.

Behavioural delta: RemoveCommand now refreshes the detached subtree's cached path lengths, so CommandPathPadding() reflects the subtree's own root immediately after detach instead of the path it had while attached.

One correction to the framing

The detach staleness isn't only a regression from this PR — it's already present on main when the tree is built top-down. main gets your bottom-up sequence right by accident, because without the attach-side refresh the cache was never updated to the longer value in the first place. Swap the two AddCommand calls and main fails the same assertion:

root.AddCommand(remote)
remote.AddCommand(authentication, tls)
root.RemoveCommand(remote)

// main today: 25 (len("app remote authentication"))
// correct:    21 (len("remote authentication"))

I verified that against an unmodified command.go:

$ git checkout main -- command.go && go test -run TestCommandPathPaddingRecomputedOnRemoveCommand .
--- FAIL: .../top-down
    max command path length after detach = 25, want 21
    padding = 25, want 21

So this isn't papering over something the PR broke — AddCommand refreshing the subtree is what makes the missing detach-side invalidation observable in both construction orders, and the recompute makes the two paths symmetric. Worth stating plainly in case it affects how you'd want it reviewed.

Tests

TestCommandPathPaddingRecomputedOnRemoveCommand runs your sequence in both orders as a table test; TestCommandPathPaddingRecomputedOnRemoveCommandForDeepSubtree covers the three-level case, since the detached subtree has stale caches at every depth just as the attached one does.

Copy-pasteable proof the tests pin the bug rather than restate the fix — revert the source hunk only and keep the tests:

$ git checkout HEAD~1 -- command.go
$ go test -run TestCommandPathPaddingRecomputedOnRemoveCommand .
--- FAIL: TestCommandPathPaddingRecomputedOnRemoveCommand/top-down
    max command path length after detach = 25, want 21
    padding = 25, want 21
--- FAIL: TestCommandPathPaddingRecomputedOnRemoveCommand/bottom-up
    max command path length after detach = 25, want 21
    padding = 25, want 21
--- FAIL: TestCommandPathPaddingRecomputedOnRemoveCommandForDeepSubtree
    nested max command path length after detach = 19, want 15
FAIL

go test ./..., go test -race ./..., go vet ./... and gofmt -l . are all clean with the change.

Deliberately left out

ResetCommands has the same class of staleness and I did not touch it, to keep this PR to the reported issue. It clears parent and commands but leaves all three max-length caches populated, and AddCommand only ever grows them:

root.AddCommand(&Command{Use: "authentication"})
root.ResetCommands()
root.AddCommand(&Command{Use: "ab"})
// maxPathLen=18 maxUseLen=14 maxNameLen=14; correct would be 6/2/2

That reproduces on main independently of this PR and also affects commandsMaxUseLen/commandsMaxNameLen, which the path-length recompute deliberately doesn't cover. Happy to fix it here if a maintainer prefers it bundled, or to file it separately — whichever is easier to review.