Skip to content

feat: Add Conductor port reservation system for Google OAuth#14

Merged
bokelley merged 1 commit into
mainfrom
feature/conductor-port-reservation
Aug 1, 2025
Merged

feat: Add Conductor port reservation system for Google OAuth#14
bokelley merged 1 commit into
mainfrom
feature/conductor-port-reservation

Conversation

@bokelley
Copy link
Copy Markdown
Collaborator

@bokelley bokelley commented Aug 1, 2025

Summary

  • Adds a port reservation system to eliminate the need to constantly update Google OAuth redirect URIs for new Conductor workspaces
  • Implements environment variable validation to ensure proper configuration
  • Provides automatic port management with reservation and release capabilities

Problem

When creating new Conductor workspaces, each workspace gets randomly generated ports. This requires adding new redirect URIs to Google OAuth configuration every time, which is tedious and error-prone.

Solution

This PR introduces a port reservation system that:

  1. Uses a predefined pool of 10 port sets (Admin UI ports 8002-8011)
  2. Automatically reserves ports when creating workspaces
  3. Automatically releases ports when cleaning up workspaces
  4. Falls back to hash-based ports if the reservation system fails

Key Features

Port Reservation System

  • 10 predefined port sets that can be registered once with Google OAuth
  • File locking to prevent conflicts between concurrent operations
  • Automatic management - ports are reserved on setup and released on cleanup
  • Easy monitoring with python3 manage_conductor_ports.py list

Environment Variable Validation

  • Required variables checked:
    • SUPER_ADMIN_EMAILS - for Admin UI access
    • GEMINI_API_KEY - for creative generation
    • GOOGLE_CLIENT_ID/SECRET - for OAuth login
  • Helpful guidance provided for missing variables
  • Interactive prompt to continue despite missing variables

Files Added

  • manage_conductor_ports.py - Port reservation manager with file locking
  • conductor_ports.json - Configuration with 10 predefined port sets
  • cleanup_conductor_workspace.sh - Cleanup script that releases ports
  • CONDUCTOR_PORTS.md - Complete documentation

Setup Instructions

  1. Run python3 manage_conductor_ports.py oauth-urls to get all redirect URLs
  2. Add these 10 URLs to your Google OAuth app (ports 8002-8011)
  3. Set required environment variables in your shell
  4. Create workspaces - they'll automatically use reserved ports

Test plan

  • Create a new Conductor workspace and verify port reservation
  • Check that environment variables are validated
  • Verify cleanup script releases ports properly
  • Test concurrent workspace creation (file locking)
  • Verify fallback to hash-based ports works

🤖 Generated with Claude Code

This adds a port reservation system to solve the Google OAuth redirect URI problem
for Conductor workspaces. Instead of generating random ports that need to be
registered with Google OAuth each time, we now use a predefined pool of 10 port
sets that can be pre-registered once.

Key features:
- Predefined pool of 10 port sets (Admin UI ports 8002-8011)
- Automatic port reservation when creating workspaces
- Automatic port release when cleaning up workspaces
- File locking to prevent conflicts between concurrent operations
- Fallback to hash-based ports if reservation system fails

Also adds environment variable validation:
- Checks for required SUPER_ADMIN_EMAILS (for Admin UI access)
- Validates GEMINI_API_KEY (for creative generation)
- Ensures Google OAuth credentials are configured
- Provides helpful guidance for missing variables
- Interactive prompt to continue despite missing variables

Files added:
- manage_conductor_ports.py: Port reservation manager with file locking
- conductor_ports.json: Configuration with 10 predefined port sets
- cleanup_conductor_workspace.sh: Cleanup script that releases ports
- CONDUCTOR_PORTS.md: Complete documentation

This eliminates the need to constantly update Google OAuth redirect URIs
when creating new Conductor workspaces.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley merged commit fb1e8f9 into main Aug 1, 2025
3 checks passed
@bokelley bokelley deleted the feature/conductor-port-reservation branch August 1, 2025 11:34
bokelley added a commit that referenced this pull request Sep 15, 2025
…reservation

feat: Add Conductor port reservation system for Google OAuth
KonstantinMirin added a commit to KonstantinMirin/prebid-salesagent that referenced this pull request Mar 31, 2026
…-al1n]

- then_excludes_delivery_data/then_no_delivery_data: fail on None resp
  instead of silently passing (findings #1, #2)
- then_has_metrics: add isinstance numeric check for clicks to match
  impressions/spend check strength (finding #3)
- then_has_packages: validate package content (package_id, totals with
  numeric impressions) not just non-empty list (finding prebid#4)
- then_has_reporting_period: parse start/end as dates and verify
  end >= start, not just not-None (finding prebid#5)
- then_has_aggregated_totals: verify specific impressions/spend metrics
  with numeric type checks instead of generic non-zero scan (finding prebid#6)
- then_no_error_for_mb/alt: extract _assert_no_error_for_mb shared
  helper (DRY), add response-level errors list check (findings prebid#7, prebid#8)
- then_skip_no_webhook: check nested media_buy_deliveries in payload
  not just top-level media_buy_id (finding prebid#14)
KonstantinMirin added a commit to KonstantinMirin/prebid-salesagent that referenced this pull request Mar 31, 2026
Address all 39 findings from inspect_bdd_steps.py across 6 step files:

uc002_create_media_buy.py (10 findings):
- Assert packages exist instead of silent if-guard skips (#1, prebid#4, prebid#6, prebid#7)
- Strengthen success path with media_buy_id verification (#2)
- Add DB verification after seller rejection (#3)
- Re-commit creative.data mutation to DB (prebid#5)
- Document proposal SPEC-PRODUCTION GAP with implementation notes (prebid#8, prebid#9, prebid#10)

uc003_update_media_buy.py (4 findings):
- Add max_daily_package_spend comparison when tenant config available (prebid#12)
- Assert tenant exists for creative status validation (prebid#13)
- Validate placement_ids non-empty and against product config (prebid#14)

uc019_query_media_buys.py (5 findings):
- Verify principal_id consistency in Given steps (prebid#18, prebid#20)
- Validate date parsing in given_today_is (prebid#19)
- Strengthen adapter/snapshot setup documentation (prebid#21, prebid#22)

uc026_package_media_buy.py (2 findings):
- Verify pricing_options content matches step parameter (prebid#27)
- Verify format_ids content matches step parameter (prebid#28)

then_error.py (2 findings):
- Fix guard logic: allow Exception subclasses with .recovery (prebid#34)
- Use _get_error_dict for consistent field extraction (prebid#35)

then_media_buy.py (2 findings):
- Explicit error path in status check (no silent fallthrough) (prebid#36)
- Clarify seller event type assertions (prebid#37)

Remaining 14 findings use correct SPEC-PRODUCTION GAP xfail pattern
(try assertion first, xfail only on known gap with descriptive message).
ChrisHuie added a commit that referenced this pull request Apr 19, 2026
…uards (L0-01)

Per .claude/notes/flask-to-fastapi/L0-implementation-plan-v2.md §L0-01,
landing rows #1-#16 of the §5.5 Structural Guards Inventory plus guard #34
(no_werkzeug_imports, reassigned to L0-01 per v2 plan §7.1 RATIFIED).

Guards (each with AST scanner + main test + planted-violation meta-fixture):
- test_architecture_no_flask_imports.py                (#1, Captured→shrink)
- test_architecture_handlers_use_sync_def.py           (#2, frozen carve-out)
- test_architecture_no_async_db_access.py              (#3)
- test_architecture_middleware_order.py                (#4, DORMANT)
- test_architecture_exception_handlers_complete.py     (#5, DORMANT)
- test_architecture_csrf_exempt_covers_adcp.py         (#6, DORMANT)
- test_architecture_approximated_middleware_path_gated.py (#7, DORMANT)
- test_architecture_admin_routes_excluded_from_openapi.py (#8, DORMANT)
- test_architecture_admin_routes_named.py              (#9, DORMANT)
- test_architecture_admin_route_names_unique.py        (#10, DORMANT)
- test_architecture_no_module_scope_create_app.py      (#11, Captured→shrink)
- test_architecture_scheduler_lifespan_composition.py  (#12)
- test_architecture_a2a_routes_grafted.py              (#13)
- test_architecture_form_getlist_parity.py             (#14, Captured→shrink)
- test_architecture_no_module_level_engine.py          (#15, Captured→shrink)
- test_architecture_no_direct_env_access.py            (#16, Captured→shrink)
- test_architecture_no_werkzeug_imports.py             (#34, Captured→shrink)

Shared helpers in tests/unit/architecture/_ast_helpers.py (walk_py_files,
read_allowlist, relpath) per the DRY invariant in CLAUDE.md.

Captured→shrink baselines seeded:
- allowlists/no_flask_imports.txt           (40 files importing flask)
- allowlists/no_werkzeug_imports.txt        (6 files importing werkzeug)
- allowlists/module_scope_create_app.txt    (7 tests/*.py files)
- allowlists/no_form_getlist.txt            (3 admin routers)
- allowlists/no_module_level_engine.txt     (2 legacy gam services)
- allowlists/no_direct_env_access.txt       (121 path:line call sites)

Meta-guard `test_structural_guard_allowlist_monotonic.py` (§5.5 row #28)
enforcing allowlist monotonicity will land in a later L0 work item.

DORMANT guards (scanner present; repo state trivially compliant until
L0-04..L0-18 foundation modules land): #4, #5, #6, #7, #8, #9, #10. Each has
a planted-violation fixture that DOES fire the scanner; dormancy is about
the repo state, not scanner correctness.

Guard #15 landed as a new guard (the sibling test_architecture_no_module_level_get_engine.py
referenced in the plan does not exist in the pre-L0 state, so #15 is
not redundant with it).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ChrisHuie added a commit that referenced this pull request May 23, 2026
Addresses three related items from the 2026-05-20 structural follow-up
review on PR #1306:

- #14: `_handle_tool_error` was defined in api_v1.py but called only
  from app.py via lazy import. Moved (renamed to public
  `handle_tool_error`) plus its helpers (`_build_error_code_to_status`,
  `_ERROR_CODE_TO_STATUS`) to src/core/tool_error_logging.py — the
  module that already hosts AdCPToolError and extract_error_info.
  Underscore prefix dropped per Konstantine's ask.

- #25: app.py now imports `handle_tool_error` eagerly at the top
  instead of the lazy `from src.routes.api_v1 import _handle_tool_error`
  inside tool_error_handler. No circular-import risk after the move
  (tool_error_logging has no transitive dependency on app).

- #10: REST module (api_v1.py) no longer imports AdCPToolError. The
  class stays in tool_error_logging.py (alongside the boundary translator
  functions that build it). Konstantine's literal suggestion was to move
  AdCPToolError to src/core/exceptions.py; this commit takes the
  alternative he listed (REST no longer imports the MCP-boundary type)
  because adding fastmcp to exceptions.py would couple the neutral
  exceptions module to MCP.

Cleanup in api_v1.py:
- Dropped imports of JSONResponse, ToolError, AdCPError, AdCPToolError,
  extract_error_info, build_two_layer_error_envelope. All moved with
  the function or no longer used after the move.

Test updates:
- tests/unit/test_error_boundary_translation.py: 10 lazy imports of
  `_handle_tool_error` updated to direct imports of `handle_tool_error`
  from tool_error_logging. Docstring references updated. Adjacent
  AdCPToolError + handle_tool_error imports merged onto one line.

Local quality: 4682 passed, 1 skipped, 20 xfailed.

Co-Authored-By: Claude Opus 4.7 (1M context) <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