Skip to content

Fix/supabase key validation and state consolidation#238

Closed
dustywill wants to merge 2 commits into
coleam00:mainfrom
dustywill:fix/supabase-key-validation-and-state-consolidation
Closed

Fix/supabase key validation and state consolidation#238
dustywill wants to merge 2 commits into
coleam00:mainfrom
dustywill:fix/supabase-key-validation-and-state-consolidation

Conversation

@dustywill

Copy link
Copy Markdown

Pull Request

Summary

Changes Made

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

# Example: python -m pytest tests/
# Example: cd archon-ui-main && npm run test

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

Additional Notes

Wirasm added 2 commits August 16, 2025 00:10
- Add backend validation to detect and warn about anon vs service keys
- Prevent startup with incorrect Supabase key configuration
- Consolidate frontend state management following KISS principles
- Remove duplicate state tracking and sessionStorage polling
- Add clear error display when backend fails to start
- Improve .env.example documentation with detailed key selection guide
- Add comprehensive test coverage for validation logic
- Remove unused test results checking to eliminate 404 errors

The implementation now warns users about key misconfiguration while
maintaining backward compatibility. Frontend state is simplified with
MainLayout as the single source of truth for backend status.
- Use python-jose (already in dependencies) instead of PyJWT for JWT decoding
- Make unknown Supabase key roles fail fast per alpha principles
- Skip all JWT validations (not just signature) when checking role
- Update tests to expect failure for unknown roles

Fixes:
- No need to add PyJWT dependency - python-jose provides JWT functionality
- Unknown key types now raise ConfigurationError instead of warning
- JWT decode properly skips all validations to only check role claim
@dustywill dustywill closed this Aug 16, 2025
@dustywill dustywill deleted the fix/supabase-key-validation-and-state-consolidation branch August 16, 2025 14:31
POWERFULMOVES added a commit to POWERFULMOVES/PMOVES-Archon that referenced this pull request Feb 12, 2026
…logs-update-helper

docs: refresh service update notes
coleam00 pushed a commit that referenced this pull request Apr 7, 2026
* Fix: PR worktrees use actual branch for same-repo PRs (#232)

When creating a worktree for a PR review, the system was always creating
a synthetic branch (`pr-N-review`) instead of using the PR's actual branch.
This caused confusion when pushing changes back to the PR, as commits went
to an orphan branch instead of updating the PR.

Changes:
- Add `isForkPR` field to IsolationHints and IsolationRequest
- Add fork detection in GitHub adapter (compares head.repo vs base.repo)
- Update createFromPR() to use actual branch for same-repo PRs
- Fork PRs still use synthetic branch (can't push to forks anyway)
- Update generateBranchName() to return actual PR branch for same-repo PRs
- Add tests for same-repo vs fork PR behavior

Fixes #232

* docs: Update PR worktree behavior documentation for fork vs same-repo PRs

* Address PR review findings: error handling and test coverage

1. **Error handling improvements**:
   - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection
   - Distinguish transient vs non-transient failures in PR fetch error handling
   - Make upstream tracking failure non-fatal (worktree still usable)

2. **Documentation**:
   - Add comment clarifying deleted fork edge case (null head.repo)

3. **Test coverage**:
   - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity)
   - Add orchestrator integration tests for isForkPR pass-through

Addresses review feedback from PR #238.

* Simplify error handling in PR fetch logic

- Remove code duplication in logging branches
- Use dynamic function selection for log level
- Remove redundant boolean fields from log output
- Consolidate comments
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
* Fix: PR worktrees use actual branch for same-repo PRs (coleam00#232)

When creating a worktree for a PR review, the system was always creating
a synthetic branch (`pr-N-review`) instead of using the PR's actual branch.
This caused confusion when pushing changes back to the PR, as commits went
to an orphan branch instead of updating the PR.

Changes:
- Add `isForkPR` field to IsolationHints and IsolationRequest
- Add fork detection in GitHub adapter (compares head.repo vs base.repo)
- Update createFromPR() to use actual branch for same-repo PRs
- Fork PRs still use synthetic branch (can't push to forks anyway)
- Update generateBranchName() to return actual PR branch for same-repo PRs
- Add tests for same-repo vs fork PR behavior

Fixes coleam00#232

* docs: Update PR worktree behavior documentation for fork vs same-repo PRs

* Address PR review findings: error handling and test coverage

1. **Error handling improvements**:
   - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection
   - Distinguish transient vs non-transient failures in PR fetch error handling
   - Make upstream tracking failure non-fatal (worktree still usable)

2. **Documentation**:
   - Add comment clarifying deleted fork edge case (null head.repo)

3. **Test coverage**:
   - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity)
   - Add orchestrator integration tests for isForkPR pass-through

Addresses review feedback from PR coleam00#238.

* Simplify error handling in PR fetch logic

- Remove code duplication in logging branches
- Use dynamic function selection for log level
- Remove redundant boolean fields from log output
- Consolidate comments
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
* Fix: PR worktrees use actual branch for same-repo PRs (coleam00#232)

When creating a worktree for a PR review, the system was always creating
a synthetic branch (`pr-N-review`) instead of using the PR's actual branch.
This caused confusion when pushing changes back to the PR, as commits went
to an orphan branch instead of updating the PR.

Changes:
- Add `isForkPR` field to IsolationHints and IsolationRequest
- Add fork detection in GitHub adapter (compares head.repo vs base.repo)
- Update createFromPR() to use actual branch for same-repo PRs
- Fork PRs still use synthetic branch (can't push to forks anyway)
- Update generateBranchName() to return actual PR branch for same-repo PRs
- Add tests for same-repo vs fork PR behavior

Fixes coleam00#232

* docs: Update PR worktree behavior documentation for fork vs same-repo PRs

* Address PR review findings: error handling and test coverage

1. **Error handling improvements**:
   - Add `prFetchFailed` flag to `IsolationHints` for degraded mode detection
   - Distinguish transient vs non-transient failures in PR fetch error handling
   - Make upstream tracking failure non-fatal (worktree still usable)

2. **Documentation**:
   - Add comment clarifying deleted fork edge case (null head.repo)

3. **Test coverage**:
   - Add fork detection unit tests (same-repo, fork, deleted fork, case sensitivity)
   - Add orchestrator integration tests for isForkPR pass-through

Addresses review feedback from PR coleam00#238.

* Simplify error handling in PR fetch logic

- Remove code duplication in logging branches
- Use dynamic function selection for log level
- Remove redundant boolean fields from log output
- Consolidate comments
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