Migrate Auth to Result-based error handling; add shared Result/error infra - #71
Merged
Merged
Conversation
…infra Split out of #69 (part 2 of N — see that PR for the full picture). - features/auth/: Session/User/Tenant.create() and every AuthRepository method (initiateLogin/handleCallback/logout) return Result<T, E> instead of throwing; OidcAuthRepository's own try/catch around oidc-client-ts stays as a contained infrastructure-adapter boundary. See docs/adr/015-auth-result-errors.md (landing in a later PR in this stack, since it documents Catalog too). - shared/: adds Result.ts (Result/success/failure/flatMapResult/ combineResults), ErrorReporter port + ConsoleErrorReporter adapter, and rewrites useAsync.ts to take () => Promise<Result<T, E>> instead of a throwing () => Promise<T>. - test/fixtures/: adds authEntityFixtures.ts (shadow Tenant/User/Session fixtures that unwrap the Result so call sites read like before) and unwrapResult.ts. Temporary compatibility shims (flagged for removal in the next PR in this stack, which migrates Catalog to Result end-to-end and removes Tags): useCategories.ts/useServices.ts/useTags.ts wrap their existing list-fetch call in success()/failure() instead of letting it reject, and ~20 old Categories/Services/Tags test files that constructed Tenant/User directly now import the shadow fixtures instead — neither changes any other behavior, they only satisfy useAsync's new contract so this PR builds and tests green on its own without pulling in the unrelated Catalog reorg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a shared ChangesResult and async foundation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LoginPage
participant AuthProvider
participant OidcAuthRepository
participant OIDCProvider
LoginPage->>AuthProvider: login()
AuthProvider->>OidcAuthRepository: initiateLogin()
OidcAuthRepository->>OIDCProvider: start OIDC redirect
OIDCProvider-->>OidcAuthRepository: Result or mapped AuthFlowError
OidcAuthRepository-->>AuthProvider: Result<void, AuthFlowError>
AuthProvider-->>LoginPage: success or feedback error
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
apps/admin-frontend/src/features/auth/application/repositories/AuthRepository.ts (1)
19-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTrim inline ADR rationale to a short pointer.
Both comments restate the full docs/adr/014 rationale inline instead of referencing it briefly. The coding guideline requires keeping comments minimal and putting architectural rationale in ADRs, not in code comments.
apps/admin-frontend/src/features/auth/application/repositories/AuthRepository.ts#L19-L21: replace the full rationale with a short pointer, for example// null covers "no usable session" for all causes - see docs/adr/014.apps/admin-frontend/src/features/auth/presentation/AuthProvider.tsx#L19-L21: apply the same trim.🤖 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 `@apps/admin-frontend/src/features/auth/application/repositories/AuthRepository.ts` around lines 19 - 21, Trim the inline ADR rationale in AuthRepository and AuthProvider to a brief comment stating that null covers all “no usable session” causes and pointing to docs/adr/014; update both specified files and preserve the existing behavior.Source: Path instructions
🤖 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
`@apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts`:
- Around line 31-50: Restore the established throw-and-catch frontend error
contract across the auth flow: in
apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts:31-50,
throw typed AuthFlowError failures instead of returning Result unions; in
apps/admin-frontend/src/features/auth/application/use-cases/Logout.ts:12-13,
restore Promise<void> and propagate typed exceptions; in
apps/admin-frontend/src/features/auth/presentation/AuthContext.ts:15-17, expose
the existing action contracts; in
apps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.tsx:17-21,
catch failures and map them with toAuthFlowFeedback; and in
apps/admin-frontend/src/features/auth/application/test-helpers/createFakeAuthRepository.ts:14-17,
align fake defaults with the restored repository contract.
In `@apps/admin-frontend/src/features/auth/domain/entities/Session.ts`:
- Around line 23-28: Update Session.create to validate input.expiresAt with
Number.isFinite(input.expiresAt.getTime()) before constructing the Session,
returning InvalidSessionError for invalid dates so malformed expiry data fails
closed. Add coverage for an invalid expiry value in the session/OIDC mapping
tests.
In `@apps/admin-frontend/src/features/auth/domain/value-objects/Tenant.ts`:
- Line 2: Remove the Application-layer Result dependency from the domain
factories in Tenant, User.create, and Session.create. Restore the established
behavior of throwing the named InvalidTenantError or corresponding DomainError
subclasses on invalid input, while returning the domain value directly on
success. Remove the test-only unwrapResult compatibility wrappers and update
affected tests and callers to use the throw-and-catch contract.
In
`@apps/admin-frontend/src/features/auth/infrastructure/mapOidcErrorToAuthFlowError.ts`:
- Around line 110-127: Update the timeout and network messages returned by
mapLogoutError to refer to the authentication or logout service instead of the
login service, while preserving their existing meaning, codes, flowCode, and
retryable values.
In
`@apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts`:
- Around line 27-29: Update the expires_at validation in oidcUserToSessionMapper
to reject undefined, non-finite, non-integer, out-of-range, and Date-invalid
claim values before constructing the session; return a typed mapping failure
consistent with MissingExpiryClaimError handling, and add tests covering each
invalid-value category and valid boundaries.
- Around line 20-50: Restore the established throw-and-catch error contract: in
apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts:20-50,
update mapOidcUserToSession to throw the named mapping and domain-validation
errors instead of returning Result values. In
apps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.tsx:21-26,
catch login failures and pass the caught error to toAuthFlowFeedback. Update the
related auth repository interfaces, use cases, fixtures, and tests to use the
established DomainError/ApiError exception flow consistently.
In `@apps/admin-frontend/src/shared/application/ErrorReporter.ts`:
- Around line 2-9: Remove the ADR-level and behavior-description comments
without changing behavior: in
apps/admin-frontend/src/shared/application/ErrorReporter.ts lines 2-9 remove the
interface and error-flow-policy comments; in
apps/admin-frontend/src/shared/application/Result.ts lines 25-37 remove helper
and pipeline-policy comments; in
apps/admin-frontend/src/shared/infrastructure/observability/ConsoleErrorReporter.ts
lines 3-5 move the adapter-selection rationale to an ADR; in
apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts lines 58-62 and
161-162 remove the policy and behavior comments, and at lines 146-150 retain
only one short comment explaining the necessary lint suppression.
In `@apps/admin-frontend/src/shared/application/Result.ts`:
- Around line 1-46: Remove the frontend Result contract and helpers from
apps/admin-frontend/src/shared/application/Result.ts. In
apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts lines 63-121,
restore the callback and async handling to Promise<T>, catching thrown
DomainError and ApiError instances and mapping them through the existing error
flow; do not apply the backend-only Result pattern in the frontend.
In `@apps/admin-frontend/src/shared/presentation/hooks/useAsync.test.tsx`:
- Around line 34-36: Update the error assertion in the useAsync test to first
require result.current.error to be an Error with toBeInstanceOf(Error), then
narrow it and assert its message is "boom" so non-Error failures cannot pass
silently.
In `@apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts`:
- Around line 63-64: Restore useAsync’s asyncFn contract to return Promise<T>
instead of Promise<Result<T, E>>, and reinstate its existing exception-catching
and typed error-mapping behavior. Update the hook’s state handling and callers
such as catalog list hooks to pass plain async operations without converting
failures into Result values, while mapping DomainError and ApiError through
useAsync.
---
Nitpick comments:
In
`@apps/admin-frontend/src/features/auth/application/repositories/AuthRepository.ts`:
- Around line 19-21: Trim the inline ADR rationale in AuthRepository and
AuthProvider to a brief comment stating that null covers all “no usable session”
causes and pointing to docs/adr/014; update both specified files and preserve
the existing behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8365899e-c1dd-4ebb-ba6e-d5b6b76d7a7c
📒 Files selected for processing (69)
apps/admin-frontend/src/app/composition/container.test.tsapps/admin-frontend/src/app/layouts/AdminLayout.test.tsxapps/admin-frontend/src/features/auth/application/errors/AuthFlowError.tsapps/admin-frontend/src/features/auth/application/repositories/AuthRepository.tsapps/admin-frontend/src/features/auth/application/test-helpers/createFakeAuthRepository.tsapps/admin-frontend/src/features/auth/application/use-cases/GetCurrentSession.test.tsapps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.test.tsapps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.tsapps/admin-frontend/src/features/auth/application/use-cases/InitiateLogin.test.tsapps/admin-frontend/src/features/auth/application/use-cases/InitiateLogin.tsapps/admin-frontend/src/features/auth/application/use-cases/Logout.test.tsapps/admin-frontend/src/features/auth/application/use-cases/Logout.tsapps/admin-frontend/src/features/auth/domain/entities/Session.test.tsapps/admin-frontend/src/features/auth/domain/entities/Session.tsapps/admin-frontend/src/features/auth/domain/entities/User.test.tsapps/admin-frontend/src/features/auth/domain/entities/User.tsapps/admin-frontend/src/features/auth/domain/value-objects/Tenant.test.tsapps/admin-frontend/src/features/auth/domain/value-objects/Tenant.tsapps/admin-frontend/src/features/auth/infrastructure/MissingExpiryClaimError.tsapps/admin-frontend/src/features/auth/infrastructure/OidcAuthRepository.test.tsapps/admin-frontend/src/features/auth/infrastructure/OidcAuthRepository.tsapps/admin-frontend/src/features/auth/infrastructure/mapOidcErrorToAuthFlowError.tsapps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.test.tsapps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.tsapps/admin-frontend/src/features/auth/presentation/AuthContext.test.tsapps/admin-frontend/src/features/auth/presentation/AuthContext.tsapps/admin-frontend/src/features/auth/presentation/AuthProvider.test.tsxapps/admin-frontend/src/features/auth/presentation/AuthProvider.tsxapps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.test.tsxapps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.tsxapps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.test.tsxapps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.tsxapps/admin-frontend/src/features/auth/presentation/ProtectedRoute.test.tsxapps/admin-frontend/src/features/auth/presentation/TenantBoundary.test.tsxapps/admin-frontend/src/features/auth/presentation/authFlowFeedback.tsapps/admin-frontend/src/features/auth/presentation/useAuthenticatedTenant.test.tsxapps/admin-frontend/src/features/catalog/application/use-cases/categories/CreateCategory.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/categories/DeleteCategory.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/categories/ListCategories.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/categories/UpdateCategory.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/services/CreateService.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/services/DeleteService.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/services/ListServices.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/services/UpdateService.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/tags/CreateTag.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/tags/DeleteTag.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/tags/ListTags.test.tsapps/admin-frontend/src/features/catalog/application/use-cases/tags/UpdateTag.test.tsapps/admin-frontend/src/features/catalog/infrastructure/repositories/ApiCategoryRepository.test.tsapps/admin-frontend/src/features/catalog/infrastructure/repositories/ApiServiceRepository.test.tsapps/admin-frontend/src/features/catalog/infrastructure/repositories/ApiTagRepository.test.tsapps/admin-frontend/src/features/catalog/presentation/categories/CategoriesPage.test.tsxapps/admin-frontend/src/features/catalog/presentation/categories/hooks/useCategories.test.tsxapps/admin-frontend/src/features/catalog/presentation/categories/hooks/useCategories.tsapps/admin-frontend/src/features/catalog/presentation/services/ServicesPage.testSupport.tsxapps/admin-frontend/src/features/catalog/presentation/services/hooks/useServices.test.tsxapps/admin-frontend/src/features/catalog/presentation/services/hooks/useServices.tsapps/admin-frontend/src/features/catalog/presentation/tags/TagsPage.test.tsxapps/admin-frontend/src/features/catalog/presentation/tags/hooks/useTags.test.tsxapps/admin-frontend/src/features/catalog/presentation/tags/hooks/useTags.tsapps/admin-frontend/src/shared/application/ErrorReporter.tsapps/admin-frontend/src/shared/application/Result.test.tsapps/admin-frontend/src/shared/application/Result.tsapps/admin-frontend/src/shared/infrastructure/observability/ConsoleErrorReporter.tsapps/admin-frontend/src/shared/presentation/hooks/useAsync.test.tsxapps/admin-frontend/src/shared/presentation/hooks/useAsync.tsapps/admin-frontend/src/test/fixtures/authEntityFixtures.tsapps/admin-frontend/src/test/fixtures/createFakeAppContainer.tsapps/admin-frontend/src/test/fixtures/unwrapResult.ts
Comment on lines
+31
to
+50
| async execute(callbackUrl: string): Promise<Result<CompletedAuthCallback, AuthFlowError>> { | ||
| if (this.cached?.url !== callbackUrl) { | ||
| this.cached = { url: callbackUrl, promise: this.performCallback(callbackUrl) } | ||
| } | ||
|
|
||
| return this.cached.promise | ||
| } | ||
|
|
||
| private async performCallback(callbackUrl: string): Promise<CompletedAuthCallback> { | ||
| const { session, returnTo } = await this.authRepository.handleCallback(callbackUrl) | ||
|
|
||
| return { | ||
| tenantContext: toTenantContext(session.user), | ||
| returnTo: resolvePostLoginPath(returnTo), | ||
| private async performCallback( | ||
| callbackUrl: string, | ||
| ): Promise<Result<CompletedAuthCallback, AuthFlowError>> { | ||
| const result = await this.authRepository.handleCallback(callbackUrl) | ||
| if (!result.success) { | ||
| return result | ||
| } | ||
|
|
||
| return success({ | ||
| tenantContext: toTenantContext(result.value.session.user), | ||
| returnTo: resolvePostLoginPath(result.value.returnTo), | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep the established frontend error contract.
This migration introduces a second error-handling model in apps/admin-frontend. Existing frontend flows use typed thrown errors and presentation-layer error mapping. The Result contract also requires temporary Catalog compatibility adapters, which confirms the contract split.
apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts#L31-L50: restore the throw-and-catch callback contract with typedAuthFlowErrorfailures.apps/admin-frontend/src/features/auth/application/use-cases/Logout.ts#L12-L13: restore thePromise<void>use-case contract and propagate typed failures through exceptions.apps/admin-frontend/src/features/auth/presentation/AuthContext.ts#L15-L17: expose the existing action contracts instead of Result unions.apps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.tsx#L17-L21: restore error catching andtoAuthFlowFeedbackmapping.apps/admin-frontend/src/features/auth/application/test-helpers/createFakeAuthRepository.ts#L14-L17: update fake defaults to match the restored repository contract.
Based on learnings, apps/admin-frontend must use the established throw-and-catch convention for domain and HTTP failures and must not apply the backend-only Result policy to frontend code.
📍 Affects 5 files
apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts#L31-L50(this comment)apps/admin-frontend/src/features/auth/application/use-cases/Logout.ts#L12-L13apps/admin-frontend/src/features/auth/presentation/AuthContext.ts#L15-L17apps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.tsx#L17-L21apps/admin-frontend/src/features/auth/application/test-helpers/createFakeAuthRepository.ts#L14-L17
🤖 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
`@apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts`
around lines 31 - 50, Restore the established throw-and-catch frontend error
contract across the auth flow: in
apps/admin-frontend/src/features/auth/application/use-cases/HandleAuthCallback.ts:31-50,
throw typed AuthFlowError failures instead of returning Result unions; in
apps/admin-frontend/src/features/auth/application/use-cases/Logout.ts:12-13,
restore Promise<void> and propagate typed exceptions; in
apps/admin-frontend/src/features/auth/presentation/AuthContext.ts:15-17, expose
the existing action contracts; in
apps/admin-frontend/src/features/auth/presentation/CallbackPage/CallbackPage.tsx:17-21,
catch failures and map them with toAuthFlowFeedback; and in
apps/admin-frontend/src/features/auth/application/test-helpers/createFakeAuthRepository.ts:14-17,
align fake defaults with the restored repository contract.
Source: Learnings
Comment on lines
+23
to
+28
| static create(input: CreateSessionInput): Result<Session, InvalidSessionError> { | ||
| if (input.accessToken.trim().length === 0) { | ||
| throw new InvalidSessionError('Session access token must not be empty') | ||
| return failure(new InvalidSessionError('Session access token must not be empty')) | ||
| } | ||
|
|
||
| return new Session(input.user, input.accessToken, input.expiresAt) | ||
| return success(new Session(input.user, input.accessToken, input.expiresAt)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject invalid session expiry values.
mapOidcUserToSession only rejects expires_at === undefined before it creates a Date. A nonnumeric or out-of-range runtime value creates an invalid Date. isExpiredAt then compares against NaN and returns false, which can treat the session as unexpired.
Validate Number.isFinite(input.expiresAt.getTime()) before constructing the session. Add coverage for an invalid expiry value so malformed OIDC data fails closed.
🤖 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 `@apps/admin-frontend/src/features/auth/domain/entities/Session.ts` around
lines 23 - 28, Update Session.create to validate input.expiresAt with
Number.isFinite(input.expiresAt.getTime()) before constructing the Session,
returning InvalidSessionError for invalid dates so malformed expiry data fails
closed. Add coverage for an invalid expiry value in the session/OIDC mapping
tests.
| @@ -1,4 +1,5 @@ | |||
| import { InvalidTenantError } from '@/features/auth/domain/errors/InvalidTenantError' | |||
| import { failure, success, type Result } from '@/shared/application/Result' | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Remove the Application-layer Result dependency from the domain factory.
Tenant is a Domain type, but it imports @/shared/application/Result. User.ts and Session.ts repeat the same outward dependency. This violates the required Domain → Application inward dependency direction.
This also replaces the established frontend DomainError throw-and-catch contract. It forces test-only unwrapResult compatibility wrappers. Restore InvalidTenantError throwing here, apply the same change to User.create and Session.create, and remove the compatibility fixture layer.
As per coding guidelines, each app must use “Domain → Application → Infrastructure/Presentation layering and inward-only dependencies.” Based on learnings, domain factories should throw named DomainError subclasses and should not introduce the backend Result pattern into apps/admin-frontend.
Also applies to: 11-18
🤖 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 `@apps/admin-frontend/src/features/auth/domain/value-objects/Tenant.ts` at line
2, Remove the Application-layer Result dependency from the domain factories in
Tenant, User.create, and Session.create. Restore the established behavior of
throwing the named InvalidTenantError or corresponding DomainError subclasses on
invalid input, while returning the domain value directly on success. Remove the
test-only unwrapResult compatibility wrappers and update affected tests and
callers to use the throw-and-catch contract.
Sources: Coding guidelines, Learnings
Comment on lines
+110
to
+127
| export function mapLogoutError(error: unknown): AuthFlowError { | ||
| if (error instanceof ErrorTimeout) { | ||
| return new AuthFlowError({ | ||
| code: 'timeout', | ||
| flowCode: 'AUTH_LOGOUT_FAILED', | ||
| message: 'O serviço de login demorou para confirmar a saída. Tente novamente.', | ||
| retryable: true, | ||
| }) | ||
| } | ||
|
|
||
| if (error instanceof TypeError) { | ||
| return new AuthFlowError({ | ||
| code: 'network', | ||
| flowCode: 'AUTH_LOGOUT_FAILED', | ||
| message: 'Não foi possível conectar ao serviço de login para concluir a saída.', | ||
| retryable: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix "login" wording in logout error messages.
O serviço de login demorou para confirmar a saída. and Não foi possível conectar ao serviço de login para concluir a saída. both name the login service during a logout failure. Update the wording to refer to the authentication or logout service, not the login service.
✏️ Proposed wording fix
if (error instanceof ErrorTimeout) {
return new AuthFlowError({
code: 'timeout',
flowCode: 'AUTH_LOGOUT_FAILED',
- message: 'O serviço de login demorou para confirmar a saída. Tente novamente.',
+ message: 'O serviço de autenticação demorou para confirmar a saída. Tente novamente.',
retryable: true,
})
}
if (error instanceof TypeError) {
return new AuthFlowError({
code: 'network',
flowCode: 'AUTH_LOGOUT_FAILED',
- message: 'Não foi possível conectar ao serviço de login para concluir a saída.',
+ message: 'Não foi possível conectar ao serviço de autenticação para concluir a saída.',
retryable: true,
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function mapLogoutError(error: unknown): AuthFlowError { | |
| if (error instanceof ErrorTimeout) { | |
| return new AuthFlowError({ | |
| code: 'timeout', | |
| flowCode: 'AUTH_LOGOUT_FAILED', | |
| message: 'O serviço de login demorou para confirmar a saída. Tente novamente.', | |
| retryable: true, | |
| }) | |
| } | |
| if (error instanceof TypeError) { | |
| return new AuthFlowError({ | |
| code: 'network', | |
| flowCode: 'AUTH_LOGOUT_FAILED', | |
| message: 'Não foi possível conectar ao serviço de login para concluir a saída.', | |
| retryable: true, | |
| }) | |
| } | |
| export function mapLogoutError(error: unknown): AuthFlowError { | |
| if (error instanceof ErrorTimeout) { | |
| return new AuthFlowError({ | |
| code: 'timeout', | |
| flowCode: 'AUTH_LOGOUT_FAILED', | |
| message: 'O serviço de autenticação demorou para confirmar a saída. Tente novamente.', | |
| retryable: true, | |
| }) | |
| } | |
| if (error instanceof TypeError) { | |
| return new AuthFlowError({ | |
| code: 'network', | |
| flowCode: 'AUTH_LOGOUT_FAILED', | |
| message: 'Não foi possível conectar ao serviço de autenticação para concluir a saída.', | |
| retryable: true, | |
| }) | |
| } |
🤖 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
`@apps/admin-frontend/src/features/auth/infrastructure/mapOidcErrorToAuthFlowError.ts`
around lines 110 - 127, Update the timeout and network messages returned by
mapLogoutError to refer to the authentication or logout service instead of the
login service, while preserving their existing meaning, codes, flowCode, and
retryable values.
Comment on lines
+20
to
50
| export function mapOidcUserToSession(oidcUser: OidcUser): Result<Session, SessionMappingError> { | ||
| const tenantId = oidcUser.profile.tenant_id | ||
|
|
||
| if (typeof tenantId !== 'string' || tenantId.trim().length === 0) { | ||
| throw new MissingTenantClaimError() | ||
| return failure(new MissingTenantClaimError()) | ||
| } | ||
|
|
||
| if (oidcUser.expires_at === undefined) { | ||
| throw new Error('oidc-client-ts User is missing expires_at; cannot determine session validity') | ||
| return failure(new MissingExpiryClaimError()) | ||
| } | ||
|
|
||
| const tenantResult = Tenant.create(tenantId) | ||
| if (!tenantResult.success) { | ||
| return tenantResult | ||
| } | ||
|
|
||
| const tenant = Tenant.create(tenantId) | ||
| const user = User.create({ | ||
| const userResult = User.create({ | ||
| id: oidcUser.profile.sub, | ||
| tenant, | ||
| tenant: tenantResult.value, | ||
| ...(typeof oidcUser.profile.email === 'string' ? { email: oidcUser.profile.email } : {}), | ||
| ...(typeof oidcUser.profile.name === 'string' ? { name: oidcUser.profile.name } : {}), | ||
| }) | ||
| if (!userResult.success) { | ||
| return userResult | ||
| } | ||
|
|
||
| return Session.create({ | ||
| user, | ||
| user: userResult.value, | ||
| accessToken: oidcUser.access_token, | ||
| expiresAt: new Date(oidcUser.expires_at * 1000), | ||
| }) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Restore the established frontend exception flow.
These changes introduce a second frontend error contract based on Result. The established frontend contract uses named DomainError and ApiError exceptions, then maps caught errors in presentation flows.
apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts#L20-L50: restore named thrown mapping and domain-validation errors instead of returningResult.apps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.tsx#L21-L26: catch the login failure and pass it totoAuthFlowFeedback.
Update the related repository interfaces, use cases, fixtures, and tests in the same change.
Based on learnings, frontend domain and HTTP failures must use the established throw-and-catch convention rather than the backend Result pattern.
📍 Affects 2 files
apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts#L20-L50(this comment)apps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.tsx#L21-L26
🤖 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
`@apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts`
around lines 20 - 50, Restore the established throw-and-catch error contract: in
apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts:20-50,
update mapOidcUserToSession to throw the named mapping and domain-validation
errors instead of returning Result values. In
apps/admin-frontend/src/features/auth/presentation/LoginPage/LoginPage.tsx:21-26,
catch login failures and pass the caught error to toAuthFlowFeedback. Update the
related auth repository interfaces, use cases, fixtures, and tests to use the
established DomainError/ApiError exception flow consistently.
Source: Learnings
Comment on lines
27
to
+29
| if (oidcUser.expires_at === undefined) { | ||
| throw new Error('oidc-client-ts User is missing expires_at; cannot determine session validity') | ||
| return failure(new MissingExpiryClaimError()) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject malformed expires_at values.
This check only rejects undefined. A defined NaN, infinity, non-integer, or out-of-range value produces Invalid Date. Session.isExpiredAt() then compares against NaN and returns false, so cached-session recovery can accept the malformed session as non-expired.
Validate that the claim is a finite integer and produces a valid Date. Return a typed mapping failure and add invalid-value tests.
As per coding guidelines, domain factories must runtime-validate externally sourced generated API values, including finite numeric and integer constraints.
🤖 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
`@apps/admin-frontend/src/features/auth/infrastructure/oidcUserToSessionMapper.ts`
around lines 27 - 29, Update the expires_at validation in
oidcUserToSessionMapper to reject undefined, non-finite, non-integer,
out-of-range, and Date-invalid claim values before constructing the session;
return a typed mapping failure consistent with MissingExpiryClaimError handling,
and add tests covering each invalid-value category and valid boundaries.
Source: Coding guidelines
Comment on lines
+2
to
+9
| /** Which capture surface this came through - e.g. "react.onCaughtError", "window.unhandledrejection". */ | ||
| readonly source: string | ||
| readonly extra?: Record<string, unknown> | ||
| } | ||
|
|
||
| // The one sink every genuinely-unexpected failure (a bug, not an expected | ||
| // Result.Failure) flows through - business/presentation code never reports | ||
| // directly, only the capture surfaces wired in main.tsx do (docs/adr/014). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Move architectural rationale out of source comments.
The shared root cause is ADR-level and behavior-description comments in implementation files. Keep only short comments that explain non-obvious constraints.
apps/admin-frontend/src/shared/application/ErrorReporter.ts#L2-L9: remove interface-description and error-flow-policy comments.apps/admin-frontend/src/shared/application/Result.ts#L25-L37: remove helper behavior and pipeline-policy comments.apps/admin-frontend/src/shared/infrastructure/observability/ConsoleErrorReporter.ts#L3-L5: move adapter-selection rationale to an ADR.apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L58-L62: remove Result-flow policy rationale.apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L146-L150: reduce the permitted lint-suppression reason to one short comment.apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L161-L162: remove behavior-description comments.
As per coding guidelines: comments must explain only non-obvious reasons and must not contain ADR-level rationale.
📍 Affects 4 files
apps/admin-frontend/src/shared/application/ErrorReporter.ts#L2-L9(this comment)apps/admin-frontend/src/shared/application/Result.ts#L25-L37apps/admin-frontend/src/shared/infrastructure/observability/ConsoleErrorReporter.ts#L3-L5apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L58-L62apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L146-L150apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L161-L162
🤖 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 `@apps/admin-frontend/src/shared/application/ErrorReporter.ts` around lines 2 -
9, Remove the ADR-level and behavior-description comments without changing
behavior: in apps/admin-frontend/src/shared/application/ErrorReporter.ts lines
2-9 remove the interface and error-flow-policy comments; in
apps/admin-frontend/src/shared/application/Result.ts lines 25-37 remove helper
and pipeline-policy comments; in
apps/admin-frontend/src/shared/infrastructure/observability/ConsoleErrorReporter.ts
lines 3-5 move the adapter-selection rationale to an ADR; in
apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts lines 58-62 and
161-162 remove the policy and behavior comments, and at lines 146-150 retain
only one short comment explaining the necessary lint suppression.
Source: Coding guidelines
Comment on lines
+1
to
+46
| export interface Success<T> { | ||
| readonly success: true | ||
| readonly value: T | ||
| } | ||
|
|
||
| export interface Failure<E> { | ||
| readonly success: false | ||
| readonly error: E | ||
| } | ||
|
|
||
| export type Result<T, E> = Success<T> | Failure<E> | ||
|
|
||
| export function success<T>(value: T): Success<T> { | ||
| return { success: true, value } | ||
| } | ||
|
|
||
| export function failure<E>(error: E): Failure<E> { | ||
| return { success: false, error } | ||
| } | ||
|
|
||
| export function mapResult<T, U, E>(result: Result<T, E>, transform: (value: T) => U): Result<U, E> { | ||
| return result.success ? success(transform(result.value)) : result | ||
| } | ||
|
|
||
| // Chains a Result-returning step onto another Result without nesting - | ||
| // the composition primitive that keeps a pipeline of possibly-failing | ||
| // steps (decode -> domain validation) throw-free end to end. | ||
| export function flatMapResult<T, U, E>( | ||
| result: Result<T, E>, | ||
| transform: (value: T) => Result<U, E>, | ||
| ): Result<U, E> { | ||
| return result.success ? transform(result.value) : result | ||
| } | ||
|
|
||
| // Fail-fast: the first Failure in the list short-circuits the rest: the | ||
| // same "any one item can spoil the batch" semantics as Promise.all, for | ||
| // synchronous Results instead of promises. | ||
| export function combineResults<T, E>(results: readonly Result<T, E>[]): Result<T[], E> { | ||
| const values: T[] = [] | ||
| for (const result of results) { | ||
| if (!result.success) { | ||
| return result | ||
| } | ||
| values.push(result.value) | ||
| } | ||
| return success(values) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Restore the existing exception-based frontend error flow.
The shared Result API and the useAsync callback contract introduce a Result-based frontend error model. This conflicts with the established DomainError and ApiError exception flow.
apps/admin-frontend/src/shared/application/Result.ts#L1-L46: remove the shared Result contract from the frontend error path.apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L63-L121: restorePromise<T>handling and catch/map typed thrown errors in the hook.
Based on learnings: apps/admin-frontend must not apply the backend-only Result-pattern policy.
📍 Affects 2 files
apps/admin-frontend/src/shared/application/Result.ts#L1-L46(this comment)apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts#L63-L121
🤖 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 `@apps/admin-frontend/src/shared/application/Result.ts` around lines 1 - 46,
Remove the frontend Result contract and helpers from
apps/admin-frontend/src/shared/application/Result.ts. In
apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts lines 63-121,
restore the callback and async handling to Promise<T>, catching thrown
DomainError and ApiError instances and mapping them through the existing error
flow; do not apply the backend-only Result pattern in the frontend.
Source: Learnings
Comment on lines
+34
to
+36
| if (result.current.error instanceof Error) { | ||
| expect(result.current.error.message).toBe('boom') | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the error type before reading its message.
If result.current.error is not an Error, this test performs no message assertion and still passes. Assert toBeInstanceOf(Error) before narrowing and checking "boom".
🤖 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 `@apps/admin-frontend/src/shared/presentation/hooks/useAsync.test.tsx` around
lines 34 - 36, Update the error assertion in the useAsync test to first require
result.current.error to be an Error with toBeInstanceOf(Error), then narrow it
and assert its message is "boom" so non-Error failures cannot pass silently.
Comment on lines
+63
to
+64
| export function useAsync<T, E = unknown>( | ||
| asyncFn: () => Promise<Result<T, E>>, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep useAsync on the existing exception-and-catch contract.
This signature requires callers to convert thrown domain and API errors into Result.Failure. That duplicates the existing presentation error boundary and forces adapters such as catalog list hooks to catch errors only to rewrap them. Restore the Promise<T> callback contract and its typed error mapping.
Based on learnings: presentation flows must catch and map DomainError and ApiError through useAsync, rather than adopting a frontend Result pattern.
Also applies to: 111-121
🤖 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 `@apps/admin-frontend/src/shared/presentation/hooks/useAsync.ts` around lines
63 - 64, Restore useAsync’s asyncFn contract to return Promise<T> instead of
Promise<Result<T, E>>, and reinstate its existing exception-catching and typed
error-mapping behavior. Update the hook’s state handling and callers such as
catalog list hooks to pass plain async operations without converting failures
into Result values, while mapping DomainError and ApiError through useAsync.
Source: Learnings
Merged
5 tasks
evertonschuster
added a commit
that referenced
this pull request
Aug 2, 2026
…ervices; physical reorg (#75) * Migrate Catalog (Categories/Services/Tags) to Result errors; remove Services vertical; physical reorg Split out of #69 (part 3 of 4 — see that PR for the full picture). Recreated from origin/main after #70 and #71 merged, since this repo's convention (and the split-large-coderabbit-pr skill) is a sequential series, not stacking on an unmerged branch — CodeRabbit also doesn't review PRs whose base isn't the default branch, so stacking silently skipped review for this and the next PR in the series. Same content as the original #72, just re-based; no functional change. This PR is larger than the <100-file target used for the other PRs in this series, deliberately — see "Why this couldn't be split further" below. - Categories, Services, and Tags all move to the Result-based error convention docs/adr/014 establishes: domain entities' create() methods, mappers, and API repositories return Result<T, AppError> instead of throwing; useAsync.ts (already Result-based, landed in #71) is the one hook every feature's data layer builds on now. - app/composition/container.ts's CatalogFacade drops the use-case-class indirection (ListCategories/CreateTag/etc. as separate classes) for direct repository delegation (`{ execute: repo.method }`) - there's no orchestration between the facade and the repository, so the extra class per operation wasn't earning its keep. The 24 now-orphaned use-case-class files (application/use-cases/{categories,services,tags}/) are deleted. - Services' frontend implementation (ServicesPage, ServiceForm, six ServicesPage.*.test.tsx files, all its components/hooks/models) is fully removed, reverting `/services` to a placeholder page (app/pages/ServicesPage/ServicesPage.tsx) - this vertical is going back to `stub` status, see docs/STATUS.md. - Categories moves to a routed create/edit dialog (features/catalog/presentation/categories/pages/CategoriesListPage/, .../CategoryEditorDialog/) per docs/adr/012, replacing the old flat CategoriesPage.tsx/useCategories.ts/CategoryEditorDialog.tsx shape. - Tags gets the equivalent internal move (hooks/useTagEditor.ts, pages/TagEditorDialog.tsx) and its own Result migration (Tag.ts/tagMapper.ts/ApiTagRepository.ts) - Tags itself is not being removed here, just migrated; its removal is a later PR in this stack (docs/adr/016). - shared/: AuthenticatedHttpClient's get/post/put/delete now return Result<T, AppError> instead of throwing; DeleteConfirmationDialog takes entityName/entityType instead of a raw title/description pair; useCreateInline is removed (no longer used once Services - its only consumer - is gone). - Also removes .husky/pre-commit and fixes architecture_guard.py's precommit check accordingly (already merged independently via #70; included here too since this branch's own ancestry needed it before #70 existed). ## Why this couldn't be split further I initially tried a narrower "Category-only foundation" PR (~99 files) deferring Services/Tags. That failed a real build: app/composition/ container.ts wires TagRepository with its *new* method signature directly (tagRepository.listAll(options) instead of the old (tenantContext, options) two-arg form) - not just a return-type change useAsync-style, but the interface itself. Making that build without also migrating TagRepository/ApiTagRepository/tagMapper/Tag.ts for real isn't a smaller wrapper shim - it's the same size of work as just finishing the migration, since there's no reduced version of an interface signature. Categories, Services, and Tags share container.ts's catalog wiring, router.tsx, and AuthenticatedHttpClient tightly enough that they're one atomic, verified-buildable unit at this layer - mirroring why Auth couldn't be split from Catalog either, just one layer down. ## Test plan - [x] `npm install` + `npm run build --workspace=apps/admin-frontend` — green - [x] `npm run lint --workspace=apps/admin-frontend` — clean, 0 warnings - [x] `npm run format:check --workspace=apps/admin-frontend` — clean - [x] `npm run test --workspace=apps/admin-frontend` — 368/368 passing - [x] `scripts/sync_agent_skills.py --check`, `scripts/check_agent_governance.py`, `scripts/architecture_guard.py` — all pass Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix categories-mobile.spec.ts's mock for the GET-by-id endpoint useCategoryEditor fetches its own category via GET /api/v1/categories/{id} (docs/adr/013), but this spec's route mock matched any /api/v1/categories* path and always returned the full list array regardless of whether the request was for the collection or a single id - so the by-id fetch received an array instead of a CategoryDto, and the edit dialog's Nome field never populated. Mock now inspects the last path segment and returns the matching single category (404 if not found) for a by-id GET, the full list otherwise. Verified against the real Playwright suite (production build + preview, matching CI): all 10 e2e specs pass, including this one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
evertonschuster
added a commit
that referenced
this pull request
Aug 2, 2026
Split out of #69 (final part of this series — see that PR for the full picture). Recreated from origin/main after #70/#71/#75 merged, since this repo's convention (and the split-large-coderabbit-pr skill) is a sequential series, not stacking on an unmerged branch. Same content as the original #73, just re-based; no functional change. Removes the entire Tags vertical from apps/admin-frontend (domain, application, infrastructure, presentation, MSW handlers, E2E specs, nav entry, route, and catalog facade wiring) while intentionally retaining the backend Tag domain entity and /api/v1/tags endpoints, including Service's many-to-many relationship to Tag - a project-owner decision, see docs/adr/016-remove-tags-frontend.md. Categories replaces Tags as the reference CRUD implementation throughout the docs and the agenza-frontend-feature skill. ## Test plan - [x] `npm install` + `npm run build --workspace=apps/admin-frontend` — green - [x] `npm run lint --workspace=apps/admin-frontend` — clean, 0 warnings - [x] `npm run format:check --workspace=apps/admin-frontend` — clean - [x] `npm run test --workspace=apps/admin-frontend` — 305/305 passing - [x] `npx playwright test` (full e2e suite, production build + preview) — 8/8 passing - [x] `scripts/sync_agent_skills.py --check`, `scripts/check_agent_governance.py`, `scripts/architecture_guard.py` — all pass Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
evertonschuster
added a commit
that referenced
this pull request
Aug 2, 2026
…#77) These 3 files (agent-skills/agenza-frontend-feature/SKILL.md and its two synced copies under .claude/skills/ and .agents/skills/) live at the repo root, outside apps/admin-frontend/ — every diff I computed while splitting #69 into #70/#71/#75/#76 was scoped to apps/admin-frontend (and backend/ for #70), so these files' accumulated updates from this session (Catalog Result migration, Auth Result migration, and finally the Tags-removal doc pass replacing TagsPage/TagForm with Categories as the reference implementation) never made it into any of the split PRs, even though the actual code changes they describe are all correctly merged. Content taken directly from the original branch's final commit (4911abb), already reviewed and governance-checked at the time. Verified again here against the current merged main: sync_agent_skills.py --check, check_agent_governance.py, and architecture_guard.py all pass, and the file paths the skill references (CategoriesListPage.tsx, CategoryForm.tsx, categoryMapper.ts, AdminLayout.tsx) all exist in the current tree. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split out of #69 (part 2 — see that PR for the full picture; independently buildable and mergeable on its own, verified with a real build/test run rather than just a partial checkout).
features/auth/:Session/User/Tenant.create()and everyAuthRepositorymethod (initiateLogin/handleCallback/logout) returnResult<T, E>instead of throwing.OidcAuthRepository's owntry/catcharoundoidc-client-tscalls stays as a contained infrastructure-adapter boundary.shared/: addsResult.ts(Result/success/failure/flatMapResult/combineResults), anErrorReporterport +ConsoleErrorReporteradapter, and rewritesuseAsync.tsto take() => Promise<Result<T, E>>instead of a throwing() => Promise<T>.test/fixtures/: addsauthEntityFixtures.ts(shadowTenant/User/Sessionfixtures that unwrap theResultso call sites read like before) andunwrapResult.ts.Temporary compatibility shims, flagged for removal in the next PR in this stack (which migrates Catalog to Result end-to-end and removes Tags):
useCategories.ts/useServices.ts/useTags.tswrap their existing list-fetch call insuccess()/failure()instead of letting it reject, and ~20 old Categories/Services/Tags test files that constructedTenant/Userdirectly now import the shadow fixtures instead. Neither changes any other behavior — they only satisfyuseAsync's new contract so this PR builds and tests green on its own, without pulling in the unrelated Catalog reorg (confirmed:useAsync.ts/Result.ts/AuthenticatedHttpClient-adjacent changes are used by every catalog feature, not just Auth — this PR deliberately excludesAuthenticatedHttpClient/HttpClient/DeleteConfirmationDialog/useDeleteConfirmationsince Auth doesn't actually depend on them; those land in the Catalog PR instead).Test plan
npm install+npm run build --workspace=apps/admin-frontend— green (verified with a real build, not just type-checking a partial file set)npm run lint --workspace=apps/admin-frontend— clean, 0 warningsnpm run format:check --workspace=apps/admin-frontend— cleannpm run test --workspace=apps/admin-frontend— 559/559 passingscripts/sync_agent_skills.py --check,scripts/check_agent_governance.py,scripts/architecture_guard.py— all pass🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Reliability