Skip to content

Validate credentials on save#1106

Open
simple-agent-manager[bot] wants to merge 6 commits into
mainfrom
sam/add-credential-validation-entry-01ksa6
Open

Validate credentials on save#1106
simple-agent-manager[bot] wants to merge 6 commits into
mainfrom
sam/add-credential-validation-entry-01ksa6

Conversation

@simple-agent-manager
Copy link
Copy Markdown
Contributor

@simple-agent-manager simple-agent-manager Bot commented May 23, 2026

Summary

  • Validate cloud provider and agent credentials against upstream APIs during save, returning typed validation metadata in save responses.
  • Keep save as warning-mode persistence when upstream rejects a token or is unavailable, so users can still save with a clear inline warning.
  • Update onboarding/settings UI to show Testing..., green success feedback, and red warning feedback from the save response.

Validation

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • Additional validation run (if applicable): pnpm build; focused API/web credential tests; Playwright visual audit on 375x667 and 1280x800

Staging Verification (REQUIRED for all code changes — merge-blocking)

  • Staging deployment greenDeploy Staging workflow triggered manually and passed for this branch
  • Live app verified via Playwright — logged into app.sammy.party (staging) using test credentials and actively tested the application
  • Existing workflows confirmed working — navigated dashboard, projects, and settings; confirmed no regressions in core flows (pages load, data displays, navigation works, no new console errors)
  • New feature/fix verified on staging — the specific changes in this PR work correctly on the live staging environment (describe what was tested below)
  • Infrastructure verification completed — N/A: no infra changes
  • Mobile and desktop verification notes added for UI changes

Staging Verification Evidence

Deploy Staging passed: https://github.com/raphaeltm/simple-agent-manager/actions/runs/26331230963

Live Playwright/API verification passed against staging:

  • Navigated /dashboard, /projects, /settings/cloud-provider, and /settings/agent-keys with an authenticated staging browser session.
  • Observed zero browser console errors during navigation.
  • Saved a temporary invalid OpenAI Codex API key with autoActivate:false; deployed API returned 201 with validation.valid=false, provider status 401, and warning-mode persistence.
  • Deleted the temporary credential immediately after verification; cleanup returned 200.
  • Explicit validation endpoint returned 400 for the same invalid key.

UI Compliance Checklist (Required for UI changes)

  • Mobile-first layout verified
  • Accessibility checks completed
  • Shared UI components used or exception documented
  • Playwright visual audit run locally — mock data scenarios tested at mobile (375x667) and desktop (1280x800); no horizontal overflow; screenshots in .codex/tmp/playwright-screenshots/

End-to-End Verification (Required for multi-component changes)

  • Data flow traced from user input to final outcome with code path citations (see .claude/rules/10-e2e-verification.md)
  • Capability test exercises the complete happy path across system boundaries
  • All spec/doc assumptions about existing behavior verified against code (not just "read the code")
  • If any gap exists between automated test coverage and full E2E, manual verification steps documented below

Data Flow Trace

  • User saves from onboarding/settings UI: apps/web/src/components/onboarding/StepShared.tsx, apps/web/src/components/AgentKeyCard.tsx, apps/web/src/components/HetznerTokenForm.tsx, apps/web/src/components/ScalewayCredentialForm.tsx.
  • Web API response carries validation metadata through shared types: packages/shared/src/types/user.ts, packages/shared/src/agents.ts.
  • Save routes validate before encryption/persistence and return warning-mode metadata: apps/api/src/routes/credentials.ts.
  • Provider validation calls are centralized in apps/api/src/services/validation.ts.
  • Route/unit/UI tests cover provider calls, warning-mode save responses, explicit validation rejection, and success/warning feedback.

Untested Gaps

No gap for the implemented warning-mode save behavior: it is covered by unit/route tests, UI tests, and live staging API verification. Live staging did not use a real invalid Hetzner/Scaleway credential in the shared user account to avoid mutating existing cloud-provider configuration; provider-specific behavior is covered by mocked route/service tests.

Post-Mortem (Required for bug fix PRs)

What broke

Credential save flows accepted cloud provider tokens and agent API keys without a live upstream validation check, so users discovered invalid credentials later during provisioning or agent startup.

Root cause

Credential format validation and explicit validation existed in parts of the codebase, but the save path did not consistently return live provider validation feedback in warning mode.

Class of bug

Trust-boundary validation gap across UI/API persistence flow.

Why it wasn't caught

Existing tests did not assert that save responses included provider validation status or that UI surfaces displayed success/warning feedback from save.

Process fix included in this PR

Added route/service/UI tests that exercise the save-with-validation contract and UI feedback.

Post-mortem file

N/A: task archive records findings and verification in tasks/archive/2026-05-23-credential-validation-on-save.md.

Specialist Review Evidence (Required for agent-authored PRs)

  • All dispatched reviewers completed and findings addressed before merge
  • If any reviewer did NOT complete: needs-human-review label added and merge deferred to human — N/A: all reviews completed
Reviewer Status Outcome
task-completion-validator ADDRESSED Initial success-feedback UI test gap fixed in 1d86daff; acceptance criteria covered.
security-auditor PASS No credential leakage or auth bypass found; provider status logged without token material.
cloudflare-specialist PASS No D1/Worker config concerns; writes still occur after validation and warning mode avoids availability coupling.
ui-ux-specialist PASS Mobile/desktop Playwright audit passed; inline status/warning states verified.
test-engineer PASS Provider, route, vertical UI/API, and feedback tests present.
constitution-validator PASS Timeout configurable via existing env; provider API URLs are appropriate external integrations.

Exceptions (If any)

  • Scope: No separate postmortem doc.
  • Rationale: This is a focused UX/API trust-boundary improvement; the task archive includes the research and verification record.
  • Expiration: N/A

Agent Preflight (Required)

  • Preflight completed before code changes

Classification

  • external-api-change
  • cross-component-change
  • business-logic-change
  • public-surface-change
  • docs-sync-change
  • security-sensitive-change
  • ui-change
  • infra-change

External References

Official documentation consulted before coding:

Codebase Impact Analysis

Affected components:

  • apps/api: credential save routes and provider validation service.
  • apps/web: onboarding/settings credential feedback surfaces.
  • packages/shared: response and agent credential validation metadata types.
  • Tests: API provider/route tests and web component tests.

Documentation & Specs

  • Task moved to tasks/archive/2026-05-23-credential-validation-on-save.md with verification evidence.
  • N/A for user-facing docs: no documented workflow or env var behavior changed.

Constitution & Risk Check

Checked Principle XI / no hardcoded values. Provider URLs are external API contracts for the providers being validated; validation timeout is configurable via AGENT_CREDENTIAL_VALIDATION_TIMEOUT_MS. Main risk is upstream provider availability, mitigated by warning-mode persistence.


Do not merge from agent automation. This PR is ready for human review.

@sonarqubecloud
Copy link
Copy Markdown

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