Skip to content

M4 Completion: Critical bug fixes + Phase 5-6 handlers#7

Merged
aidankhogg merged 6 commits into
mainfrom
claude/m4-completion-1782083665
Jun 21, 2026
Merged

M4 Completion: Critical bug fixes + Phase 5-6 handlers#7
aidankhogg merged 6 commits into
mainfrom
claude/m4-completion-1782083665

Conversation

@aidankhogg

@aidankhogg aidankhogg commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR completes M4 implementation (Supabase + pgmq event queue + operator API) by fixing 6 critical P0 bugs and 3 medium P1 functional gaps that were blocking Phase 5-6 handler execution.

P0 Critical Fixes (Blocking Execution)

  1. pgmq_client.py - EventEnvelope serialization bug

    • Issue: Used event.dict() on dataclass instead of Pydantic model
    • Fix: Changed to event.to_dict() method (2 occurrences, lines 14, 45)
    • Added retry_count initialization check in archive_to_dlq
    • Impact: All pgmq message sends would crash with AttributeError without this

  2. app_handler.py - Missing datetime import

    • Issue: Line 67 used datetime.utcnow() without importing datetime
    • Fix: Added from datetime import datetime import
    • Impact: App deployment would crash with NameError

  3. phase_registries.py - Missing abstract methods

    • Issue: Did not implement required healthcheck() and should_skip() methods
    • Fix: Added both methods to comply with BasePhaseHandler interface
    • Impact: Orchestrator would fail with AttributeError when executing Phase 5

  4. phase_inworld_identity.py - Missing abstract methods

    • Issue: Did not implement required healthcheck() and should_skip() methods
    • Fix: Added both methods with proper SSL context handling for self-signed Keycloak certs
    • Impact: Orchestrator would fail with AttributeError when executing Phase 6

  5. run_migrations.py - Non-existent Supabase endpoint

    • Issue: Called /api/rest/v1/rpc/exec_sql endpoint that doesn't exist
    • Fix: Complete rewrite to use subprocess + psql command-line tool
    • Now supports all PostgreSQL connection parameters via environment variables
    • Impact: Database migrations were silently failing or returning 404 errors

  6. app.py - Empty Keycloak credentials

    • Issue: Hardcoded empty password in Keycloak BasicAuth (line 41)
    • Fix: Read admin_password from RuntimeState or KEYCLOAK_ADMIN_PASSWORD env var
    • Added proper 500 error if credentials not configured
    • Impact: OIDC token validation would always fail

P1 Medium Fixes (Functional Gaps)

  1. app_handler.py - Missing gateway IP derivation logic

    • Issue: Tried to read non-existent gateway_ip field from address_leases table
    • Fix: Refactored _get_gateway_ip() to calculate from CIDR using ipaddress module
    • Gateway IP = first usable IP in subnet (network.network_address + 1)
    • Impact: App deployment to ANDs would fail with KeyError

  2. 001_initial.sql - pgmq_pop() return type

    • Issue: pgmq.pop() returns record but function tried to return raw JSONB
    • Fix: Properly wrapped return value with json_build_object() for field serialization
    • Impact: DLQ queue functionality was broken

  3. 001_initial.sql - Missing app_deployments table

    • Issue: app_handler.py references non-existent table (line 70-71)
    • Fix: Added complete table schema with columns: id, org, app, domain, container_id, client_id, deployed_at
    • Impact: App deployment state tracking would fail with "relation does not exist" error

New Tests

  • tests/integration/test_m4_phases.py - Comprehensive Phase 5-6 handler contract tests
    • Phase 5 (RegistriesPhaseHandler): 4 test methods
    • Phase 6 (InWorldIdentityPhaseHandler): 4 test methods
    • Validates interface compliance, skip logic, and healthcheck behavior

Test Results

✅ All 99 tests passing (2 skipped)

  • No regressions from any fixes
  • Code formatted with black, isort, flake8

Impact

These fixes unblock:

  • Phase 5 (Registries): World + Domain Registry provisioning
  • Phase 6 (In-world Identity): Keycloak L2 realm setup
  • Event queue operations (pgmq message handling)
  • App deployment flow (complete 4-step deployment pipeline)
  • Database migrations (proper PostgreSQL connectivity)
  • Operator API authentication (OIDC token validation)

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2

Generated by Claude Code

claude added 6 commits June 21, 2026 22:59
@claude
Fix M4 critical bugs: 4 missing imports + SQL function
dbe9d08 
- phase_platform_identity.py: Add PKIHandler import (line 35 usage)
- phase_registries.py: Add json import (line 67 json.loads usage)
- api/app.py: Add DNSHandler import (line 158 usage)
- handlers/and_handler.py: Add ipaddress import (line 33 usage)
- migrations/001_initial.sql: Add pgmq_delete() RPC function for message acknowledgment

All 69 existing tests pass. M4 handlers now compile and are ready for M3 integration.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2
@claude
M3 Step 1: Orchestrator integration + phase handler completion
90bc029 
- Rewrote Orchestrator class to use proper BasePhaseHandler registry pattern
  - Phase handlers (0-8) now properly registered in execution sequence
  - execute_phases() implements loop with dependency checking, error handling, state persistence
  - Uses healthcheck() and should_skip() from all handlers
  - Proper PhaseContext DI for all handlers

- Completed Phase 3 and Phase 4 handlers with required abstract methods
  - PKIPhaseHandler: Added should_skip() to check phase_completed[3]
  - PlatformIdentityPhaseHandler: Added healthcheck() and should_skip()
    - healthcheck() verifies Keycloak running + OIDC discovery endpoint responds

- Fixed Phase 7 handler import: BasePhaseHandler from _base, not handlers package

Orchestrator is now ready to execute full Phase 0-8 bootstrap sequence.
All 69 existing tests still passing.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2
@claude
M3 Steps 2-4: Phase handler fixes + integration tests
cd864ed 
- Fixed Phase 4 handler RuntimeState attribute access
  - Changed context.runtime_state.get() to getattr() since RuntimeState is a dataclass
  - Fixed bootstrap_admin_password and keycloak_platform_container_id access patterns

- Improved Phase 4 healthcheck method
  - Added proper async/await handling for aiohttp ClientSession
  - Added SSL context configuration to allow self-signed certs
  - Added proper timeout and exception handling for network errors
  - Returns False gracefully if container not running or not found

- Fixed Phase 3 event emission
  - Changed correlation_id and parent_event_id to use getattr() with defaults
  - Prevents AttributeError if these fields don't exist on RuntimeState

- Added comprehensive M3 integration tests (13 test methods)
  - TestPKIPhaseHandlerContract: Verify Phase 3 implements BasePhaseHandler interface
  - TestPlatformIdentityPhaseHandlerContract: Verify Phase 4 implements interface
  - TestM3OrchestratorIntegration: Verify handlers registered correctly in sequence
  - All tests passing, validates handler contracts and phase ordering

Test results: 82 passed, 2 skipped (M3+ placeholder tests)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2
@claude
M3 Step 5: Orchestrator behavior tests
6bf735b 
Added comprehensive orchestrator tests validating:

- Phase execution control
  - Skip already-completed phases
  - Respect up_to_phase limit (don't execute beyond it)
  - Mark phases complete after successful execution
  - Raise error on healthcheck failure
  - Persist error state to RuntimeState on failure

- Phase sequencing and ordering
  - Phases ordered sequentially 0-8
  - All 9 phases have handlers
  - Phase 3 executes before Phase 4

Test results: 90 passed, 2 skipped
- M0-M2 tests: 69 passing (spec, substrate, DNS)
- M3 tests: 21 new tests (13 handler contract + 8 orchestrator behavior)
- 2 skipped placeholder tests (Phase 4-5 scaffolds)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2
@claude
fix: Code formatting and import ordering (black, isort, flake8)
81c0d80
@claude
M4 Completion: Fix critical bugs + implement Phase 5-6 handlers
b15df65 
P0 Critical Fixes (blocking execution):
- pgmq_client.py: Fixed EventEnvelope serialization (event.dict() → event.to_dict())
  Added retry_count initialization check in archive_to_dlq method
- app_handler.py: Added missing datetime import for deployment timestamp generation
- phase_registries.py: Implemented healthcheck() and should_skip() abstract methods
- phase_inworld_identity.py: Implemented healthcheck() and should_skip() abstract methods
  Proper SSL context handling for self-signed Keycloak certificates
- run_migrations.py: Complete rewrite to use subprocess + psql instead of non-existent
  Supabase REST endpoint, supports all database connection parameters
- app.py: Fixed Keycloak admin credentials to read from environment/RuntimeState instead
  of using empty password, with proper 500 error handling

P1 Medium Fixes (functional gaps):
- app_handler.py: Refactored gateway_ip derivation to calculate from CIDR instead of
  reading non-existent database field using ipaddress module
- 001_initial.sql: Fixed pgmq_pop() return type wrapper with json_build_object()
  to properly serialize pgmq record returns
- 001_initial.sql: Added app_deployments table for tracking deployed applications
  with schema: id, org, app, domain, container_id, client_id, deployed_at

New Tests:
- tests/integration/test_m4_phases.py: Comprehensive Phase 5-6 handler contract tests
  covering interface compliance, skip logic, and healthcheck behavior (8 test methods)

Test Results:
- All 99 tests passing (2 skipped)
- No regressions from any fixes
- Code formatting with black, isort, flake8 validated

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8FoC1a2wcbGVTE3jMwRg2
@aidankhogg aidankhogg self-assigned this Jun 21, 2026
@aidankhogg aidankhogg added bug Something isn't working enhancement New feature or request labels Jun 21, 2026
@aidankhogg aidankhogg merged commit 48593e0 into main Jun 21, 2026
6 checks passed
@aidankhogg aidankhogg deleted the claude/m4-completion-1782083665 branch June 21, 2026 23:19
@aidankhogg aidankhogg mentioned this pull request Jun 21, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@aidankhogg aidankhogg

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aidankhogg @claude