fix(parse): keep a re-declared global's aliases on one flag

[jdx]

What

merge_subcommand_flags in lib/src/parse.rs could leave one logical flag registered under two different Arc<SpecFlag>s in available_flags. The collision guard now compares flag origins instead of Arc identity.

Why

Reproduction: a global declared with both a short and a long (flag "-y --yes" global=#true), re-declared non-globally by a subcommand (flag "-y --yes").

The child's flag keys iterate in BTreeMap order, so --yes comes before -y:

1. Key --yes: inherited_global resolves to the original global A. available["--yes"] is also A, so Arc::ptr_eq holds and the skip guard doesn't fire. A merged flag M is built and inserted at --yes.
2. Key -y: inherited_global now resolves via available["--yes"], which is M. The guard checks available["-y"] — still A — and A.global && !Arc::ptr_eq(A, M) is true, so it continues. The -y key keeps pointing at the pre-merge A.

The guard's stated intent is "never clobber a different inherited global's alias", but M isn't a different global — it was derived from A in this same loop, so the identity comparison misfires.

Impact is mild but real. Parsing itself still works because lookups are by key, so this only shows up in code that reads the flag objects rather than the keys. When the re-declaration adds a third alias (global -y --yes, child -y --yes --assume-yes), the flag reachable via -y is missing assume-yes from its long list.

How

Track which inherited global each merged flag was derived from (merged_origin), and resolve a flag to its origin — itself, or for a merged flag its source global — before comparing. The guard now skips only when the existing key's origin differs from the incoming global's origin, so a flag this loop just merged is no longer mistaken for a colliding global.

Chained merges are handled too: a new merged flag inherits global_origin rather than pointing at its immediate base, so if the global has further aliases still to be processed they all resolve back to the same origin.

Reviewer notes

• New test test_redeclared_global_aliases_share_one_flag covers the three-alias case and asserts all three keys resolve to a flag whose long contains every alias, that unique_flags sees exactly one object, and that the global's effect="write" survives the merge. Confirmed it fails on the pre-fix code with exactly the reported symptom (-y → long: ["yes"]) and passes after.
• The genuine-collision behavior is unchanged and still covered by the existing test_orphan_short_does_not_clobber_unrelated_global: an unrelated global (-r --restrict) keeps its alias, as global precedence dictates.
• 296 usage-lib tests pass; cargo clippy --all --all-features -- -D warnings and cargo fmt --all --check are clean. Two failures in usage-cli --test examples (test_usage_double_slash_execution{,_old}) are pre-existing and environmental — they fail identically on clean main.
• Follow-up, not in this PR: usage::available_flags on feat/usage-mcp (feat(mcp): serve a usage spec to an agent over stdio #746) works around this by collapsing duplicates by name after the pointer dedup. That branch isn't an ancestor of main, so I left it alone; its workaround can be simplified when it next rebases onto this fix.

This PR was generated by Claude Code.

---

Note

Medium Risk
Touches subcommand descent flag-merging in the parser (available_flags), which affects completions and metadata on flag objects; behavior is narrow and covered by new tests, with existing collision tests unchanged.

Overview
Fixes merge_subcommand_flags so a subcommand that re-declares an inherited global no longer leaves different alias keys (-y, --yes, etc.) pointing at different Arc<SpecFlag> objects with inconsistent merged metadata.

The collision guard now compares flag origins (via merged_origin / origin_of) instead of Arc pointer identity, so a merged flag from an earlier loop iteration is not treated as a competing global. On the first merge for a child re-declaration, all existing map entries for that global’s aliases are rebound to the single merged flag—covering cases where the child only re-declares a subset of the global’s aliases.

Adds regression tests for full and partial re-declaration; unrelated global short collisions are unchanged.

^{Reviewed by Cursor Bugbot for commit ae7be7f. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes
  • Fixed alias handling for inherited global options when subcommands redefine those options.
  • Ensured all aliases consistently resolve to one option and preserve the complete alias set.
  • Corrected behavior when a subcommand adds a new alias or omits an existing alias during redefinition.

[coderabbitai]

📝 Walkthrough

Walkthrough

Inherited-global flag merging now tracks logical origins, avoids incorrect redeclaration skips, and rebinds all related aliases to one merged flag. Regression tests cover full and partial alias redeclarations.

Changes

Inherited flag merging

Layer / File(s)	Summary
Origin-aware alias merging lib/src/parse.rs	merge_subcommand_flags tracks inherited-global origins, compares origins during redeclaration checks, rebinds matching aliases to the merged flag, and tests merged alias identity, lists, global status, and effects.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit with aliases bright,
Merging each flag just right.
One shared arc, paths aligned,
Old and new names neatly twined.
Tests hop happily through the night.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: keeping a re-declared global flag's aliases bound to one flag.

---

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.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 90e248d. Configure here.}

[cursor]

Baseline tak notes not fetched

Medium Severity

The PR perf job runs tak compare against the merge-base SHA but never fetches refs/notes/tak, where perf.yml stores main’s instruction-count history. After perf:record only the head commit has a local note, so the baseline measurement for the base commit is usually missing and compare cannot gate regressions reliably.

Additional Locations (1)

• .github/workflows/perf-pr.yml#L79-L90

 

^{Reviewed by Cursor Bugbot for commit 90e248d. Configure here.}

[greptile-apps]

Greptile Summary

This PR consolidates re-declared global flag aliases onto one shared flag object.

• Tracks the inherited global origin of flags merged during subcommand descent.
• Rebinds omitted inherited aliases to the merged flag.
• Adds regression coverage for full and partial alias redeclarations.

Confidence Score: 3/5

The parser fix appears sound, but the PR is not safe to merge while manual benchmark runs remain inert and pull-request comparisons can lack their recorded baseline.

The benchmark workflow still skips its only job for workflow_dispatch, and its comparison path fetches the base branch without fetching the refs/notes/tak data consumed by the baseline comparison.

Files Needing Attention: .github/workflows/perf-pr.yml

Important Files Changed

Filename	Overview
lib/src/parse.rs	Updates subcommand flag merging to compare global origins and keep every alias mapped to the same merged Arc, with focused regression tests.

_{Reviews (2): Last reviewed commit: "fix(parse): rebind a merged global's unt..." | Re-trigger Greptile}

[greptile-apps]

Baseline notes are never fetched

When a base measurement exists only in the remote refs/notes/tak history, this workflow fetches the commit graph but not the notes ref before calling tak compare, causing the regression gate to fail or report that nothing was compared instead of evaluating the pull request.

[greptile-apps]

Manual runs skip comparison job

When a maintainer manually dispatches this workflow, the sole job rejects every event other than pull_request, causing the run to finish without producing a measurement or comparison report.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/perf-pr.yml:
- Around line 75-78: Update the “Measure this pull request” workflow step to
fetch the notes baseline before recording. Add a quiet shallow fetch of origin’s
refs/notes/tak into the local refs/notes/tak immediately before mise run
perf:record, preserving the existing recording command and no-push behavior.

In @.github/workflows/perf.yml:
- Around line 34-38: Pin the performance workflow runner from ubuntu-latest to
an explicit versioned label such as ubuntu-24.04 in both
.github/workflows/perf.yml (lines 34-38) and .github/workflows/perf-pr.yml
(lines 36-40), preserving the existing timeout and performance-series
configuration.

In `@lib/src/parse.rs`:
- Line 124: Update the alias-merging logic around merged_origin so every
inherited alias for the merged flag is rebound to the merged Arc, including
aliases omitted by the child declaration; do not redirect only the currently
iterated child key. Add a regression test covering a parent flag with multiple
aliases where the child redeclares only one alias, and verify all aliases
preserve shared identity.

🪄 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: a881488e-ef1e-4c11-abd1-973520c49418

📥 Commits

Reviewing files that changed from the base of the PR and between accc8a4 and 90e248d.

⛔ Files ignored due to path filters (1)

• mise.lock is excluded by !**/*.lock

📒 Files selected for processing (5)

• .github/workflows/perf-pr.yml
• .github/workflows/perf.yml
• lib/src/parse.rs
• mise.toml
• tak.toml

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-remote --exit-code origin refs/notes/tak
rg -n 'refs/notes/tak|perf:record|tak compare' .github/workflows/perf-pr.yml

Repository: jdx/usage

Length of output: 377

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== perf-pr workflow relevant sections =="
sed -n '1,120p' .github/workflows/perf-pr.yml

echo
echo "== perf record implementation =="
fd -a '.*' . | sed 's#^\./##' | grep -Eh '(^mise\.toml$|perf|tak|measure|benchmark)' | head -50

echo
echo "== tak/measure references =="
rg -n 'tak |perf:record|take|compare|record|measure|baseline|notes/tak|GIT_NOTES|git notes' -S . --glob '!**/.git/**' --glob '!**/target/**' --glob '!**/node_modules/**'

Repository: jdx/usage

Length of output: 39138

---

🌐 Web query:

tak-cli git notes refs/notes/tak compare documentation

💡 Result:

In the context of the tak-cli (a Rust-based benchmarking tool), refs/notes/tak is the specific Git notes reference used for storing benchmark performance results [1][2]. The tool stores benchmark data in this ref as JSON objects, one per line, and utilizes Git's cat_sort_uniq merge strategy to manage concurrent writes from CI environments without conflict [1][2]. Because the notes tree is keyed by commit SHA as path names, it enables efficient data retrieval (e.g., shallow fetches) without requiring a full repository clone [1][2]. The tool includes functionality to manage this reference automatically: - Automated Fetching and Merging: The tool is designed to fetch remote data into the local refs/notes/tak reference [3]. It uses merge logic rather than overwriting to ensure local, unpushed measurements are not discarded when syncing with remote data [3][4]. - Refspec Configuration: To make the notes visible to users who do not run the tak binary directly, the tool can modify the Git configuration to include a specific fetch refspec (+refs/notes/tak:refs/notes/tak) for the remote [3]. This ensures that standard git fetch operations automatically retrieve these performance notes [3]. It is important to note that tak-cli is distinct from other tools sharing the "tak" name, such as the Microsoft Touch Adaptation Kit (used for game streaming layouts) [5][6] or the node-tak library (used for TAK/CoT data management) [7]. Documentation for tak-cli is primarily available via its crate page on crates.io and its associated documentation on docs.rs [2][8]. As of the current version, the documentation indicates that while the tool supports recording, pushing, and viewing history via this ref, the tak compare command (for interleaved A/B comparisons) is listed as a feature that has not yet been fully implemented [1][2].

Citations:

• 1: https://docs.rs/crate/tak-cli/latest/source/README.md
• 2: https://crates.io/crates/tak-cli
• 3: https://docs.rs/crate/tak-cli/latest/source/src/notes.rs
• 4: https://docs.rs/crate/tak-cli/latest/source/tests/notes_race.rs
• 5: https://learn.microsoft.com/en-us/gaming/gdk/docs/features/common/game-streaming/tak-command-line-tool/game-streaming-tak-command-line?view=gdk-2604
• 6: https://github.com/microsoft/xbox-game-streaming-tools/blob/main/README.md
• 7: http://node-tak.cloudtak.io/index.html
• 8: https://docs.rs/tak-cli/latest/tak_cli/

---

🌐 Web query:

GitHub jdx tak-cli README refs/notes/tak notes compare

💡 Result:

The project tak-cli (binary tak) is an experimental CLI benchmarking tool by jdx that uses Git notes for data storage [1][2]. Its README highlights refs/notes/tak as the mechanism for persisting benchmark results directly within the Git repository, avoiding the need for external databases or services [1][3]. Key details regarding the implementation include: Storage Mechanism: It stores benchmark results as Git notes under the refs/notes/tak reference [1][3]. Each note is a JSON object per line, and the system uses the cat_sort_uniq merge strategy [1][4]. This strategy allows concurrent CI writers to update benchmarks without creating conflicting history, as it merges incoming data rather than overwriting existing notes [1][4]. Data Integrity and Race Conditions: The tool specifically addresses race conditions that can occur in CI environments. In earlier iterations, tak would inadvertently overwrite remote records if a local checkout had not fetched the latest notes before pushing [5]. To prevent this, tak now employs an absorb_remote mechanism that merges the remote state into the local record using Git notes merging rather than performing a forced overwrite [4][5]. Refspec Usage: The tool configures Git refspecs to manage how data is fetched and pushed [4]. Specifically, it uses +refs/notes/tak:refs/notes/tak to ensure that updates are synchronized correctly and that local records are not lost during the fetch process [4]. Status: As of July 2026, the project is highly experimental. The author explicitly cautions against using it for production or as a dependency, describing it as an experiment intended to validate an idea rather than a stable, packaged tool [2][3].

Citations:

• 1: https://github.com/jdx/tak
• 2: https://crates.io/crates/tak-cli
• 3: https://docs.rs/tak-cli/latest/tak_cli/
• 4: https://docs.rs/crate/tak-cli/latest/source/src/notes.rs
• 5: https://docs.rs/crate/tak-cli/latest/source/tests/notes_race.rs

---

Fetch the notes baseline before recording HEAD.

.github/workflows/perf-pr.yml never fetches refs/notes/tak, and tak compare "$BASE_SHA" needs the base commit’s measurements from that notes ref before the base commit can be compared. Add git fetch --quiet --depth=1 origin '+refs/notes/tak:refs/notes/tak' before mise run perf:record.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/perf-pr.yml around lines 75 - 78, Update the “Measure this
pull request” workflow step to fetch the notes baseline before recording. Add a
quiet shallow fetch of origin’s refs/notes/tak into the local refs/notes/tak
immediately before mise run perf:record, preserving the existing recording
command and no-push behavior.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n 'runs-on: ubuntu-' .github/workflows/perf.yml .github/workflows/perf-pr.yml

Repository: jdx/usage

Length of output: 264

---

🌐 Web query:

GitHub Actions ubuntu-latest runner label versioned OS image ubuntu-24.04

💡 Result:

As of July 27, 2026, the ubuntu-latest runner label in GitHub Actions points to the Ubuntu 24.04 image [1][2]. The transition of the ubuntu-latest label to Ubuntu 24.04 was completed in late 2024, following a migration period that began in September 2024 [3]. Users who require a specific OS environment—such as those who may face compatibility issues with newer package versions or system changes in Ubuntu 24.04—can explicitly target a versioned runner label in their workflow file (e.g., runs-on: ubuntu-22.04) to avoid unexpected migrations when the label is updated in the future [2][4]. GitHub continues to provide and maintain versioned labels for GitHub-hosted runners, allowing workflows to remain stable by pinning them to specific OS releases [1][2]. Currently, both ubuntu-22.04 and ubuntu-24.04 are supported [1], and a public preview of ubuntu-26.04 is also available [1][5].

Citations:

• 1: https://docs.github.com/en/actions/reference/runners/github-hosted-runners
• 2: https://github.com/actions/runner-images
• 3: https://github.blog/changelog/2024-09-25-actions-new-images-and-ubuntu-latest-changes/
• 4: Ubuntu-latest workflows will use Ubuntu-24.04 image actions/runner-images#10636
• 5: https://github.com/actions/runner-images/releases/tag/ubuntu24/20260720.247

---

Pin the runner OS used by the performance series.

ubuntu-latest remains a mutable GitHub-hosted runner label, so future image changes can compare PR measurements against history collected on a different OS environment and invalidate the 1% gate. Use an explicit versioned label, such as ubuntu-24.04, in both perf.yml and perf-pr.yml.

📍 Affects 2 files

• .github/workflows/perf.yml#L34-L38 (this comment)
• .github/workflows/perf-pr.yml#L36-L40

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/perf.yml around lines 34 - 38, Pin the performance
workflow runner from ubuntu-latest to an explicit versioned label such as
ubuntu-24.04 in both .github/workflows/perf.yml (lines 34-38) and
.github/workflows/perf-pr.yml (lines 36-40), preserving the existing timeout and
performance-series configuration.

[jdx]

Rebased onto main and addressed the review feedback.

CodeRabbit's lib/src/parse.rs finding — valid, fixed in ae7be7f

Update the alias-merging logic around merged_origin so every inherited alias for the merged flag is rebound to the merged Arc, including aliases omitted by the child declaration; do not redirect only the currently iterated child key.

Correct, and it's the same defect this PR set out to fix reached by a different route. The merge loop only visits the alias keys the child declared, so any alias of the global the child left out kept pointing at the pre-merge flag. A -y --yes --confirm global re-declared as just --yes --assume-yes left -y and --confirm on a flag whose long was missing assume-yes.

Fixed by rebinding every key that shares the merged flag's origin global when the merged flag is first built (once per re-declared child, not per alias key). Added test_partially_redeclared_global_keeps_all_aliases_on_one_flag covering exactly the requested case — a parent flag with multiple aliases where the child re-declares only one — asserting both shared identity (unique_flags(...).count() == 1) and that every alias key carries the full long list. Verified it fails before the fix with -y → long: ["yes", "confirm"] and passes after.

The genuine-collision behavior is unchanged: the rebind only touches keys whose value already resolves to the same origin global, so an unrelated global keeps its alias, as test_orphan_short_does_not_clobber_unrelated_global still verifies.

The .github/workflows/perf*.yml comments — no longer part of this PR

The notes-baseline finding (Cursor Bugbot, Greptile P1, CodeRabbit) and the runner-pinning and workflow_dispatch comments were all against perf-pr.yml / perf.yml / tak.toml / mise.toml. Those files were only in this PR's diff because the branch carried an unmerged copy of the perf commit; that work landed on main separately via #748. The rebase dropped the duplicate, so this PR is now a single-file change to lib/src/parse.rs and none of that feedback applies here. Worth re-filing against main if the notes-fetch gap is still real there — it looked like a legitimate point, just not one this PR can fix.

Verification

297 usage-lib tests pass; cargo clippy --all --all-features -- -D warnings and cargo fmt --all --check clean. The two usage-cli --test examples failures (test_usage_double_slash_execution{,_old}) are pre-existing and environmental in my sandbox — they fail identically on unmodified main.

This comment was generated by Claude Code.

[github-actions]

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
markdown	▁▁██	102,869,108 → 102,907,538	+0.04%	17.06 → 17.09ms	+0.18%
startup	▄▁█▄	1,160,310 → 1,158,269	-0.18%	1.18 → 1.15ms	-2.31%

No instruction-count regression above 1%.

_{Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.}

_{Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.}

_{ae7be7f1c870 vs accc8a4e3eda · measured on this runner, not pushed to the history.}