Skip to content

fix: thread resolved share URL into lazyBackfillAuthUserID#398

Merged
tomasz-tomczyk merged 1 commit intomainfrom
fix-lazy-backfill-share-url
Apr 30, 2026
Merged

fix: thread resolved share URL into lazyBackfillAuthUserID#398
tomasz-tomczyk merged 1 commit intomainfrom
fix-lazy-backfill-share-url

Conversation

@tomasz-tomczyk
Copy link
Copy Markdown
Owner

Summary

  • lazyBackfillAuthUserID was always resolving the whoami URL via resolveShareURL("", cfg, defaultShareURL), ignoring the --share-url flag that runShare had already resolved into sf.svcURL.
  • Whoami went to https://crit.md regardless, 401'd against selfhosted-issued tokens, and the 401 handler silently cleared the cached token before the actual share request fired — breaking crit share against any enforcing selfhosted instance.
  • Fix threads serverURL into lazyBackfillAuthUserID(cfg, serverURL) and updates both call sites (runShare, runAuthLogin). Empty-string fallback preserved for back-compat.

Review

  • Code review: passed (go-expert, fresh agent)
  • Parity audit: N/A (backend-only)

Test plan

  • 5 new tests in auth_test.go, two of which were verified to fail pre-fix:
    • explicit server is hit (not https://crit.md)
    • token NOT cleared when wrong server 401s
    • token IS still cleared when correct server 401s (semantics preserved)
    • env fallback when serverURL=""
    • skip cases (nil cfg, empty token, cached id)
  • go test -race -count=1 ./... → pass
  • golangci-lint run ./... → 0 issues

🤖 Generated with Claude Code

The whoami request now hits the URL passed via --share-url (or
CRIT_SHARE_URL), instead of always falling back to https://crit.md.
Previously, sharing to a selfhosted server caused whoami to 401 against
crit.md, which silently cleared the user's cached auth token before the
actual share request, breaking enforced selfhosted instances.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.09%. Comparing base (28c167f) to head (9eeeb38).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
auth.go 75.00% 1 Missing ⚠️
main.go 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (60.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #398      +/-   ##
==========================================
+ Coverage   66.66%   67.09%   +0.42%     
==========================================
  Files          19       19              
  Lines        8220     8221       +1     
==========================================
+ Hits         5480     5516      +36     
+ Misses       2314     2276      -38     
- Partials      426      429       +3     
Flag Coverage Δ
e2e 34.05% <0.00%> (-0.13%) ⬇️
unit 63.13% <60.00%> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tomasz-tomczyk tomasz-tomczyk merged commit cc4f6ef into main Apr 30, 2026
7 of 8 checks passed
@tomasz-tomczyk tomasz-tomczyk deleted the fix-lazy-backfill-share-url branch April 30, 2026 11:34
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.

1 participant