fix(cli): stabilize delegated credential shard lookup

[khaliqgant]

Summary

• Canonicalize delegated credential shard keys through a shared workspace shard helper.
• Probe all catalog aliases (id, relayWorkspaceId, name) across every matching record when the canonical shard is missing, then heal with a create-if-missing write.
• Add polluted-catalog regression coverage for the inverted id/rw row, missing cross-link twin, benign duplicates, scope guard, and no-clobber heal helper.

Verification

• go test ./internal/delegatedauth
• focused go test ./cmd/relayfile-cli -run ... regression set
• go test ./cmd/relayfile-cli
• go test ./...

Non-blocking follow-ups

• Deduplicate the catalog-pollution warning in loadDelegatedCredentialsForRequest once per process to reduce log noise on permanently polluted catalogs.
• Track the upstream source that wrote the inverted id=rw_/rw=uuid workspace catalog row so new poisoned records stop accumulating.

Merge hold: review-ready for #321, not auto-merge.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 20fff95d-fbed-49ef-8016-09df8637898f

📥 Commits

Reviewing files that changed from the base of the PR and between 8644851 and 717179d.

📒 Files selected for processing (4)

• cmd/relayfile-cli/main.go
• cmd/relayfile-cli/main_test.go
• internal/delegatedauth/delegatedauth.go
• internal/delegatedauth/delegatedauth_test.go

---

📝 Walkthrough

Walkthrough

Delegated-credential on-disk storage is migrated from a single flat key to a workspace-shard-keyed directory layout (~/.relayfile/delegated/<workspace-shard>/<scope-shard>.json). Workspace alias canonicalization is added to derive consistent shard keys. Load logic gains legacy-path probing and atomic migration via a new SaveAtomicIfMissing primitive in internal/delegatedauth.

Changes

Workspace-sharded delegated credential storage and migration

Layer / File(s)	Summary
SaveAtomicIfMissing: atomic create-only persistence internal/delegatedauth/delegatedauth.go, internal/delegatedauth/delegatedauth_test.go	SaveAtomic is refactored to delegate to an internal saveAtomic(path, bundle, createOnly) helper. New exported SaveAtomicIfMissing skips writing when the target already exists; when it writes, it uses os.Link instead of os.Rename so a concurrent writer wins silently. Tests assert creation returns created=true with 0600 permissions, and a pre-existing file returns created=false with original tokens preserved.
Workspace shard key derivation and path helpers cmd/relayfile-cli/main.go (lines 6995–7135)	Replaces the old delegatedCredentialsWorkspaceKey with workspaceShardKey/rawWorkspaceShardKey and adds canonicalWorkspaceShardValue*/workspaceRecordForShardValue helpers. Introduces delegatedCredentialsRawPathForRequest for legacy probing. resolveDelegatedCredentialsPathForRequest now routes to the scoped path only when workspace or scopes are non-empty.
Load with legacy-path probing and migration cmd/relayfile-cli/main.go (lines 7152–7234)	loadDelegatedCredentialsForRequest resolves the canonical workspace shard (with a stderr warning when resolution is ambiguous), loads from the canonical path, and—on missing-credentials failures for non-explicit callers—probes legacy shard paths enumerated by legacyDelegatedCredentialsPathsForRequest. When a legacy bundle satisfies requested scopes it is migrated atomically via SaveAtomicIfMissing; if a race is detected the canonical path is reloaded.
CLI path canonicalization and migration tests cmd/relayfile-cli/main_test.go (lines 2812–2935)	Three tests cover: (1) multiple workspace alias inputs resolving to the same canonical shard path, (2) ambiguous relay-workspace alias shard values being preserved raw without canonicalization, and (3) end-to-end migration from a legacy minted shard path to the canonical resolved path with catalog-ordering determinism.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• Delegated-cred shard key diverges: relayfile login mints under sha256(workspace UUID), relayfile start/mount reads under sha256(rw_ handle) → mount can never find its cred #321: This PR directly implements the fix for the delegated-credential shard key divergence described in that issue — canonicalization of workspace identifiers (including rw_ handles) to record.ID before shard-key derivation, plus legacy-path migration that heals the mount-unable-to-find-credentials regression.

Possibly related PRs

• AgentWorkforce/relayfile#274: This PR's SaveAtomic refactor and new SaveAtomicIfMissing in internal/delegatedauth directly extend the delegated-credential refresh/persistence work introduced there.
• AgentWorkforce/relayfile#294: Both PRs modify delegatedCredentialsPathForRequest and loadDelegatedCredentialsForRequest in cmd/relayfile-cli/main.go for workspace-scoped storage and legacy migration fallback.

Poem

🐇 Hop hop, the shards are aligned,
Old paths once lost, now easy to find.
A link beats a rename when racers collide,
The canonical shard keeps credentials inside.
No alias left raw when one truth can be known—
The warren of creds finally feels like a home! 🏠

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: stabilizing delegated credential shard lookup through canonicalization and robust probing logic.
Description check	✅ Passed	The description is directly related to the changeset, covering the canonicalization strategy, probing mechanism, test coverage, and verification steps performed.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/delegated-cred-workspace-shard-key

---

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 and usage tips.}

[github-actions]

Relayfile Eval Review

Run: .relayfile/evals/runs/2026-06-19T06-40-13-844Z-HEAD-provider
Mode: provider
Git SHA: beeffbd

Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0

Human Review Cases

No reviewable human-review cases captured Relayfile output.

[khaliqgant]

Follow-up issue for the non-blocking upstream item (the source that writes the inverted id=rw_/relayWorkspaceId=UUID catalog row): #323. Root cause pinned there — upsertWorkspaceDetails defaulting record.ID = record.Name (main.go:7771-7773) when a workspace is keyed by its rw_ handle, then persistDelegatedWorkspace (1823-1830) putting the canonical UUID into RelayWorkspaceID. This PR's defensive fix makes resolution robust despite those rows; #323 stops them being written. — cloud-expert

[agent-relay-code]

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

This PR has no HTTP API surface changes, so the OpenAPI contract rule doesn't apply. No bot/reviewer comments were provided in the harness inputs (.workforce/ contains only diff, changed-files, and context). I made no file edits — the PR is mechanically clean (gofmt/vet pass) and I leave semantic logic untouched per policy.

Here is my review.

---

Review: PR #322 — fix(cli): stabilize delegated credential shard lookup

Summary

The PR canonicalizes the workspace value before deriving the delegated-credential shard key, so name/ID/relay-workspace-ID aliases that map to the same workspace resolve to one shard. It also adds a self-healing legacy-shard probe with a create-only atomic write (SaveAtomicIfMissing) to migrate credentials from old raw-aliased shards into the canonical shard.

Scope is tight and consistent with the PR purpose (headRef: fix/delegated-cred-workspace-shard-key). No edits were required.

Verification

• go build ./... — passes (exit 0)
• go test ./... (the exact CI command) — all packages pass, including cmd/relayfile-cli and internal/delegatedauth with the new tests
• gofmt -l on all four changed files — clean (no formatting drift)
• go vet ./cmd/relayfile-cli/... ./internal/delegatedauth/... — clean
• Traced callers: removed helpers (delegatedCredentialsWorkspaceKey, resolveDelegatedCredentialsWorkspaceValue) have zero remaining references; SaveAtomic retains its original error-only signature so its ~15 callers are unaffected; SaveAtomicIfMissing is only called from the new migration path.

Correctness notes (verified against the current checkout)

• saveAtomic create-only path is sound: on os.Link success cleanup stays true, so the deferred remove drops the temp file while the hard link survives. On os.ErrExist it returns (false, nil) and the temp is cleaned up — no overwrite of an existing credential file, preserving the create-only guarantee.
• workspaceRecordForShardValue correctly returns (_, false) when aliases are ambiguous (map to differing IDs) or when a matching record has an empty ID, so ambiguous values fall back to the raw shard rather than being force-collapsed. This is the safer (fail-to-raw, not fail-to-wrong-shard) behavior and is covered by TestDelegatedCredentialsPathLeavesAmbiguousRelayWorkspaceAliasRaw.
• The legacy probe seeds its seen set with the canonical path, avoiding redundant re-probe of the canonical shard. Migration writes are best-effort: on a benign migrate failure or non-created result it still returns the legacy bundle, so a write-permission issue degrades to "still works, just not migrated" rather than failing the request.

Addressed comments

• No bot or human review comments were provided to this run. The harness inputs (.workforce/) contain only pr.diff, changed-files.txt, and context.json; there is no comments file and no review payload, so there are no threads to account for.

Advisory Notes

• loadDelegatedCredentialsForRequest (main.go:7152-7155) computes canonicalWorkspaceValue only to emit a warning; on the not-OK branch the warned value is the un-canonicalized raw value. The message is accurate enough ("was not uniquely resolved") but slightly misnames the variable's contents. Purely cosmetic in a stderr warning — not worth a code change in this PR; flagging for awareness only.
• Whether the legacy-shard migration/probe needs additional test coverage for the "migrate write fails" branch is a human call; the existing tests cover the create and already-exists paths. I did not add tests (per policy, new tests are a human decision).

Verdict

Build, full test suite, gofmt, and vet all pass on the working tree as-is. The change is well-scoped, the fail-to-raw fallback is conservative, and no safety/lifecycle defaults are altered. No mechanical fixes were needed and no semantic changes are warranted from me.

I am not printing READY: I cannot confirm from this sandbox the live state of GitHub's required checks (pending/in-progress) or mergeability, and no human review presence was provided — those are post-harness conditions outside what I can verify here.