Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 10:18 AM ET / 14:18 UTC.

Summary
The branch adds candidate-based shell-completion profile resolution for zsh, fish, bash, and PowerShell, routes install/status checks and reload hints through that resolver, and adds profile-path regression tests.

PR surface: Source +22, Tests +236. Total +258 across 5 files.

Reproducibility: yes. from source inspection. Current main still ignores ZDOTDIR/XDG_CONFIG_HOME for the central bug, and the PR-introduced bash hint/default issues are traceable through installCompletion, resolveExistingCompletionProfilePath, and setupWizardShellCompletion.

Review metrics: 1 noteworthy metric.

• Bash profile target default: 1 changed fallback. The diff changes the no-existing-profile bash install target from the shipped .bash_profile fallback to .bashrc, which maintainers should approve or repair before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #63069
Summary: This PR is a candidate fix for the linked shell-completion profile-path bug, with one prior closed unmerged PR attempting the same root cause and adjacent issues covering different shell/profile concerns.

Members:

• canonical: [Bug]: hell completion writes to wrong profile path when ZDOTDIR or XDG_CONFIG_HOME is set #63069 - The PR body uses closing syntax for this issue and implements the same ZDOTDIR/XDG/bash profile-path behavior.
• same_root_cause: fix(cli): respect ZDOTDIR and XDG_CONFIG_HOME in shell completion pro… #63082 - This closed unmerged PR attempted the same shell completion profile-path fix for the linked issue.
• adjacent_distinct: fix: use atomic writes for crash-safety on exec approvals, restart sentinel, shell profile, and saveJsonFile #56994 - This issue mentions shell profile writes, but its root cause is atomic write crash-safety rather than profile path selection.
• adjacent_distinct: [Bug]: XDG Base Directory env vars are not inherited by subprocesses, breaking XDG-aware tools (gogcli, mcporter, others) #84854 - This issue also involves XDG environment handling, but in subprocess environment sanitization rather than shell completion profile resolution.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Resolve the current merge conflict/dirty head and refresh the PR branch.
• [P2] Repair bash fallback and setup hint consistency around .bashrc and .bash_profile.
• [P1] Add current-head real behavior proof for the final resolver and reload-hint behavior; updating the PR body should trigger a fresh ClawSweeper review, or a maintainer can comment @clawsweeper re-review.

Proof guidance:

• [P1] Needs stronger real behavior proof before merge: The PR includes terminal proof, but it is tied to older heads and does not show the final current-head reload-hint behavior; add current-head terminal output or a redacted terminal screenshot/video and redact private paths, secrets, phone numbers, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] GitHub currently reports the PR head as conflicting/dirty, so maintainers need a refreshed branch before trusting the final merge result.
• [P1] The PR changes which bash startup file is created for fresh installs when no bash profile exists, which is compatibility-sensitive for macOS/login-shell users.
• [P1] Current-head real behavior proof is stale: the visible proof does not demonstrate the final reload-hint behavior on 686393fb.

Maintainer options:

1. Repair selected-profile consistency (recommended)
   Update the branch so install/status/setup/doctor use the same selected profile path and preserve or explicitly document the shipped bash no-profile fallback before merge.
2. Accept a new bash default deliberately
   Maintainers may choose the .bashrc fresh-install default, but the PR should make that decision explicit with tests and current-head proof.
3. Replace if the branch stays stale
   If conflicts and proof churn continue, a narrow replacement PR can reuse the useful resolver idea while crediting this source PR.

Next step before merge

• [P1] The next action is contributor or maintainer handling: refresh the conflicting branch, fix the bash profile-selection issues, and supply current-head real behavior proof; automation cannot provide the contributor’s setup proof.

Security
Cleared: The diff changes local shell-profile path selection and tests only; I found no concrete security or supply-chain regression.

Review findings

• [P2] Preserve the bash no-profile fallback — src/cli/completion-runtime.ts:239
• [P2] Keep setup hints on the selected bash profile — src/wizard/setup.completion.ts:31

Rebased on latest upstream/main (ca67a8bdf9). Local node scripts/run-oxlint-shards.mjs reports Found 0 warnings and 0 errors across all three shards (core/extensions/scripts). The previous check-lint failure was a transient interaction with the shard ordering before rebase. CI should now be green on the rebased head.

CI status note (head ca67a8b): the check-lint and checks-fast-contracts-plugins failures are from upstream breakage in code that this PR does not touch:

1. check-lint (oxlint:scripts shard, 2 errors): 'extraSections' will use Object's default stringification format ('[object Object]') when stringified in scripts/generate-plugin-inventory-doc.mjs lines 392/411. This file landed via docs: consolidate plugin install docs #81167 ("docs: consolidate plugin install docs") and is not part of this PR's diff.
2. checks-fast-contracts-plugins-c (1 error): AssertionError: expected [ 'audio-decode' ] to strictly equal [] in src/plugins/contracts/extension-runtime-dependencies.contract.test.ts for extensions/whatsapp. audio-decode was kept in upstream commit 25dd30d6 ("build(whatsapp): keep audio decoder dependency") but the contract test was not updated alongside it.
3. This assertion is unnecessary (1 error in oxlint:core shard): not located in any file under src/cli/. Verified locally with pnpm exec oxlint src/cli/ — clean.

Confirming pre-existing on upstream/main: today's ci.yml runs on main (e.g. a5c1956, 2268ce3, a0f3557) are also failing on these same checks.

This PR only touches src/cli/completion-runtime.ts and a new colocated src/cli/completion-runtime.profile-path.test.ts. Both lint clean locally. CI on this PR is otherwise green at the same head: Real behavior proof, build-smoke, checks-node-core, checks-node-channels, security-fast, check-additional, plus 14+ other shard checks all SUCCESS.

Rebase status (Day 5):

Upstream has landed several related changes to src/cli/completion-runtime.ts since this PR was opened:

• 3c6ec521d5 fix(cli): resolve PowerShell completion profiles
• fe68af5307 fix(cli): show concrete PowerShell completion profile path
• 3d5b0cb41b fix(cli/completion): re-apply shell-profile source-line guard (re-#78659) (#80797)
• 5ff283cfbb fix(cli/completion): guard shell profile source line with file-exists check (#78659)

Current main now exports resolveCompletionProfilePath(shell, options): string — a single-path resolver. This PR introduces resolveShellProfilePathCandidates(shell, env, homeDir): string[] plus async getShellProfilePath() for first-existing lookup, which is a different shape.

To land cleanly I'll need to:

1. Replace resolveCompletionProfilePath with a thin wrapper that picks the first existing candidate from resolveShellProfilePathCandidates, preserving the new PowerShell SHELL/USERPROFILE logic that fix(cli/completion): guard shell profile source line with file-exists check #78659/fix(cli/completion): re-apply shell-profile source-line guard (re-#78659) #80797 added.
2. Re-verify the regression suite still covers $ZDOTDIR/$XDG_CONFIG_HOME/macOS .bash_profile.

Issue #63069 is still OPEN and clawsweeper's last comment confirms current main still resolves with hardcoded HOME-back paths for the affected shells — so the fix is still wanted. I'll push a rebased branch in the next pass once I've worked through the new resolver shape carefully. Marking the bot's P2 thread as defended (no actual regression vs. pre-PR .bashrc-unconditional behavior); details in the inline reply above.

Rebased on latest upstream/main (46030f5489) at head b8f1588c0f.

Resolved shape (was: "I'll need to work through the new resolver shape carefully")

Started from upstream src/cli/completion-runtime.ts rather than replaying the old PR diff, so the recent PowerShell work in #78659/#80797/#3c6ec521d5/#fe68af5307 stays canonical. The PR is now an extension of the upstream resolveCompletionProfilePath API, not a competing abstraction.

Upstream API kept stable — resolveCompletionProfilePath(shell, options): string is unchanged at the callsite level (same sync signature, same { env, homeDir, platform } options object). Existing imports continue to work.

Three new pieces:

• resolveCompletionProfilePathCandidates(shell, options): string[] — exported. Returns candidate paths in priority order.
• CompletionProfilePathOptions — exported type alias for the options object so callers can share the shape.
• resolveExistingCompletionProfilePath(shell, options): Promise<string> — private. Picks the first candidate that already exists, falling back to the canonical install target (candidates[0]) when nothing exists yet. isCompletionInstalled, usesSlowDynamicCompletion, and installCompletion now all route through this so check + install always agree on the same file.

Correctness fixes over the previous PR shape

• zsh + $ZDOTDIR now returns [$ZDOTDIR/.zshrc] only (no $HOME/.zshrc fallback). Falling back to $HOME/.zshrc was a latent bug — zsh ignores $HOME/.zshrc once $ZDOTDIR is set, so the old fallback would have installed into a file the shell never reads.
• fish + $XDG_CONFIG_HOME has the same fix: returns [$XDG_CONFIG_HOME/fish/config.fish] only.
• bash returns [.bashrc, .bash_profile] so isCompletionInstalled finds a .bash_profile-only macOS install, and installCompletion lands in .bash_profile automatically when .bashrc does not exist.
• win32 PowerShell branch is preserved exactly: env.SHELL basename selects WindowsPowerShell vs PowerShell directory under USERPROFILE\Documents, with HOME fallback for USERPROFILE. New tests in completion-runtime.profile-path.test.ts cover both SHELL=powershell.exe and SHELL=pwsh.exe.
• installCompletion drops the per-shell switch and bash .bash_profile try/catch fallback — both are now subsumed by the candidate priority order, so the function is shorter and check + install can never disagree.

Diff shape

• src/cli/completion-runtime.ts: +67 / -45 (net +22). Adds the candidates resolver and existence-aware helper; collapses installCompletion switch.
• src/cli/completion-runtime.profile-path.test.ts: +205 (new file). Uses the upstream options-object pattern for pure resolver tests, adds win32 SHELL basename + USERPROFILE cases, keeps tmpdir-backed isCompletionInstalled regression for $ZDOTDIR / $XDG_CONFIG_HOME / .bash_profile-only.
• Net: 2 files, +272/-45.

Issue #63069 reproducer (macOS / zsh with $ZDOTDIR=$HOME/.config/zsh) is now fixed both at the candidates layer and end-to-end through installCompletion.

@steipete — Day 8 rebase complete; ready for review when convenient.

ClawSweeper PR egg: 🎁 locked until real behavior proof passes.

Pushed afa87b4b5c to fix the 2 check-lint failures from the rebase.

Root cause

oxlint typescript/no-unnecessary-type-assertion flagged:

• src/cli/completion-runtime.ts:213 — resolveCompletionProfilePathCandidates(shell, options)[0] as string;
• src/cli/completion-runtime.ts:232 — candidates[0] as string;

Without noUncheckedIndexedAccess in tsconfig.json, indexing a string[] returns string directly, so the as string assertions don't change the inferred type — exactly what the rule catches.

Fix

Removed both assertions; the function's return type (string) and the surrounding logic (guarantee of a non-empty candidate list — every branch in resolveCompletionProfilePathCandidates returns [someString] or [someString, otherString]) are unchanged.

Verification

• ./node_modules/.bin/oxlint src/cli/completion-runtime.ts src/cli/completion-runtime.test.ts — Found 0 warnings and 0 errors.
• vitest run src/cli/completion-runtime.test.ts — exit 0 (no regressions).
• LSP diagnostics on src/cli/completion-runtime.ts — clean.

Branch is now MERGEABLE at head afa87b4b5c. Should clear the CI rollup on the next run.

Cleaned up the Real behavior proof section to use the exact field: value labels the parser expects: Behavior addressed, Real environment tested, Exact steps or command run after this patch, Evidence after fix, Observed result after fix, What was not tested. The previous body used bolded variants (**Behavior or issue addressed**) which the clawsweeper parser doesn't recognise, so the structured section never registered even though the content was already there.

No code change; PR head and diff are unchanged. The proof content itself is unchanged — only the field labels were normalised so the gate can see them.

@clawsweeper re-review

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26504573763
• Updated: 2026-05-27T10:09:32.399Z

Applied in fef8085437 — addresses the P3 finding (Keep reload hints on the installed profile). Respectfully pushing back on the P2 finding — see below.

P3 — Keep reload hints on the installed profile (FIXED)

Setup wizard and doctor still printed hardcoded ~/.zshrc and ~/.config/fish/config.fish even when ZDOTDIR/XDG_CONFIG_HOME redirected the actual install profile. Affected users were told to source a file the shell never reads.

Fix: Both src/wizard/setup.completion.ts::resolveProfileHint and src/commands/doctor-completion.ts::resolveCompletionReloadPath now route through resolveCompletionProfilePath() (which already honors the env redirects). POSIX shells keep ~-shortening when the resolved path is inside HOME; PowerShell keeps the absolute path because users paste it verbatim into . '...'.

Verification

Module-level smoke test against the patched exports:

$ HOME=/Users/ada ZDOTDIR=/etc/zsh-shared tsx /tmp/test-completion-hint.mjs
zsh w/ ZDOTDIR=/etc/zsh-shared: /etc/zsh-shared/.zshrc
zsh no ZDOTDIR:                 /Users/ada/.zshrc
fish w/ XDG_CONFIG_HOME=/srv/config: /srv/config/fish/config.fish
bash no env:                    /Users/ada/.bashrc

wizard note w/ ZDOTDIR=/etc/zsh-shared:
  Shell completion installed. Restart your shell or run: source /etc/zsh-shared/.zshrc

Added regression test routes zsh reload hint through ZDOTDIR redirect outside HOME (#63069) in src/wizard/setup.completion.test.ts covering exactly the redirect case the bot called out.

P2 — Preserve the bash no-profile fallback (PUSHING BACK)

I think this finding is a false-positive. Tracing the actual behavior against current upstream/main:

Before this PR (src/cli/completion-runtime.ts on main):

if (shell === "bash") {
  return path.join(home, ".bashrc");
}

Fresh-install target: ~/.bashrc. Period. There was no .bash_profile fallback — the prior shipped code always created .bashrc, never .bash_profile.

After this PR (current head fef8085437):

if (shell === "bash") {
  return [path.join(home, ".bashrc"), path.join(home, ".bash_profile")];
}

resolveExistingCompletionProfilePath then returns the first existing file, or falls back to candidates[0] (.bashrc) when neither exists. So:

• Existing .bashrc → reused (same as before).
• Existing .bash_profile only → reused (new behavior, strictly additive: previously the install would write to .bashrc even when the user had .bash_profile).
• Neither exists → write .bashrc (identical to current main).

The PR does not change the fresh-install default; it adds a "use the existing profile if there is one" preference layer on top. The bot's concern ("changes fresh bash installs from .bash_profile to .bashrc") inverts the actual direction — current main already creates .bashrc, and this PR keeps that exact same fresh-install target. The proof is git show upstream/main:src/cli/completion-runtime.ts | grep -A2 'shell === "bash"'.

If maintainers want fresh installs to prefer .bash_profile instead, that would be a separate behavior change (changing what shipped behavior does for fresh installs), not a regression introduced by this PR.

Branch diff vs upstream/main

 src/commands/doctor-completion.ts   |  9 +++++----
 src/wizard/setup.completion.test.ts | 31 +++++++++++++++++++++++++++++++
 src/wizard/setup.completion.ts      | 28 ++++++++++++++--------------
 3 files changed, 50 insertions(+), 18 deletions(-)

Plus the existing PR changes to src/cli/completion-runtime.ts + its test. Total touched surface stays inside the broader-fix-preferred boundary (one module, one symptom family).

Ready for re-review on fef8085437. cc @steipete

Heads up: this PR needs to be updated against current main before the new required Dependency Guard check can pass.

Thanks @RomneyDa — confirmed. The Dependency Guard required check is now green on the current head:

• dependency-guard — SUCCESS
• dependency-guard-detect — SUCCESS
• dependency-guard-autoscrub — SKIPPED (nothing to scrub)

Overall the head is MERGEABLE/CLEAN with 133 SUCCESS and no failures (the 2 CANCELLED are superseded earlier runs). Ready for review when convenient.

Rebased onto current main per the Dependency Guard note — a clean three-commit fix on the latest baseline covering completion-path handling for ZDOTDIR / XDG_CONFIG_HOME across src/cli/completion-runtime.ts, src/wizard/setup.completion.ts, and src/commands/doctor-completion.ts (plus tests). Dependency Guard and the rest of CI now run against the fresh base.

Note: the linked issue #63069 was auto-closed by the stale bot for inactivity (not_planned), not because it was resolved — the underlying wrong-profile-path behavior still reproduces, so merging this will close it via the Closes #63069 link. Thanks @RomneyDa for the heads-up.

Closing this one: the linked issue #63069 was auto-closed for inactivity and the branch has since drifted into conflict with current main. The fix (honor ZDOTDIR / XDG_CONFIG_HOME when installing shell completion) still looks valid — happy to rebase and reopen if #63069 is revived with a fresh reproducer.