Skip to content

feat: add e2e test suite for PDS#34

Merged
ascorbic merged 6 commits into
claude/deactivate-account-pattern-EDLhHfrom
claude/e2e-test-suite
Dec 31, 2025
Merged

feat: add e2e test suite for PDS#34
ascorbic merged 6 commits into
claude/deactivate-account-pattern-EDLhHfrom
claude/e2e-test-suite

Conversation

@ascorbic
Copy link
Copy Markdown
Owner

Summary

  • Add comprehensive e2e test suite using @atproto/api AtpAgent against live dev server
  • Tests cover CRUD operations, session auth, blob storage, firehose WebSocket, and CAR export
  • Uses static fixture directory copied to temp, with Vite dev server

Test Coverage (32 tests)

  • CRUD (11 tests): createRecord, getRecord, listRecords, deleteRecord, putRecord, applyWrites
  • Session (8 tests): login, getSession, refreshSession, describeServer
  • Blobs (5 tests): uploadBlob, getBlob, listBlobs
  • Firehose (4 tests): WebSocket connection, commit events, cursor backfill
  • Export (4 tests): getRepo CAR export, describeRepo

Test Plan

cd packages/pds
pnpm test:e2e

🤖 Generated with Claude Code

ascorbic and others added 5 commits December 31, 2025 18:45
- Switch from programmatic Vite to subprocess for better isolation
- Use test.local as handle (requires 2+ parts)
- Fix BlobRef CID access (use .ref.toString() for CID object)
- Skip getLatestCommit tests (endpoint not implemented)
- Update deleteRecord test to expect error

All 32 tests pass (2 skipped for unimplemented endpoint)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create e2e/fixture/ directory with static template files
- Copy fixture to temp directory instead of programmatic file creation
- Use npm run dev instead of npx vite
- Replace {{PDS_PACKAGE_PATH}} placeholder with actual path

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Dec 31, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
atproto-pds f99b612 Dec 31 2025, 10:57 PM

@ascorbic ascorbic changed the base branch from main to claude/deactivate-account-pattern-EDLhH December 31, 2025 22:58
@ascorbic ascorbic merged commit fd17bad into claude/deactivate-account-pattern-EDLhH Dec 31, 2025
3 of 4 checks passed
@ascorbic ascorbic deleted the claude/e2e-test-suite branch December 31, 2025 22:58
ascorbic added a commit that referenced this pull request Jan 1, 2026
* feat: implement deactivated account pattern for migration

Implements the deactivated account pattern to support account migration:

**Account State Management:**
- Add `active` column to `repo_state` table to track activation state
- Add `INITIAL_ACTIVE` env var to control initial account state
- Add RPC methods: `rpcGetActive()`, `rpcActivateAccount()`, `rpcDeactivateAccount()`

**Write Operation Guards:**
- Add `ensureActive()` check before all write operations
- Block `createRecord`, `putRecord`, `deleteRecord`, `applyWrites` when deactivated
- Return "AccountDeactivated" error with helpful message

**New Endpoints:**
- `POST /xrpc/com.atproto.server.activateAccount` - Enable writes and firehose
- `POST /xrpc/com.atproto.server.deactivateAccount` - Disable writes
- Enhanced `getAccountStatus` to return actual activation state

**Init Script Migration Flow:**
- Ask if migrating existing account vs creating new account
- For migration: prompt for current handle, resolve to DID, deploy deactivated
- For new account: generate DID, set up identity, deploy active
- Add handle resolver utility using DNS and HTTPS well-known methods
- Show migration steps (export, import, PLC update, activate)
- Show identity setup instructions for new accounts

**Testing:**
- All 169 tests pass
- Existing functionality preserved
- Deactivated accounts can read and import but not write

* fix: improve init wizard UX and handle resolution

- Use @atproto-labs/handle-resolver with Cloudflare DoH for reliable
  handle-to-DID resolution instead of custom implementation
- Fix retry flow when handle resolution fails: users can now choose
  to try a different handle or enter DID manually
- Default handle to hostname for new accounts (simpler setup)
- Show different identity setup notes based on whether handle matches
  hostname - when they match, explain that verification is automatic
- Add link to official AT Protocol migration docs in migration steps
- Add INITIAL_ACTIVE to VarName type for TypeScript compatibility

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: add migration progress tracking endpoints

Add endpoints and infrastructure for reliable account migration:

- listMissingBlobs: returns blobs referenced but not yet uploaded
- getBlocks: returns CAR file with specific blocks by CID
- Enhanced getAccountStatus: returns full migration metrics
  (repoBlocks, indexedRecords, expectedBlobs, importedBlobs)

Infrastructure:
- record_blob table tracks blob refs in records (populated on import)
- imported_blobs table tracks successful uploads
- extractBlobCids() helper for recursive blob detection

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: add migrate command and improve init/deploy workflow

- Add `pds migrate` command for migrating accounts from source PDS
  - Defaults to production, use --dev for local testing
  - Progress bar for blob transfers
  - Bright styling for notes using picocolors

- Improve init workflow
  - Ask for worker name (slugified handle as default)
  - Offer to push secrets to Cloudflare at end
  - Updated next steps with local testing instructions

- Fix CID serialization in record methods for imported repos
- Fix blob extraction to handle CID objects directly
- Add `pds-client.ts` for XRPC communication with PDS
- Change create-pds to ask for "Folder name" instead of "Project name"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* chore: add changeset for deactivated account pattern

* chore: add changesets for create-pds and oauth-provider

* Update create-pds-ux.md

* docs: add comprehensive README for oauth-provider package

* fix: address PR review comments

Address all review comments from PR #33:

1. Fix duplicate success message in init.ts (line 213-215)
   - Consolidated spinner.stop() and log.success() into single output

2. Fix hardcoded localhost:5173 in migrate.ts (line 445)
   - Changed to use ${targetDomain} variable

3. Fix variable shadowing in migrate.ts (totalBlobs declared twice)
   - Renamed outer variable to expectedBlobs to avoid shadowing

4. Implement missing 'pds activate' command
   - Created packages/pds/src/cli/commands/activate.ts
   - Added client methods activateAccount()/deactivateAccount()
   - Registered commands in CLI index

5. Implement missing 'pds deactivate' command
   - Created packages/pds/src/cli/commands/deactivate.ts
   - Both commands handle dev/production modes
   - Proper error handling and confirmation prompts

6. Add tests for activate/deactivate endpoints
   - Added 4 new tests in migration.test.ts
   - Tests cover auth requirements and state transitions
   - Verify writes are blocked when deactivated

7. Remove unused status variable in migration.test.ts (line 327)
   - Removed unused variable assignment

8. Fix test environment
   - Added INITIAL_ACTIVE: "true" to vitest.config.ts
   - Ensures tests start with active accounts by default

* fix: prevent test state leakage and improve import validation

- Add afterEach hook to reactivate account after each test
  This prevents deactivated state from leaking between tests

- Fix deactivate test assertion
  Changed from parsing JSON error to checking status code
  The error response is plain text, not JSON

- Improve importRepo validation logic
  Only reject imports if account is ACTIVE and repo exists
  Allow imports on deactivated accounts (for migration)
  This enables safe re-import workflow with --clean flag

All 181 tests now passing ✅

* refactor: address Copilot review comments

Critical fixes:
- Add retry limits to prevent infinite loops
  - Handle resolution: max 3 attempts with helpful error message
  - Password prompt: max 3 attempts before exit

- Extract duplicate helper functions to shared module
  - Created cli-helpers.ts with getTargetUrl() and getDomain()
  - Removed duplication from activate.ts, deactivate.ts, migrate.ts
  - Follows DRY principles

Minor improvements:
- Document that activated/active fields are intentional
  - Both used by different parts of the codebase
  - activated: used in migrate.ts and tests
  - active: more descriptive, returned by getActive()

Note: Other Copilot suggestions deferred:
- SQL ordering assumption is correct (CIDs are deterministic)
- 10s timeout is reasonable for DNS-over-HTTPS
- Error logging already exists for failed blob transfers
- Magic number "10" already removed in previous commit

* refactor: remove backwards compatibility for activated field and add resetMigration tests

- Remove duplicate 'activated' field from getAccountStatus endpoint, keep only 'active'
- Update migrate.ts to use 'active' field consistently
- Update all test assertions to use 'active' instead of 'activated'
- Add comprehensive test coverage for gg.mk.experimental.resetMigration endpoint:
  - Test authentication requirement
  - Test failure on active accounts (AccountActive error)
  - Test success on deactivated accounts with deletion count verification

All 24 migration tests passing.

* fix: correct type definition in migration test (activated -> active)

Address PR review comment: Type definition at line 406 was inconsistent
with actual API response field. Changed from 'activated: boolean' to
'active: boolean' to match the getAccountStatus endpoint response.

All 184 tests passing.

* fix: prevent state leakage from resetMigration tests

Move gg.mk.experimental.resetMigration describe block inside Account Migration
describe so it benefits from the afterEach hook that reactivates the account.
This prevents deactivated state from leaking to subsequent test files.

All 184 tests passing.

* fix: reset repoInitialized flag in resetMigration

When resetMigration() clears the repo, it must also reset the
repoInitialized flag. Otherwise, subsequent getRepo() calls skip
initialization and return null, causing test failures in firehose
tests that run after migration tests.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* feat: add e2e test suite for PDS (#34)

* wip

* fix: complete e2e test suite setup

- Switch from programmatic Vite to subprocess for better isolation
- Use test.local as handle (requires 2+ parts)
- Fix BlobRef CID access (use .ref.toString() for CID object)
- Skip getLatestCommit tests (endpoint not implemented)
- Update deleteRecord test to expect error

All 32 tests pass (2 skipped for unimplemented endpoint)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor: use static fixture for e2e tests

- Create e2e/fixture/ directory with static template files
- Copy fixture to temp directory instead of programmatic file creation
- Use npm run dev instead of npx vite
- Replace {{PDS_PACKAGE_PATH}} placeholder with actual path

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* chore: add e2e fixture .dev.vars (test credentials only)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* choire: run all tests

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* ci: add e2e test workflow

Runs e2e tests against live Vite dev server on PRs and main pushes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* attempt: disable inspector for e2e tests

Attempted fixes for InspectorProxyController.updateConnection error:
- Set miniflare.inspector: false in vite.config.ts
- Set dev.inspector_port: 0 in wrangler.jsonc
- Set NO_INSPECTOR=1 environment variable

However, the error persists. The InspectorProxyController is still attempting
to make fetch calls that fail with 'TypeError: fetch failed'. This appears
to be an environment-specific issue with the Cloudflare Vite plugin's
inspector proxy trying to connect in CI/test environments.

Issue: https://github.com/ascorbic/atproto-worker/actions/runs/20628674621

* revert: remove inspector disable attempts (sandbox-specific issue)

The InspectorProxyController.updateConnection fetch error is caused by this
sandbox's proxy environment, not an actual issue with the code or CI.

The proxy intercepts localhost connections that the inspector needs, causing
fetch to fail. This doesn't occur on Mac (no proxy) or in GitHub CI.

Reverting inspector disable configurations as they don't address any real
issue and add unnecessary complexity.

* feat: add detailed logging to e2e test setup

Add comprehensive logging to help diagnose CI failures:
- Environment information (Node version, platform, temp dir)
- Timing information for each step
- npm install stdout/stderr output (even on success)
- Incremental Vite server output as it arrives
- Package.json path replacement details
- Total setup time tracking

This will help identify where the e2e setup is failing in GitHub CI.

* fix: simplify Vite port detection regex

The previous regex pattern didn't match because Vite's output includes
ANSI escape codes (colors, bold, unicode arrows) that break pattern matching.

Changed from: /Local:\s+http:\/\/localhost:(\d+)/
Changed to:   /localhost:(\d+)/

This simpler pattern will match regardless of ANSI formatting or unicode
characters in the output.

* fix: strip ANSI codes before matching Vite port

Vite's output includes ANSI escape codes for formatting (bold, colors)
even within the port number itself. Strip these codes before matching
to ensure the regex can find the port number.

* refactor: clean up e2e setup logging

Remove verbose logging now that e2e tests are working:
- Removed detailed environment info, timing, and incremental output
- Keep essential messages: dependency install and server start
- Preserve full output in error messages for debugging

Maintains ANSI escape code stripping for reliable port detection.

* fix: address knip unused code warnings

- Ignore e2e/fixture directory (used at runtime by e2e tests)
- Add vite and @cloudflare/vite-plugin to ignoreDependencies (used in e2e fixture)
- Remove unused TEST_AUTH_TOKEN export
- Remove ws/@types/ws from ignoreDependencies (no longer flagged)

* chore: don't run e2e in main test file

* refactor: use @atproto helpers for record serialization and blob extraction

Replace custom type-checking logic with official @atproto/lex-data helpers:
- Use `asCid()` instead of duck-typing CID detection in serializeRecord
- Use `isBlobRef()` instead of manual $type checking in extractBlobCids

This makes the code more maintainable and relies on official implementations.

* fix: changes from review

---------

Co-authored-by: Claude <noreply@anthropic.com>
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