Skip to content

fix: return HTTP 403 with AccountDeactivated error for write operations on deactivated accounts#35

Merged
ascorbic merged 2 commits into
mainfrom
copilot/sub-pr-33
Jan 1, 2026
Merged

fix: return HTTP 403 with AccountDeactivated error for write operations on deactivated accounts#35
ascorbic merged 2 commits into
mainfrom
copilot/sub-pr-33

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 1, 2026

Plan: Address AccountDeactivated Error Handling

  • Understand the feedback and current implementation
  • Update write operation handlers to catch AccountDeactivated errors
    • Update createRecord in repo.ts
    • Update deleteRecord in repo.ts
    • Update putRecord in repo.ts
    • Update applyWrites in repo.ts
  • Return HTTP 403 with error type "AccountDeactivated" for these errors
  • Update test expectations from 500 to 403
  • Address code review feedback
    • Use startsWith() instead of includes() for more precise error detection
    • Fix indentation consistency in test file
    • Add JSDoc documentation for error checking function
  • Merge main branch (includes PR feat: implement deactivated account pattern for migration #33 and subsequent updates)
  • Complete implementation

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Jan 1, 2026

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 6eec27a Jan 01 2026, 06:04 PM

@ascorbic ascorbic marked this pull request as ready for review January 1, 2026 09:05
Base automatically changed from claude/deactivate-account-pattern-EDLhH to main January 1, 2026 09:06
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 1, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • pkg.pr.new
    • Triggering command: /usr/local/bin/node node /home/REDACTED/.local/share/pnpm/.tools/pnpm/10.26.2/bin/pnpm install (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] WIP on feedback for deactivated account migration Return HTTP 403 with AccountDeactivated error for write operations on deactivated accounts Jan 1, 2026
Copilot AI requested a review from ascorbic January 1, 2026 09:08
@ascorbic
Copy link
Copy Markdown
Owner

ascorbic commented Jan 1, 2026

@copilot rebase this

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 1, 2026

@copilot rebase this

I've successfully rebased the branch onto the latest main (which includes the merged PR #33). The 4 commits from this PR are now based on top of main at commit 46a0842.

However, I'm unable to force push the rebased history to the remote branch due to authentication constraints. The rebase is complete locally with the following rebased commits:

3f62181 docs: add JSDoc for checkAccountDeactivatedError function
c7c430a refactor: use startsWith for more precise error detection and fix indentation  
d20f023 fix: return HTTP 403 AccountDeactivated error for blocked writes
6334e9b Initial plan

The branch needs a force push to update the remote. Could you please force push the local branch, or would you like me to try an alternative approach?

@ascorbic
Copy link
Copy Markdown
Owner

ascorbic commented Jan 1, 2026

@copilot do a merge instead of rebase then so there's no need for a force push

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 1, 2026

@copilot do a merge instead of rebase then so there's no need for a force push

Merge completed successfully in commit b8a745b. The branch now includes all changes from main (including PR #33, PR #39 for preferences migration, and the latest releases) while preserving the AccountDeactivated error handling improvements (HTTP 403 instead of 500).

The merge resolved conflicts in:

  • packages/pds/test/migration.test.ts - Kept the 403 error handling test
  • packages/pds/src/cli/commands/migrate.ts - Merged with new preferences migration step
  • packages/pds/src/cli/utils/pds-client.ts - Merged with new preferences methods

@ascorbic ascorbic changed the title Return HTTP 403 with AccountDeactivated error for write operations on deactivated accounts fix: return HTTP 403 with AccountDeactivated error for write operations on deactivated accounts Jan 1, 2026
When write operations (createRecord, putRecord, deleteRecord, applyWrites)
are attempted on a deactivated account, return a proper HTTP 403 with
error type "AccountDeactivated" instead of a generic 500 error.

This gives clients clear feedback that the account needs to be activated.

Also removes redundant step number comments from migrate.ts.

🤖 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>
@ascorbic ascorbic enabled auto-merge (squash) January 1, 2026 18:04
@ascorbic ascorbic merged commit 735981d into main Jan 1, 2026
5 checks passed
@ascorbic ascorbic deleted the copilot/sub-pr-33 branch January 1, 2026 18:05
@mixie-bot mixie-bot Bot mentioned this pull request Jan 1, 2026
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.

2 participants