Normalize browser API auth helpers to use authedFetch#332
Conversation
Replace all plain fetch() calls in browser UI components with authedFetch() to ensure Basic Auth credentials are included for protected mutating routes. - src/app/automation/page.tsx: Replace 8 fetch() calls with authedFetch() - src/app/automation/repos/[...repo]/page.tsx: Replace 3 fetch() calls with authedFetch() - src/components/issue-card.tsx: Replace 5 fetch() calls with authedFetch() - Add vitest regression tests for client-auth Basic Auth behavior Fixes #311
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: review@https://litellm.jory.dev/v1 (openai)
Recommendation
Approve. This PR correctly addresses the issue of inconsistent authentication in the browser UI by replacing plain fetch() calls with the project-standard authedFetch() for mutating API routes. It also includes comprehensive regression tests for the authentication logic.
Change-by-Change Findings
src/app/automation/page.tsx: Replaced multiplefetch()calls (GET and POST/DELETE) withauthedFetch()to ensureAuthorizationheaders are included whenDISPATCH_AUTH_MODE=basicis enabled.src/app/automation/repos/[...repo]/page.tsx: Replacedfetch()calls withauthedFetch()for repository details and automation run triggers.src/components/issue-card.tsx: Updated issue action triggers (agent assignment, sync, unassign, groom) to useauthedFetch().src/lib/client-auth.test.ts: Added a new test suite using Vitest to verifyauthedFetchcorrectly attaches Basic Auth headers, preserves existing headers, and handles empty credentials.
Standards Compliance
- Authentication: Follows the established pattern of using
authedFetch()for protected mutating routes as described in theREADME.mdandAGENTS.md. - Testing: Adheres to the requirement for meaningful testing by adding a dedicated test file for the client authentication utility.
- Code Quality: The implementation is consistent with the existing codebase and addresses the specific technical debt identified in the linked issue.
Linked Issue Fit
- Issue PR 311: This PR directly implements the recommendation from the audit: "replace protected mutating UI
fetchcalls withauthedFetchor a shared client wrapper and add Basic Auth mode regression tests." It covers all the specific file locations identified in the issue body.
Unknowns or Needs Verification
- None identified. The changes are localized to client-side authentication handling and are well-covered by the new tests.
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: review@https://litellm.jory.dev/v1 (openai)
Recommendation
Approve. This PR correctly addresses the issue of inconsistent authentication in the browser UI by replacing plain fetch() calls with authedFetch() for mutating API routes. It also includes comprehensive regression tests for the authentication logic.
Change-by-Change Findings
src/app/automation/page.tsx: Replaced multiplefetch()calls (GET and POST/DELETE) withauthedFetch()to ensureAuthorizationheaders are included whenDISPATCH_AUTH_MODE=basicis enabled.src/app/automation/repos/[...repo]/page.tsx: Replacedfetch()calls withauthedFetch()for repository details and automation run triggers.src/components/issue-card.tsx: Updated several mutation actions (assigning agents, syncing, unassigning, and grooming) to useauthedFetch().src/lib/client-auth.test.ts: Added a new test suite usingvitestto verifyauthedFetchcorrectly attaches Basic Auth headers, preserves existing headers, and handles missing credentials.
Standards Compliance
- Code Standards: Follows the pattern of using
authedFetchfor protected routes as documented in the repository's README. - Error Handling: The implementation maintains existing error handling patterns in the UI components.
- Testing: The addition of
src/lib/client-auth.test.tsprovides necessary coverage for the authentication helper, ensuring no regressions in header injection logic.
Linked Issue Fit
- Issue PR 311: The PR directly implements the recommendation from the linked issue: "Replace protected mutating UI
fetchcalls withauthedFetchor a shared client wrapper and add Basic Auth mode regression tests." The implementation matches the requirements perfectly.
Unknowns or Needs Verification
- None identified. The logic for
authedFetchis verified by the new test suite.
Fixes #311
Replace all plain
fetch()calls in browser UI components withauthedFetch()to ensure Basic Auth credentials are included for protected mutating routes.
Changes:
src/app/automation/page.tsx: Replace 8fetch()calls withauthedFetch()src/app/automation/repos/[...repo]/page.tsx: Replace 3fetch()calls withauthedFetch()src/components/issue-card.tsx: Replace 5fetch()calls withauthedFetch()client-authBasic Auth behaviorProblem:
When
DISPATCH_AUTH_MODE=basic, plainfetch()calls to protected API routesdo not include the
Authorization: Basic ...header, causing 401 errors.Some UI paths used
authedFetch()while others used plainfetch(), leadingto inconsistent behavior depending on which page/component was active.