Skip to content

feat(runtime): multi-container v1 completion (M8 consolidation)#511

Merged
CiroGamboa merged 8 commits into
mainfrom
feat/multi-container-v1-completion
Jul 20, 2026
Merged

feat(runtime): multi-container v1 completion (M8 consolidation)#511
CiroGamboa merged 8 commits into
mainfrom
feat/multi-container-v1-completion

Conversation

@CiroGamboa

Copy link
Copy Markdown
Collaborator

TL;DR

Capability completion on the shipped multi-container substrate. Three interlocking arcs — environment-model follow-ups, observability v1, and network-policy completion — brought to their stated ship condition in a single milestone (the arcs converge at the same manifest + runtime substrate, so batching them removed cross-arc breakage windows). Ten PRs across nine sub-issues shipped: eight code+doc PRs merged into the integration branch, one issue partial-closed with a reframe + two prereqs filed, one umbrella decomposed into six workload-gated sub-issues. Additive on every compatibility surface — metrics.yaml gains provisioning_duration_s, aggregate.json gains captured_service_logs, EnvironmentManifest.stack gains five endpoint-override fields + a limited_internet_allowlist — all default-safe for existing packs. network_policy becomes fully three-valued (full_internet / no_internet / limited_internet) with real enforcement across every stack shape the harness materialises. Provision-failed trials now produce a minimal trial bundle so cost-aggregation and post-mortem tooling see a consistent shape. docs/SECURITY.md — actively wrong before this milestone (described a vanished env-net (internal:true)) — is rewritten to match the real runner-net posture. Closes #146, #354, #339, #338, #337, #144, #324, #323, #164 (reframed), #145 (decomposed), #373 (umbrella), #374 (umbrella).

Impact on existing tasks — read this first

Near-term (today):

  • Every existing task pack — canonical, in-tree examples, and out-of-tree packs — loads and runs byte-identically. All new manifest fields (stack.runner_port / db_service / db_port / rag_service / rag_port, stack.limited_internet_allowlist) default to None, and their absence reproduces prior behaviour exactly.
  • metrics.yaml gains a top-level provisioning_duration_s: float on every successful trial. Additive — consumers reading unknown keys are unaffected. No schema_version bump.
  • aggregate.json gains a captured_service_logs sub-object on RunAggregate. Always emitted (zero envelope on clean runs); no schema_version bump.
  • Provision-failed trials now produce a <output_dir>/trials/<task>/<idx>/ directory containing trajectory.yaml + metrics.yaml (with error: "provision_error") + grade.yaml. Prior behaviour: no directory at all. Cost-aggregation, post-mortem tooling, and tolokaforge status now see a consistent trial-directory shape whether the trial completed or failed to provision.
  • SharedStackRuntimeBackend run-level materialise failures now write <output_dir>/services/<name>.log + _capture.yaml before teardown. Prior behaviour: zero logs on run-level materialise failure.

Longer-term commitments:

  • The stack.limited_internet_allowlist schema is a compatibility surface: task packs declaring network_policy: limited_internet MUST carry a non-empty allowlist; other policies MUST omit it. Enforced at manifest LOAD with entry-named errors. Documented in docs/PROJECTS.md + ADR-0018.
  • The network_policy enum stays closed (full_internet / no_internet / limited_internet). No new values added; the three-value contract is now fully honoured across every stack shape.
  • The captured-service-logs vocabulary is durable: on-disk _capture.yaml.capture_reason values are provision_error / materialise_error / graded_failure (lowercase enum); rollup category enum is ServiceLogCaptureSource = provision_failure / trial_body / shared_stack_materialise. Task authors debugging with rg 'provision_error' still find every relevant artefact in one query; the rollup's category-level enum is one abstraction above the on-disk reason so it stays stable if a future producer varies its capture_reason text.

Safety guards, still in force:

  • SecurityContext + resource-cap defaults from earlier milestones; runner-net non-internal by construction (Case A EngineStack).
  • Runner stays on edge-net directly under all network policies — LLM-provider egress for llm_judge grading is preserved; the allowlist governs task-service egress only.

Follow-up tickets filed and left for later:

Design walkthrough

flowchart LR
    subgraph M8["Milestone 8 — Multi-container v1 completion"]
        direction TB

        subgraph EnvModel["Environment-model follow-ups"]
            direction TB
            e144["#144: 5 endpoint-override fields<br/>on stack: (runner_port / db_service / db_port /<br/>rag_service / rag_port). Fail-loud unknown-service<br/>validation at manifest LOAD."]
            e146["#146: task-pack image-layering guide<br/>(SWE-bench 3-tier: base → environment → instance).<br/>Docs-only, cache-friendliness rules."]
        end

        subgraph Observability["Observability v1 (umbrella #373)"]
            direction TB
            o354["#354: provisioning_duration_s<br/>time.monotonic() around provision → await_ready → endpoints.<br/>Additive to metrics.yaml."]
            o338["#338: provision-fail trial bundle<br/>ProvisionError writes trajectory.yaml + metrics.yaml +<br/>grade.yaml via existing TrialArtifactWriter."]
            o339["#339: shared-stack materialise capture<br/>Per-service logs to output_dir/services/*.log +<br/>_capture.yaml on materialise-error branch."]
            o337["#337: aggregate.json rollup<br/>CapturedServiceLogsRollup across all 3 surfaces.<br/>Category enum: provision_failure / trial_body / shared_stack_materialise."]
        end

        subgraph NetPolicy["Network policy completion (umbrella #374)"]
            direction TB
            n324["#324: Case A EngineStack decision<br/>Full_internet by construction (no untrusted task code<br/>on runner-net). ADR-0018 amended;<br/>SECURITY.md rewritten."]
            n323["#323: limited_internet enforcement<br/>squid forward-proxy sidecar with dstdomain ACL,<br/>Safe_ports 80/443, HTTP(S)_PROXY per app service,<br/>digest-pinned image."]
        end

        subgraph Meta["Meta / scope discipline"]
            direction TB
            m164["#164: DISCOVERY-BLOCKER + reframe<br/>Not code-shippable — $12-30 real spend + interpretation.<br/>Filed #503, #504 as prereqs; ADR-0016 stays load-bearing."]
            m145["#145: umbrella decomposed<br/>6 workload-gated sub-issues (#505-#510).<br/>None gate M8 per the umbrella's own framing."]
        end
    end

    e144 -.->|manifest surface| Observability
    e144 -.->|manifest surface| NetPolicy
    o354 -.->|informs future study prereqs| m164
    o337 -.->|informs future study prereqs| m164
    n324 -.->|shared runner-net invariant| n323
Loading

Key design choices

Decision Rationale
time.monotonic() for duration measurement, not time.time() (#354). Monotonic clock is immune to wall-clock adjustments (NTP, DST, user reset). Correct interval-measurement primitive; same reasoning applies anywhere the codebase measures durations.
File-level YAML amendment, not model field, for provisioning_duration_s / error / captured_service_logs (#354, #338). Metrics (Pydantic extra="forbid") is written by the conductor inside run(). Adding fields there would force a default + overwrite double-write path. Post-run() read-add-write via _amend_trial_metrics is a single durable path; both existing readers use yaml.safe_load + .get(), not model round-trip.
Additive amendment to metrics.yaml, not schema break (#354, #338). No schema_version bump; consumers reading unknown keys work unchanged. Same posture as prior milestone-12 additions.
Shared _amend_trial_metrics(task_id, trial_idx, updates) helper covers every amendment site (#354). DRY: provisioning_duration_s (new) and captured_service_logs (from #418) route through one helper; old _amend_metrics_with_captured_logs deleted (no shim). Future per-trial metrics land in the same seam.
Third log-capture surface, not a fourth mechanism (#339). #302 established per-trial provision-fail capture; #418 extended to grade-fail; #339 extends to run-level materialise-fail. All three write the same .log + _capture.yaml shape with distinct capture_reason values (provision_error / graded_failure / materialise_error). One capture pattern, three trigger sites.
<output_dir>/services/ mirrors <trial_dir>/services/, one level up (#339). Run-level bundle sits next to trials/ when the failure is at the run level (before any trial ran). Location signals scope.
Wiring gap discovered + fixed (#339). Orchestrator._construct_runtime_backend was forwarding log_capture to PerTrialRuntimeBackend but not to SharedStackRuntimeBackend (env-manifest branch). One-line forward closes it; backend attribute defaults to None → silent no-op when unconfigured.
Reuse TrialArtifactWriter — no schema duplication for the provision-fail bundle (#338). Same writer the conductor's success path uses. Zero risk of failed-bundle and completed-bundle schemas drifting; both paths pick up future schema bumps automatically.
Vocabulary continuity across the durable record (#338). Lowercase enum values (provision_error / materialise_error / graded_failure) shared between _capture.yaml.capture_reason, metrics.yaml.error, and trajectory.yaml.termination_reason. Task authors debugging with rg 'provision_error' find every relevant artefact in one query.
Behaviour-preserving refactor separated as its own stage (#338 stage 1). Stage 1 drops log_capture param, keys on output_dir (invariant: LogCaptureConfig.output_root == output_dir). Stage 2 is the feature. Reviewability: the diff of the feature commit isolates the actual behavioural change.
Category-level enum in aggregate rollup, not literal capture_reason string (#337). ServiceLogCaptureSource values are provision_failure / trial_body / shared_stack_materialise — semantic categories one abstraction above on-disk capture_reason. Keeps rollup classification stable if a future producer varies its capture_reason text; per-entry provenance still carries the exact literal.
All three capture surfaces rolled up, not just the one the issue named (#337). Issue asked for per-trial _capture.yaml; common path is metrics.yaml.captured_service_logs. Rolling up only the named surface would miss most captures. Architect surfaced the widening before implementation.
Always-emit zero envelope on clean runs (#337). Distinguishes "feature shipped, nothing captured" (captures: 0) from "pre-feature file" (field absent). Consumers can rely on the field's presence to gate their reader.
Fail-safe file-scan on rollup collection (#337). Malformed _capture.yaml / metrics.yaml log-and-skip; a corrupt artefact never breaks aggregate generation. Same posture as _collect_existing_cost.
Five individual endpoint-override fields, not an endpoint_map dict (#144). Each endpoint has a distinct typed consumer (runner → gRPC; db → HTTP; rag → candidate-scan). Dict's "scales to N endpoints" advantage is illusory when the Nth endpoint requires new wiring anyway. Individual fields also map 1:1 onto resolve_env_endpoints' kwargs, so Stage 2 was "stop hardcoding, read the manifest."
None = convention lookup; explicit non-None = override (#144). For db_service / db_port / rag_service / rag_port. runner_port stays a required int (default 50051) — no None state because the runner always resolves; a bad port fails loud at provision.
Fail-loud on unknown service overrides at manifest LOAD (#144). _check_endpoint_services_declared validator checks db_service / rag_service against load_compose()["services"] keys. db_service: "nonexistent" raises ValidationError listing the compose's actual services. Not deferred to runtime.
Env-identity hash unaffected by endpoint fields (#144). Endpoint overrides are operational, not architectural identity. Additive schema + stable identity = cache-safe (matches the M18 env-identity snapshot posture from #419).
Case A is full_internet by construction, not by config (#324). Built-in stack materialises only first-party engine services (runner + db-service ± mock-web ± rag-service). No untrusted task code to isolate; runner needs LLM-provider egress for llm_judge in-container grading. Threading network_policy onto the built-in path would add a knob whose safe value is the only value.
Network.internal=True primitive kept as faithful docker-py serialiser (#324). Dead code today but the abstraction is right; covered by a round-trip unit test. Removing it would strand Network's parity with docker-py's API.
docs/SECURITY.md rewritten in full — Rule 8 (doc-freshness) (#324). Half-fixing (patching only one line) would leave known-false threat-table entries next to the correction. All references to env-net / docker-compose.yaml deleted; threat table refreshed with the actual mechanisms.
squid over tinyproxy — decisive for security (#323). dstdomain maps safely to exact + *.-suffix ACLs; tinyproxy's Filter regex is an unanchored-substring footgun where an allowlist entry openai.com would match evil-openai.com.attacker.net. Correctness of the allowlist control beats image size.
CONNECT-only for HTTPS, no TLS interception (#323). No CA plumbing in runner or app containers; allowlist matches on SNI/CONNECT target. Adequate for the "reach this host, not that one" use case; simpler failure mode.
Safe_ports 80 443 restriction on non-CONNECT methods (#323). Reviewer-driven hardening: without it, plain-HTTP GET http://allowlisted:8080/ was forwarded because dstdomain doesn't constrain the port on non-CONNECT paths. acl Safe_ports port 80 443 + http_access deny !Safe_ports before the allow chain — plan's stated intent, missed on first cut, caught in review.
Runner stays on edge-net directly, not proxied (#323). Same posture as no_internet — runner egress is engine-scope (LLM providers for llm_judge), independent of the task's allowlist. The allowlist governs task-service egress only.
Digest-pinned proxy image (#323). ubuntu/squid@sha256:6a097f68… — not a floating tag. Immutable across pulls per the multi-container image-pinning contract.
HTTP 403 (proxy deny), NOT returncode != 0 (transport failure) (#323). A proxy-denied CONNECT still completes at the transport layer; only the HTTP response reveals the deny. The no_internet test pattern (returncode != 0) would silently under-test here. Integration test asserts on %{http_code} explicitly.
Dead-code removal in the same series (#323). verify_network_policy_supported (interim fail-loud shim from #301) and NetworkPolicyError (only referenced by the pre-rewrite test) both deleted. Contradiction removal — those code paths are unreachable once enforcement lands.
Fail-loud manifest validation at LOAD, not runtime, for limited_internet_allowlist (#323). Cross-field validator: limited_internet requires non-empty allowlist; other policies forbid any allowlist. Per-entry validator rejects schemes / ports / paths / IPs / multi-star / dupes with entry-named errors.
Docs turn theory into recipe (#146). ADR-0009 documents the SWE-bench pattern as an "industry precedent studied"; the guide turns that into concrete task-authoring recipes (three copy-pasteable Dockerfiles + a compose snippet + cache-friendliness rules) so a task-pack author has a mechanical path from "isolation-safe" to "cache-friendly" without re-deriving the pattern.
No new engine flag for cache tuning (#146). SWE-bench's harness carries a --cache_level knob; we deliberately don't. Docker's content-addressable layer cache handles most of the work automatically when task-pack authors pin images by digest/immutable tag. The pinned-image contract we already ship is the cache-key contract we need.
Not everything ships as code (#164). Some issues are meta-work (measurement studies, roadmap decisions). Owning the framing honestly ("code-ship vs not") is part of the milestone. Closing #164 with a reframe + two filed prereqs (#503, #504) is better than either half-shipping a harness that answers ~1.5 of 4 questions, or leaving the milestone open on a deliverable the pipeline can't produce.
Workload-gated decomposition for perf umbrellas (#145). Umbrella body explicitly said "each becomes an individual PR once its perf value is either measured (via the benchmark harness) or forced by a real workload." Decomposed into 6 sub-issues (#505-#510) left milestone-less; sequencing is opportunistic, not milestone-gated.

Industry precedents

  • SWE-bench 3-tier image hierarchy (base → environment → instance). Cited in ADR-0009 as prior art on isolation + cache-friendliness; [docs] Task-pack image-layering guide (SWE-bench 3-tier pattern for cache-with-isolation) #146 turned that reference into task-authoring recipes. What we borrowed: the tier decomposition + the "instance layer must be the smallest re-buildable unit" invariant. What we deliberately rejected: SWE-bench's --cache_level harness knob — Docker's content-addressable layer cache with pinned-image digests is the cache-key contract we already ship.
  • squid dstdomain vs tinyproxy Filter (feat(runtime): limited_internet egress allowlist enforcement (proxy sidecar) #323). squid's dstdomain ACL matches domains exactly or via *.-suffix wildcards, both anchored — the safe, documented model for allowlist enforcement. tinyproxy's Filter accepts unanchored regexes, where an allowlist entry openai.com would match evil-openai.com.attacker.net. Chose squid over the smaller-image tinyproxy on this correctness ground alone.
  • Safe_ports hardening (feat(runtime): limited_internet egress allowlist enforcement (proxy sidecar) #323 post-review). Standard squid deployment pattern — restrict non-CONNECT methods to canonical HTTP(S) ports before the allow chain. Reviewer flagged the omission; added acl Safe_ports port 80 443 + http_access deny !Safe_ports.

Suggested review order

  1. [docs] Task-pack image-layering guide (SWE-bench 3-tier pattern for cache-with-isolation) #146 — task-pack image-layering guide (docs-only, no code impact) — bc784e4.
  2. Record per-trial provisioning duration as a first-class metric #354provisioning_duration_s metric (introduces the _amend_trial_metrics seam that later issues extend) — 0c91c9c.
  3. runtime(observability): capture shared-stack compose logs on run-level materialise failure #339 — shared-stack materialise-failure log capture (the third capture surface — reviewing this before runtime(observability): surface captured service logs in run-level aggregate report #337 makes the aggregate rollup's three-surface handling obvious) — 6f3542a.
  4. runtime(observability): provision-failed trials get no full trial bundle #338 — provision-fail trial bundle (behaviour-preserving refactor + feature, two commits) — 0e42b36.
  5. runtime(observability): surface captured service logs in run-level aggregate report #337captured_service_logs rollup in aggregate.json (three-commit series consuming all three surfaces from feat(runtime): per-service log capture on trial failure (Multi-container 4/5) #302/Per-service Docker logs not captured on grade-fail (only on provision/trial-body fail) #418/runtime(observability): capture shared-stack compose logs on run-level materialise failure #339) — 8ab1191.
  6. [design] EnvironmentManifest: endpoint-resolution flexibility (runner_port, db_service, db_port, rag_service, rag_port) #144 — endpoint-resolution override fields on EnvironmentManifest.stack (three-commit series: schema → wire → docs) — 3a20d1d.
  7. fix(runtime): built-in EngineStack (Case A) ignores network posture #324 — Case A EngineStack full_internet decision (design-recording PR; runtime code diff empty; docs/SECURITY.md full rewrite) — 9f131ab.
  8. feat(runtime): limited_internet egress allowlist enforcement (proxy sidecar) #323limited_internet egress-proxy sidecar (largest single item; 4 stages + 1 post-review Safe_ports hardening) — 4e245c4.

Verification

Per-PR CI: each of the eight PRs shipped with lint + test-smoke lanes green on the branch (and consistent with main) — those two lanes were the actual merge gates. hygiene-review is known-broken on every PR from this session (tracked as infra-side #425); its per-PR failure was not a merge gate on any of the eight.

Behaviour-locking test tier per PR (as required by AGENTS.md — never mocks):

Post-merge validation run on feat/multi-container-v1-completion: make docker-status clean (stack was warmed once at Step 2.5 and kept warm across the milestone); targeted run_tests against the affected markers on every merge; no LLM-layer changes so no capability tests were needed.

Rebase: single CHANGELOG conflict on rebase (main cut v0.9.1 empty-release-marker after the integration branch started); resolved by placing the branch's ## Unreleased section above the release marker. Subsequent 7 commits applied cleanly.

Deliberately skipped: no test-integration-full or LLM-scale end-to-end run against real providers — the eight PRs are all substrate + observability + docs, none touching model dispatch. The eventual #164-reframed measurement study (once #503 + #504 land) is the load-bearing end-to-end validation for the multi-container substrate; running it now would be premature.

What's next

Immediate follow-ups filed and left for later: #503 (per-role token/cost accounting in metrics.yaml) + #504 (per-stage wall-clock brackets) unblock the reframed #164 measurement study — those three together will produce the interpreted cost/latency comparison the original #164 asked for. Six workload-gated sub-issues (#505-#510) decomposed from #145 stand ready to ship opportunistically as their perf value is measured (via #508's benchmark harness) or forced by a real workload. Pre-existing schema drift #486 (AggregateMetrics extra="forbid" vs percentile keys) is filed and separable from M8.

#485)

* refactor(runtime): decouple _amend_trial_metrics from log_capture (#338 stage 1)

* feat(runtime): minimal trial bundle for provision-failed trials (#338 stage 2)

When ProvisioningTrialExecutor catches a ProvisionError, write a minimal
per-trial bundle (trajectory.yaml + metrics.yaml + grade.yaml) to
<output_dir>/trials/<task>/<idx>/ by reusing the run's TrialArtifactWriter,
then amend metrics.yaml with top-level error: provision_error + error_reason
via the shared _amend_trial_metrics path. Bundle-write failure is best-effort
and never masks the synthesized failed TrialResult.
…) (#488)

* feat(observability): captured_service_logs schema on RunAggregate (#337 stage 1)

* feat(observability): produce captured_service_logs rollup in aggregate.json (#337 stage 2)

* docs(tests): describe actual scope in orchestrator service-log rollup test module (#337)
* feat(manifest): endpoint-resolution override fields on StackPatch + EnvironmentManifest (#144 stage 1)

* feat(compose): read endpoint overrides from EnvironmentManifest in resolve_env_endpoints (#144 stage 2)

* docs(manifest): endpoint-resolution override fields — ADR-0009 + guide + CHANGELOG (#144 stage 3)
…on (#324) (#497)

Record the Option-B decision that the built-in EngineStack path (Case A)
is full_internet by construction and carries no network_policy surface:
runner-net is a non-internal bridge and the runner retains LLM-provider
egress for in-container LLM-as-judge grading. Only task-declared stacks
(Case B/C) have an enforceable network_policy.

- ADR-0018: new "Case A network posture" subsection under network-policy
  enforcement + repaired Case A forward-reference + dated amendment.
- RUNTIME_BACKENDS.md: new "Network posture" section.
- SECURITY.md: rewrite architecture overview, threat table, testing, and
  checklist to describe the actual runner-net (non-internal, docker-py
  EngineStack) model, replacing the vanished env-net / docker-compose.yaml
  description and the false "addressed by env-net internal:true" threat.
- tests/unit/test_network_internal.py: lock the Network.internal docker-py
  round-trip and the EngineStack.create_networks non-internal invariant.
…idecar) (#323) (#502)

* feat(manifest): limited_internet_allowlist schema + validation (#323 stage 1)

* feat(runtime): limited_internet squid-sidecar enforcement transform (#323 stage 2)

* test(runtime): real-docker limited_internet enforcement + NetworkPolicyError removal (#323 stage 3)

* docs(runtime): limited_internet shipped — ADR-0018 + MULTI_CONTAINER_GUIDE + CHANGELOG (#323 stage 4)

* fix(runtime): squid Safe_ports restriction on limited_internet proxy (#323)

Restrict non-CONNECT proxied requests to ports 80 and 443 so plain-HTTP GETs to odd ports on allowlisted hosts default-deny.
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

@CiroGamboa
CiroGamboa merged commit a26c7cb into main Jul 20, 2026
7 of 8 checks passed
@CiroGamboa
CiroGamboa deleted the feat/multi-container-v1-completion branch July 20, 2026 16:10
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.

[docs] Task-pack image-layering guide (SWE-bench 3-tier pattern for cache-with-isolation)

1 participant