Open
Conversation
toderian
force-pushed
the
refactor-car-ports
branch
2 times, most recently
from
913d2ec to
7e4fb69
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Deeploy’s plugin preparation/semaphore wiring and the Container App Runner (CAR) port/tunnel configuration around a normalized EXPOSED_PORTS model, while also introducing background persistence for deployed pipeline metadata to improve create/update request latency.
Changes:
- Add extensive unit-test baselines for Deeploy request preparation, shmem/semaphore resolution, and CAR exposed-ports/tunnel/health behavior.
- Refactor CAR runtime port allocation + tunnel command construction to use normalized
EXPOSED_PORTS(including per-portprotocol) and export additional semaphore networking keys. - Refactor Deeploy pipeline create/update flows to (a) resolve shmem references via
plugin_name-based semaphore keys and (b) persist pipeline metadata asynchronously via a background worker.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| extensions/business/deeploy/tests/test_update_requests.py | New tests for update request preparation and config field stripping/preservation (incl. EXPOSED_PORTS, DYNAMIC_ENV). |
| extensions/business/deeploy/tests/test_semaphore_wiring.py | New tests validating native↔container semaphore autowiring behavior and skip conditions. |
| extensions/business/deeploy/tests/test_dynamic_env_resolution.py | New tests for shmem path rewriting, provider validation, and semaphore key assignment. |
| extensions/business/deeploy/tests/test_create_requests.py | New tests for create request preparation, grouping, duplicate IDs, EXPOSED_PORTS validation, and shmem resolution. |
| extensions/business/deeploy/tests/support.py | Lightweight Deeploy test harness (stubs heavy deps; constructs a minimal _DeeployMixin host). |
| extensions/business/deeploy/tests/init.py | Adds Deeploy tests package marker. |
| extensions/business/deeploy/deeploy_mixin.py | Core Deeploy refactor: signature verification logging, EXPOSED_PORTS validation, shmem resolution API changes, response-key reset helper, updated create/update return values. |
| extensions/business/deeploy/deeploy_manager_api.py | Adds background pipeline metadata persistence worker + queues persistence after successful deployments. |
| extensions/business/deeploy/deeploy_job_mixin.py | Adds persist_job_pipeline_metadata() and CID deletion helper; reuses helper in existing delete path. |
| extensions/business/container_apps/tests/test_tunnel_runtime_behavior.py | New CAR tests for protocol-aware tunnel commands and extra tunnel mapping behavior. |
| extensions/business/container_apps/tests/test_semaphore_exports.py | New CAR tests for legacy + explicit semaphore networking exports (HOST_IP, HOST_PORT, etc.). |
| extensions/business/container_apps/tests/test_port_runtime_behavior.py | New CAR tests for runtime port allocation driven by legacy ports vs EXPOSED_PORTS. |
| extensions/business/container_apps/tests/test_legacy_config_mapping.py | New CAR tests ensuring legacy EXTRA_TUNNELS can drive exposed ports even without legacy ports config. |
| extensions/business/container_apps/tests/test_health_check_behavior.py | New CAR tests for health port validation and defaulting based on normalized exposed ports. |
| extensions/business/container_apps/tests/test_exposed_ports_model.py | New CAR tests for exposed-ports normalization rules, legacy compatibility, conflicts, and protocol fields. |
| extensions/business/container_apps/tests/support.py | Lightweight CAR test harness with dummy base plugin and deterministic port allocation. |
| extensions/business/container_apps/tests/init.py | Adds CAR tests package marker. |
| extensions/business/container_apps/README.md | Documents normalized EXPOSED_PORTS, new semaphore exports, and dynamic env guidance. |
| extensions/business/container_apps/container_utils.py | Implements exposed-ports normalization + legacy synthesis; refactors runtime port allocation and extra-tunnel derivation. |
| extensions/business/container_apps/container_app_runner.py | Uses normalized exposed ports for health defaults, tunnels, port selection, and semaphore env exports; adds HealthCheckMode.DISABLED. |
| docs/wip/2026-03-30T10-55-00Z-deeploy-pipeline-metadata-consistency.md | WIP note explaining eventual consistency of pipeline persistence after deployment confirmation. |
| docs/wip/2026-03-29T00-00-00Z-flatten-exposed-ports-add-protocol.md | WIP design note describing the flattened EXPOSED_PORTS tunnel shape + protocol support and migration rules. |
| AGENTS.md | Records the phased refactor plan/status and test-gate commands for CAR/Deeploy networking work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Legacy URL uses the container port, which is only reachable inside Docker's network. HOST_URL uses host_port so consumer plugins on the host can reach the container without port confusion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_validate_docker_image_format() was defined but never called, so invalid image names only failed at container start time. Now checked early in _validate_runner_config() for clear, immediate feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Docker image references have many valid forms (bare names, custom registries, digests) that a format regex would struggle to cover. The Docker daemon validates fully at pull/run time, so we only guard against missing or blank IMAGE here. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The non-empty string check in _validate_runner_config is sufficient; Docker handles real format validation at pull/run time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Changes the semaphore key format from sanitize_name("app_id__instance_id")
to "app_id__plugin_name". This makes keys deterministic from request data,
eliminating the need for a separate resolution pass and fixing the UPDATE
pipeline path which previously skipped shmem resolution entirely.
- Replace _resolve_shmem_references() with _resolve_shmem_in_plugins()
that uses plugin_name-based keys and runs inline during prepare
- Delete _compile_dynamic_env_ui() and _translate_dynamic_env_ui_in_instance_payload()
(DYNAMIC_ENV_UI compilation layer no longer needed — UI sends DYNAMIC_ENV directly)
- Remove plugin_semaphore_map from deeploy_specs (no longer needed)
- deeploy_prepare_plugins() now accepts app_id, returns just plugins (no tuple)
- Add duplicate plugin_name validation
- Add unknown shmem provider validation (fail at deploy time, not runtime)
- _autowire_native_container_semaphore() prefers plugin_name when available
- Update path now gets shmem resolution via flattened _resolve_shmem_in_plugins call
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… overrides
Flatten the nested tunnel sub-object into top-level fields on each
port config entry. Accept both flat and legacy nested input for
backward compatibility.
New normalized port config shape:
{ is_main_port, host_port, token, protocol, engine,
max_retries, backoff_initial, backoff_max }
- token: present = tunnel enabled (replaces tunnel.enabled + tunnel.token)
- protocol: tunnel origin scheme, default "http" (was hardcoded)
- engine: default "cloudflare", only cloudflare supported this phase
- Per-port retry overrides with global fallback defaults
- Flat fields win over nested tunnel fields when both present
- _build_tunnel_command uses protocol parameter
- _start_extra_tunnel accepts richer per-port config dicts
- get_cloudflare_protocol() override reads from normalized port config
- _validate_extra_tunnels_config stores rich config objects
- Validation: protocol against allowed set, engine cloudflare-only,
retry non-negative, backoff_max >= backoff_initial
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on markers - Add HealthCheckMode.DISABLED: app is ready immediately when container is running, bypasses all probing and delay - Export HOST_PROTOCOL semaphore key from main port's protocol field - HOST_URL now uses actual protocol instead of hardcoded http:// - Group deprecated config fields (PORT, CLOUDFLARE_TOKEN, EXTRA_TUNNELS, CLOUDFLARE_PROTOCOL, TUNNEL_ENGINE_PARAMETERS) at bottom of CONFIG with @deprecated comments pointing to EXPOSED_PORTS equivalents Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove max_retries, backoff_initial, backoff_max from EXPOSED_PORTS normalization and extra tunnel config. Per-port retry customization is deferred until a concrete need appears. Global tunnel retry settings remain unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
deeploy_verify_and_get_inputs crashed with 'NoneType has no attribute lower' when signature verification failed. Add null check with a clear error message before the address comparison. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Log the claimed sender, signature prefix, no_hash flag, and message prefix before verification. On failure, log a clear diagnostic message explaining what to check. On mismatch, log both recovered and claimed addresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pipeline CSTORE saves are moved off the request critical path into a dedicated background thread with retry logic, preventing slow R1FS operations from blocking deploy responses. Chainstore response keys are now reset before command dispatch so early confirmations are not lost. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
toderian
force-pushed
the
refactor-car-ports
branch
from
6d84063 to
3819a99
Compare
plugin_name from request payload was used unsanitized in semaphore key construction. Now enforced to [a-zA-Z0-9_-]+ in both _resolve_shmem_in_plugins and deeploy_prepare_plugins.
If an instance already has an explicit SEMAPHORE that differs from the derived key, raise ValueError instead of silently overwriting. Matching values are left untouched.
Apply same conflict check as _resolve_shmem_in_plugins: raise if an instance already has an explicit SEMAPHORE that differs from the derived key.
nested_token was used as fallback even when tunnel.enabled was explicitly False, unintentionally enabling tunnels. Now clears nested_token and nested_engine when enabled is falsy, so only a flat token can override a disabled nested tunnel.
_resolve_shmem_in_plugins eagerly sets SEMAPHORE on all named instances, which makes _autowire_native_container_semaphore treat the config as manually wired and skip adding SEMAPHORED_KEYS to containerized instances. Gate shmem resolution behind _has_shmem_dynamic_env in both create and update paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.