feat(agent-challenge): CVM sizing, token handoff, and Phala eval deploy hardening - #49
Merged
Conversation
Mirror standalone agent-challenge dual-flag admission: GHCR live-registry refs, package_tree_sha score-chain refuse, progress module, and fixture pins.
Keep package-scoped ruff line-length 100 after GHCR path retarget.
…e tests Lock stage defaults (review tdx.small/20GB, eval tdx.xlarge/100GB), disk bounds/billing, lifecycle budget with disk, provision disk_size emission, and frozen eval/review compose hashes before any production sizing change.
…faults Split review (tdx.small/20GB) and eval (tdx.xlarge/100GB) defaults, add disk bounds and billing helpers, and fold optional disk into projected cost while keeping CpuShape free of a disk field.
Charge both stages for compute plus disk against the shared $20 money cap and update the ordered CLI lifecycle fixture for disk-aware totals.
Thread stage disk_size_gb on eval/review deployment plans and send disk_size as a sibling of compose_file without mutating measured compose documents.
… CLI Default eval to tdx.xlarge, add --review-disk-size-gb/--eval-disk-size-gb, pass disks into lifecycle budget checks, and harden offline deploy test doubles.
Record review tdx.small/20GB and eval tdx.xlarge/100GB defaults, disk rate, and unchanged CPU-only $20 money cap in miner and validator self-deploy docs.
Lock DELETE /cvms/{id} allowlist behavior, 204/404 success, unique app_id
resolution, and CLI teardown without a phala binary before the fix lands.
Allow DELETE /cvms/{id} (204/404 success) and refuse ambiguous app_id
matches when resolving teardown identity without a phala binary.
Route teardown through PhalaCloudClient.delete_cvm, accept optional --cvm-id/--app-id, and resolve unique app_id matches from GET /cvms.
Assert delete_cvm path, ambiguous app_id refusal, and docs no longer require a phala binary in default_phala_teardown.
Describe DELETE /cvms/{id} as the primary path and keep manual phala
cvms list/delete strings for VAL-DEPLOY-020 verification.
Check --cvm-id/--app-id before constructing PhalaCloudClient so missing identity fails closed without requiring credentials.
Mirror review prepare recovery: when eval prepare returns token-less secret_delivery, cancel the current run and retry so deploy receives a fresh one-shot capability. Production residual on submission 3 left attempts 1 and 2 stuck after standalone prepare/retry spent the token. Never cancel when the token is already present; never invent tokens.
Assert token-present skips cancel/retry, token-absent recovers via
cancel+retry, sticky absence raises RouteClientError, and the
{env_key,token} secret_delivery shape stays fail-closed. Also pin
review deploy fixtures to the production REVIEW_API_BASE_URL.
…UN_TOKEN Cover the production closed loop where eval deploy injects the one-time run token into the CVM but never surfaces it for eval result. Assert --emit-run-token / --token-output handoff, fail-closed without either on live deploy, dry-run exemption, --output hygiene, and prepare/status redaction regressions.
Live eval deploy now requires --token-output and/or --emit-run-token so the host can post via eval result. Always include eval_run_id on success stdout; write the token only to the secure 0600 file or optional stdout key. Never put the token in --output, redacted prepare/status, logs, or exception text. CVM encrypted_env injection is unchanged.
…pends it The handoff check depends only on argv, but ran after _obtain_eval_prepare_with_token had already consumed the single EVAL_RUN_TOKEN delivery. A miner who omitted the flags burnt the token and then hit the error, reproducing the unrecoverable state the handoff exists to prevent. Validate the destination before any remote mutation and cover it with a test asserting eval_prepare is never called.
Lock RED→GREEN contract for plan vs CLI shape mismatch and optional --expected-measurement rtmr0 pin check before Phala create.
…la create Name plan and CLI shapes plus vm_shape/instance_type/rtmr0, warn that a stale allowlist pin only surfaces as a generic key-release denial, and abort before spend. Optional --expected-measurement compares rtmr0 with truncated prefixes only.
Document guest emit-only design, token handoff flags, host scrape of BASE_BENCHMARK_RESULT, eval result posting, teardown, and the shape/pin footgun so miners can finish a run from the docs alone.
📝 WalkthroughWalkthroughThe PR aligns live container references with GHCR, adds stage-specific disk sizing and budget accounting, replaces CLI-based CVM teardown with HTTP deletion, hardens eval token and measurement handoff, adds attested progress ingestion, and threads review materials through score-chain verification. ChangesSelf-deploy lifecycle and safety
Evaluation verification and progress
GHCR live registry migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/shapes.py (1)
245-254: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winOver-cap message can raise
KeyErrorfor whitespace-padded shape names.
projected_cost_usdlooks upCPU_TDX_SHAPES.get(instance_type.strip()), but the error path indexesCPU_TDX_SHAPES[instance_type]unstripped. A padded" tdx.xlarge "that exceeds the cap raises an unhandledKeyErrorinstead ofOverCapError, which callers (lifecycle, CLI) do not catch.🛡️ Proposed fix
cost = projected_cost_usd( instance_type, max_runtime_hours=max_runtime_hours, disk_size_gb=disk_size_gb, ) if cost > money_cap_usd: + shape = CPU_TDX_SHAPES[(instance_type or "").strip()] disk_note = "" if disk_size_gb is not None: disk_note = f" + disk {disk_size_gb}GB" raise OverCapError( f"projected cost ${cost:.2f} ({instance_type}{disk_note} @ " - f"${CPU_TDX_SHAPES[instance_type].usd_per_hour}/h x {max_runtime_hours}h) " + f"${shape.usd_per_hour}/h x {max_runtime_hours}h) "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/shapes.py` around lines 245 - 254, Update the over-cap error construction in the cost-validation flow to use the normalized shape name, matching the stripped lookup used by projected_cost_usd, when reading CPU_TDX_SHAPES and displaying the instance type. Ensure whitespace-padded names still raise OverCapError rather than KeyError.packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py (1)
127-152: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winKeep required package-tree proof in every eval-plan fixture.
Both fixtures construct eval plans without
package_tree_sha; the parity test then expectsvalidate_eval_plan()and deployment-plan construction to proceed. This weakens regression coverage for the required review residual/package-tree proof gate.
packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py#L127-L152: restore a validpackage_tree_shain the prepared plan.packages/challenges/agent-challenge/tests/test_eval_keyrelease_ratls_bootstrap.py#L487-L535: include the same required proof in_stub_eval_plan.As per coding guidelines, “Require AGATE package LLM rules residual and
package_tree_shaproof before TEE authentication; otherwise fail closed without evaluation or attestation.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py` around lines 127 - 152, The eval-plan fixtures omit the required package-tree proof, weakening coverage of the validation gate. In packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py lines 127-152, add a valid package_tree_sha to the prepared plan; in packages/challenges/agent-challenge/tests/test_eval_keyrelease_ratls_bootstrap.py lines 487-535, add the same required proof to _stub_eval_plan. Preserve the existing fixture structure and ensure both plans satisfy validate_eval_plan() and deployment-plan construction.Source: Coding guidelines
🧹 Nitpick comments (5)
packages/challenges/agent-challenge/tests/test_selfdeploy_shapes_disk.py (1)
224-240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the dead placeholder
encryptedobject.The first
SimpleNamespace(withenv_keys=plan) is overwritten two statements later and only adds confusion; the local import can move to the top of the file.♻️ Proposed cleanup
plan = _minimal_review_plan(disk_size_gb=20) - encrypted = SimpleNamespace( - assignment_id="asg-1", - app_identity=plan.app_identity, - kms_public_key_sha256=plan.kms_public_key_sha256, - measurement_allowlist_sha256=plan.measurement_allowlist_sha256, - env_keys=plan # placeholder replaced below - ) from agent_challenge.selfdeploy import review as review_mod encrypted = SimpleNamespace(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/tests/test_selfdeploy_shapes_disk.py` around lines 224 - 240, Remove the initial overwritten SimpleNamespace assigned to encrypted in the test, retaining only the complete object that uses review_mod.REVIEW_ALLOWED_ENVS and ciphertext. Move the agent_challenge.selfdeploy.review import to the file’s top-level imports.packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py (1)
246-246: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tmp_pathis apathlib.Path, notTempPathFactory.The annotation is wrong (the factory fixture is
tmp_path_factory); usePathas the other new test module does.Also applies to: 369-369, 404-404
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py` at line 246, Correct the tmp_path parameter annotations in the affected tests to use pathlib.Path rather than pytest.TempPathFactory, while leaving the separate tmp_path_factory fixture annotation unchanged. Apply this consistently at the occurrences near the tests around lines 246, 369, and 404.packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.py (1)
66-77: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winSilent fallback can desync the provisioned disk from the budgeted disk.
If
replacefails and the attribute set also fails, the plan silently keeps its defaultdisk_size_gbwhilevalidate_lifecycle_budgetwas computed against the requested size — provisioning would then bill a different disk than budgeted. Prefer failing closed (or at least narrowing the swallow toAttributeError).♻️ Proposed refactor
try: return replace(plan, disk_size_gb=disk_size_gb) except TypeError: # Offline tests inject SimpleNamespace plan doubles. - try: - plan.disk_size_gb = disk_size_gb - except Exception: - pass - return plan + plan.disk_size_gb = disk_size_gb + return plan🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.py` around lines 66 - 77, Update _with_disk_size so failure to apply disk_size_gb cannot silently return a plan with its default size. Narrow the fallback exception handling to the expected attribute-assignment failure and otherwise propagate errors; if assignment still cannot be performed, raise an explicit error instead of returning the unchanged plan, keeping the plan’s disk size synchronized with the validated budget.packages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.py (1)
45-93: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTwo independent full scans per progress call; consider merging and bounding growth.
_existing_progress_eventand_max_client_sequenceeach issue a separate, unfiltered-at-SQL-level query over allTaskLogEventrows for the(submission_id, task_id, event_type)tuple, then parsemetadata_jsonper row in Python for two different purposes. This can be merged into one query + one loop that computes both the existing-match and the highest sequence together, halving DB round trips and JSON parsing per request. There's also no cap on the number of progress events recordable per task (unlikeeval_result_max_event_log_entriesused elsewhere for the final result), so this cost grows unbounded over a long-running eval.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.py` around lines 45 - 93, The progress handling currently performs two full database scans and allows unbounded event accumulation. Merge _existing_progress_event and _max_client_sequence into one query and iteration that returns both the matching event and highest client sequence, then update callers to use that result; add an appropriate per-task progress-event limit consistent with the existing eval_result_max_event_log_entries configuration.packages/challenges/agent-challenge/src/agent_challenge/evaluation/direct_result.py (1)
695-743: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOutcome-parsing logic duplicates
fresh_review_gate.py; consider extracting a shared helper.
_load_review_materials_for_runre-implements the samereview_verification_outcome_jsonstring/dict parsing andpackage_residualextraction that already exists inadmit_eval_cvm_launch_from_assignment. Keeping two independent parsers for this AGATE-security-relevant payload risks silent behavioral drift (e.g., one handling malformed JSON slightly differently from the other).Also:
import json as _json(line 722) is a local import; prefer a top-levelimport jsonif not already present. Thedict[str, Any] | Nonereturn type also forces# type: ignore[assignment]at the call site (lines 939-941) — a smallTypedDict/dataclass for{envelope, outcome, package_residual}would remove the need for that.outcome_json = getattr(assignment, "review_verification_outcome_json", None) in
fresh_review_gate.py'sadmit_eval_cvm_launch_from_assignmentmirrors the new parsing here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/challenges/agent-challenge/src/agent_challenge/evaluation/direct_result.py` around lines 695 - 743, Extract the shared verification-outcome parsing and package_residual extraction used by _load_review_materials_for_run and admit_eval_cvm_launch_from_assignment into one helper, preserving identical malformed-input behavior. Use a top-level json import instead of the local alias, and introduce a TypedDict or dataclass for the returned envelope/outcome/package_residual structure so callers no longer need type-ignore assignments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/challenges/agent-challenge/docs/miner/self-deploy.md`:
- Around line 286-289: Make the HTTP teardown documentation executable and
consistent: in packages/challenges/agent-challenge/docs/miner/self-deploy.md
lines 286-289, remove or rewrite the stale `phala cvms delete <id> -f` CLI
instruction to reflect the Python/API teardown flow; in
packages/challenges/agent-challenge/docs/validator/self-deploy.md lines 281-282,
replace the `HTTP DELETE /cvms/{id}` entry inside the bash fence with the
executable `python -m agent_challenge.selfdeploy ... teardown` command,
preserving the documented ID/app resolution behavior.
In `@packages/challenges/agent-challenge/golden/live-registry-refs.json`:
- Around line 3-25: Update the live registry manifest so parse_live_registry()
accepts only images currently available in GHCR: publish/copy the orchestrator
and task images, then replace their registry_ref and orchestrator_image values
with the resulting manifest digests, or remove unavailable entries until
publication is complete. Remove stale OPS_REQUIRED/BLOCKED_REPUBLISH references
from shipped entries.
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.py`:
- Around line 159-196: Serialize the progress check-and-insert flow by locking
the relevant EvalRun row with the existing transaction locking pattern before
calling _existing_progress_event and _max_client_sequence. Update the
surrounding progress submission function so the lock is acquired before both
reads and remains held through record_task_event, preserving idempotent retries
and monotone sequencing for concurrent submissions.
In `@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.py`:
- Around line 1037-1052: Update _write_eval_run_token_file so file-descriptor
ownership transfers to fdopen before handle.write(token) executes. Ensure the
finally block cannot close the descriptor after the context manager has closed
it, while preserving cleanup if fdopen itself fails and allowing the original
write exception to propagate.
---
Outside diff comments:
In
`@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/shapes.py`:
- Around line 245-254: Update the over-cap error construction in the
cost-validation flow to use the normalized shape name, matching the stripped
lookup used by projected_cost_usd, when reading CPU_TDX_SHAPES and displaying
the instance type. Ensure whitespace-padded names still raise OverCapError
rather than KeyError.
In
`@packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py`:
- Around line 127-152: The eval-plan fixtures omit the required package-tree
proof, weakening coverage of the validation gate. In
packages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.py
lines 127-152, add a valid package_tree_sha to the prepared plan; in
packages/challenges/agent-challenge/tests/test_eval_keyrelease_ratls_bootstrap.py
lines 487-535, add the same required proof to _stub_eval_plan. Preserve the
existing fixture structure and ensure both plans satisfy validate_eval_plan()
and deployment-plan construction.
---
Nitpick comments:
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/direct_result.py`:
- Around line 695-743: Extract the shared verification-outcome parsing and
package_residual extraction used by _load_review_materials_for_run and
admit_eval_cvm_launch_from_assignment into one helper, preserving identical
malformed-input behavior. Use a top-level json import instead of the local
alias, and introduce a TypedDict or dataclass for the returned
envelope/outcome/package_residual structure so callers no longer need
type-ignore assignments.
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.py`:
- Around line 45-93: The progress handling currently performs two full database
scans and allows unbounded event accumulation. Merge _existing_progress_event
and _max_client_sequence into one query and iteration that returns both the
matching event and highest client sequence, then update callers to use that
result; add an appropriate per-task progress-event limit consistent with the
existing eval_result_max_event_log_entries configuration.
In `@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.py`:
- Around line 66-77: Update _with_disk_size so failure to apply disk_size_gb
cannot silently return a plan with its default size. Narrow the fallback
exception handling to the expected attribute-assignment failure and otherwise
propagate errors; if assignment still cannot be performed, raise an explicit
error instead of returning the unchanged plan, keeping the plan’s disk size
synchronized with the validated budget.
In `@packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py`:
- Line 246: Correct the tmp_path parameter annotations in the affected tests to
use pathlib.Path rather than pytest.TempPathFactory, while leaving the separate
tmp_path_factory fixture annotation unchanged. Apply this consistently at the
occurrences near the tests around lines 246, 369, and 404.
In `@packages/challenges/agent-challenge/tests/test_selfdeploy_shapes_disk.py`:
- Around line 224-240: Remove the initial overwritten SimpleNamespace assigned
to encrypted in the test, retaining only the complete object that uses
review_mod.REVIEW_ALLOWED_ENVS and ciphertext. Move the
agent_challenge.selfdeploy.review import to the file’s top-level imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f2754be-7621-45e3-85f9-6052ae718a9d
📒 Files selected for processing (32)
packages/challenges/agent-challenge/docker/review/phala_pre_launch.shpackages/challenges/agent-challenge/docs/miner/self-deploy.mdpackages/challenges/agent-challenge/docs/validator/self-deploy.mdpackages/challenges/agent-challenge/golden/live-registry-refs.jsonpackages/challenges/agent-challenge/src/agent_challenge/canonical/live_registry.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/direct_result.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/score_chain_gate.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/eval.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/lifecycle.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/measurements.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/phala.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/review.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/shapes.pypackages/challenges/agent-challenge/tests/test_canonical_build_push.pypackages/challenges/agent-challenge/tests/test_eval_compose_phala_provision_parity.pypackages/challenges/agent-challenge/tests/test_eval_deploy_shape_pin_loud_fail.pypackages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.pypackages/challenges/agent-challenge/tests/test_eval_keyrelease_ratls_bootstrap.pypackages/challenges/agent-challenge/tests/test_live_registry.pypackages/challenges/agent-challenge/tests/test_ordered_selfdeploy_cli.pypackages/challenges/agent-challenge/tests/test_own_runner_backend_live_registry.pypackages/challenges/agent-challenge/tests/test_own_runner_live_registry_resolution.pypackages/challenges/agent-challenge/tests/test_phala_create_ack_and_cli_token.pypackages/challenges/agent-challenge/tests/test_residual_orch_probes.pypackages/challenges/agent-challenge/tests/test_selfdeploy_docs_accuracy.pypackages/challenges/agent-challenge/tests/test_selfdeploy_live_registry_wiring.pypackages/challenges/agent-challenge/tests/test_selfdeploy_ordered_trust_hardening.pypackages/challenges/agent-challenge/tests/test_selfdeploy_shapes_disk.pypackages/challenges/agent-challenge/tests/test_selfdeploy_teardown_http.pypackages/challenges/agent-challenge/tests/test_sizing_compose_hash_freeze.py
Comment on lines
+286
to
+289
| Deletes the review CVM via the Phala Cloud HTTP API (`DELETE /cvms/{id}`). | ||
| No external `phala` CLI binary is required. Pass `--cvm-id` from the deploy | ||
| output, or `--app-id` (the plan `app_identity`) to resolve a unique match from | ||
| `GET /cvms`. Ambiguous matches are refused. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the HTTP teardown documentation internally consistent and executable.
packages/challenges/agent-challenge/docs/miner/self-deploy.md#L286-L289: remove or rewrite the following stale statement that the CLI usesphala cvms delete <id> -f; it contradicts the new “no externalphalaCLI” behavior.packages/challenges/agent-challenge/docs/validator/self-deploy.md#L281-L282: replaceHTTP DELETE /cvms/{id}inside the bash fence with an executablepython -m agent_challenge.selfdeploy ... teardowncommand; the current block attempts to runHTTP.
📍 Affects 2 files
packages/challenges/agent-challenge/docs/miner/self-deploy.md#L286-L289(this comment)packages/challenges/agent-challenge/docs/validator/self-deploy.md#L281-L282
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/challenges/agent-challenge/docs/miner/self-deploy.md` around lines
286 - 289, Make the HTTP teardown documentation executable and consistent: in
packages/challenges/agent-challenge/docs/miner/self-deploy.md lines 286-289,
remove or rewrite the stale `phala cvms delete <id> -f` CLI instruction to
reflect the Python/API teardown flow; in
packages/challenges/agent-challenge/docs/validator/self-deploy.md lines 281-282,
replace the `HTTP DELETE /cvms/{id}` entry inside the bash fence with the
executable `python -m agent_challenge.selfdeploy ... teardown` command,
preserving the documented ID/app resolution behavior.
Comment on lines
+3
to
+25
| "note": "SIDE manifest (NOT the frozen golden dataset digest). Maps a small deterministic subset of Terminal-Bench 2.1 task_ids to PULLABLE, digest-pinned GHCR refs under ghcr.io/baseintelligence so an in-CVM DooD orchestrator can pull them for a live smoke E2E. D6: only GHCR shipping refs are accepted (see agent_challenge.canonical.live_registry). This file does NOT affect golden/dataset-digest.json, its per-task content_digest_sha256, the canonical_content_digest_sha256, or the canonical compose/measurement. Resolution is opt-in and fail-closed. OPS_REQUIRED: orchestrator_image digest is the T1 local buildx manifest (not yet pushed to GHCR); task image digests retain prior content hashes under the new GHCR path pattern and must be republished to GHCR before live pull succeeds.", | ||
| "dataset": "terminal-bench/terminal-bench-2-1", | ||
| "namespace": "docker.io/mathiiss", | ||
| "orchestrator_image": "docker.io/mathiiss/agent-challenge-canonical@sha256:02331f0909f617e333f113be376d353770a673669946bcddaac3c53cbde7c9d8", | ||
| "orchestrator_image_source": "services.yaml build-canonical (`uv run python -m agent_challenge.canonical.build --build`) built reproducibly (BuildKit SOURCE_DATE_EPOCH + rewrite-timestamp, provenance/sbom off) and pushed to the miner Docker Hub namespace", | ||
| "namespace": "ghcr.io/baseintelligence", | ||
| "orchestrator_image": "ghcr.io/baseintelligence/agent-challenge-canonical@sha256:ea399cee1b3c9015024918a6070901e6b7b4bee432c8afe1e7dd40a95547e0bc", | ||
| "orchestrator_image_source": "T1 local agent-recipe buildx (SOURCE_DATE_EPOCH + rewrite-timestamp, provenance/sbom off) → manifest sha256:ea399cee1b3c9015024918a6070901e6b7b4bee432c8afe1e7dd40a95547e0bc; tag was ghcr.io/baseintelligence/agent-challenge-canonical:t1-local. OPS_REQUIRED: replace with the first published GHCR digest from agent-recipe publish-eval-image.yml before T3/prod pin.", | ||
| "tasks": { | ||
| "adaptive-rejection-sampler": { | ||
| "registry_ref": "docker.io/mathiiss/agent-challenge-tb21-adaptive-rejection-sampler@sha256:7c8bd5835f19506222805de68d65f83d6cca5b502f7d71dc5e2d9d4dd447c0c8", | ||
| "registry_ref": "ghcr.io/baseintelligence/agent-challenge-tb21-adaptive-rejection-sampler@sha256:7c8bd5835f19506222805de68d65f83d6cca5b502f7d71dc5e2d9d4dd447c0c8", | ||
| "source_ref": "alexgshaw/adaptive-rejection-sampler:20251031", | ||
| "content_digest_sha256": "bcaa2399985cd57666018025846289ab25e193ae0dd8fb7f0ffab2410c24d4de" | ||
| "content_digest_sha256": "bcaa2399985cd57666018025846289ab25e193ae0dd8fb7f0ffab2410c24d4de", | ||
| "ops_status": "BLOCKED_REPUBLISH — path retargeted to GHCR; digest is historical content pin until image is pushed under ghcr.io/baseintelligence/" | ||
| }, | ||
| "bn-fit-modify": { | ||
| "registry_ref": "docker.io/mathiiss/agent-challenge-tb21-bn-fit-modify@sha256:c0371862a0861f282eb206471452ea9aa8d494e3687bdc8167c96ab39d73db50", | ||
| "registry_ref": "ghcr.io/baseintelligence/agent-challenge-tb21-bn-fit-modify@sha256:c0371862a0861f282eb206471452ea9aa8d494e3687bdc8167c96ab39d73db50", | ||
| "source_ref": "alexgshaw/bn-fit-modify:20251031", | ||
| "content_digest_sha256": "b5f9644970c17ad9ddb46b7266f7bcd87c761d77d7e6f55d7cfe7284d5ff66e9" | ||
| "content_digest_sha256": "b5f9644970c17ad9ddb46b7266f7bcd87c761d77d7e6f55d7cfe7284d5ff66e9", | ||
| "ops_status": "BLOCKED_REPUBLISH — path retargeted to GHCR; digest is historical content pin until image is pushed under ghcr.io/baseintelligence/" | ||
| }, | ||
| "break-filter-js-from-html": { | ||
| "registry_ref": "docker.io/mathiiss/agent-challenge-tb21-break-filter-js-from-html@sha256:2a5bd51bab582993befc1a24252c03af673cb1db7cf7d0b2195d66a42a3872a7", | ||
| "registry_ref": "ghcr.io/baseintelligence/agent-challenge-tb21-break-filter-js-from-html@sha256:2a5bd51bab582993befc1a24252c03af673cb1db7cf7d0b2195d66a42a3872a7", | ||
| "source_ref": "alexgshaw/break-filter-js-from-html:20251031", | ||
| "content_digest_sha256": "678008d1a4fd1e6e1b9b3cc9a327219fe4b410a31eafc52e9099bbf947eea600" | ||
| "content_digest_sha256": "678008d1a4fd1e6e1b9b3cc9a327219fe4b410a31eafc52e9099bbf947eea600", | ||
| "ops_status": "BLOCKED_REPUBLISH — path retargeted to GHCR; digest is historical content pin until image is pushed under ghcr.io/baseintelligence/" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Do not ship known-unavailable live image pins.
Line 7 states the orchestrator image is not pushed to GHCR, and Lines 13, 19, and 25 state task images still require republishing. These refs nevertheless pass parse_live_registry() and reach the live Docker-pull path, so any opt-in live run fails during image resolution rather than failing validation. Publish/copy the images first and replace these with their actual GHCR manifest digests, or omit unavailable entries until then.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/challenges/agent-challenge/golden/live-registry-refs.json` around
lines 3 - 25, Update the live registry manifest so parse_live_registry() accepts
only images currently available in GHCR: publish/copy the orchestrator and task
images, then replace their registry_ref and orchestrator_image values with the
resulting manifest digests, or remove unavailable entries until publication is
complete. Remove stale OPS_REQUIRED/BLOCKED_REPUBLISH references from shipped
entries.
Comment on lines
+159
to
+196
| allowed_tasks = {item["task_id"] for item in plan["selected_tasks"]} | ||
| if validated["task_id"] not in allowed_tasks: | ||
| raise EvalProgressError( | ||
| "progress task_id is not in the eval plan", | ||
| code="progress_task_unknown", | ||
| ) | ||
|
|
||
| existing = await _existing_progress_event( | ||
| session, | ||
| submission_id=run.submission_id, | ||
| eval_run_id=run.eval_run_id, | ||
| task_id=validated["task_id"], | ||
| client_sequence=validated["sequence"], | ||
| ) | ||
| if existing is not None: | ||
| return ( | ||
| _progress_receipt( | ||
| eval_run_id=run.eval_run_id, | ||
| task_id=validated["task_id"], | ||
| sequence=validated["sequence"], | ||
| event_id=existing.id, | ||
| created=False, | ||
| ), | ||
| False, | ||
| ) | ||
|
|
||
| highest = await _max_client_sequence( | ||
| session, | ||
| submission_id=run.submission_id, | ||
| eval_run_id=run.eval_run_id, | ||
| task_id=validated["task_id"], | ||
| ) | ||
| if validated["sequence"] <= highest: | ||
| raise EvalProgressError( | ||
| "progress sequence must be monotone per task", | ||
| code="progress_sequence_stale", | ||
| ) | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Check-then-act dedup is racy under concurrent duplicate submissions.
_existing_progress_event and _max_client_sequence are read checks with no locking, followed by an unguarded record_task_event insert. Two concurrent requests for the same (eval_run_id, task_id, sequence) — e.g. a client retry after a timed-out first attempt — can both see "no existing event," both pass the monotonicity check, and both insert, breaking the idempotent-replay guarantee stated in the docstring. There's no DB-level unique constraint available here since eval_run_id/sequence live inside metadata_json, not indexed columns.
Consider serializing progress writes per run, e.g. by locking the EvalRun row (similar to the with_for_update() pattern already used in direct_result.py's process_direct_eval_result) before the check-then-insert sequence, or adding a dedicated indexed uniqueness mechanism.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/progress.py`
around lines 159 - 196, Serialize the progress check-and-insert flow by locking
the relevant EvalRun row with the existing transaction locking pattern before
calling _existing_progress_event and _max_client_sequence. Update the
surrounding progress submission function so the lock is acquired before both
reads and remains held through record_task_event, preserving idempotent retries
and monotone sequencing for concurrent submissions.
Comment on lines
+1037
to
+1052
| def _write_eval_run_token_file(path: str, token: str) -> None: | ||
| """Write the one-time run token to PATH with mode 0o600 (create securely).""" | ||
|
|
||
| # O_CREAT|O_WRONLY|O_TRUNC with mode 0o600 — never create world-readable then chmod. | ||
| fd = os.open( | ||
| path, | ||
| os.O_CREAT | os.O_WRONLY | os.O_TRUNC, | ||
| 0o600, | ||
| ) | ||
| try: | ||
| with os.fdopen(fd, "w", encoding="utf-8") as handle: | ||
| handle.write(token) | ||
| fd = -1 # fdopen owns it | ||
| finally: | ||
| if fd >= 0: | ||
| os.close(fd) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Double-close on write failure masks the real error.
fd = -1 is only assigned after handle.write(token) succeeds. If the write raises, the with block closes the descriptor and then finally closes it again, surfacing OSError: EBADF instead of the original failure. Transfer ownership before writing.
🐛 Proposed fix
try:
with os.fdopen(fd, "w", encoding="utf-8") as handle:
+ fd = -1 # fdopen owns it
handle.write(token)
- fd = -1 # fdopen owns it
finally:
if fd >= 0:
os.close(fd)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _write_eval_run_token_file(path: str, token: str) -> None: | |
| """Write the one-time run token to PATH with mode 0o600 (create securely).""" | |
| # O_CREAT|O_WRONLY|O_TRUNC with mode 0o600 — never create world-readable then chmod. | |
| fd = os.open( | |
| path, | |
| os.O_CREAT | os.O_WRONLY | os.O_TRUNC, | |
| 0o600, | |
| ) | |
| try: | |
| with os.fdopen(fd, "w", encoding="utf-8") as handle: | |
| handle.write(token) | |
| fd = -1 # fdopen owns it | |
| finally: | |
| if fd >= 0: | |
| os.close(fd) | |
| def _write_eval_run_token_file(path: str, token: str) -> None: | |
| """Write the one-time run token to PATH with mode 0o600 (create securely).""" | |
| # O_CREAT|O_WRONLY|O_TRUNC with mode 0o600 — never create world-readable then chmod. | |
| fd = os.open( | |
| path, | |
| os.O_CREAT | os.O_WRONLY | os.O_TRUNC, | |
| 0o600, | |
| ) | |
| try: | |
| with os.fdopen(fd, "w", encoding="utf-8") as handle: | |
| fd = -1 # fdopen owns it | |
| handle.write(token) | |
| finally: | |
| if fd >= 0: | |
| os.close(fd) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.py`
around lines 1037 - 1052, Update _write_eval_run_token_file so file-descriptor
ownership transfers to fdopen before handle.write(token) executes. Ensure the
finally block cannot close the descriptor after the context manager has closed
it, while preserving cleanup if fdopen itself fails and allowing the original
write exception to propagate.
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
1 participant
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.
Summary
Hardens agent-challenge Phala self-deploy for production eval/review CVMs:
tdx.xlargeeval, disk billing in lifecycle budget)phalabinary (DELETE + unique app_id resolve)--token-output/ minerEVAL_RUN_TOKEN) with prepare-token recoveryLive proof (master prod)
Full attested E2E on master prod completed after this branch work:
review_verified/ alloweval_deployed, shapetdx.xlarge, plan rtmr0ec216f1d…score_quote_ok, envelope rtmr0 matches planverified=trueterminal=truereceipt_5b8d5c6953744ba985f9289201700e6ceval_9b25a9f0bdf948eebd551f46366650c1(submission 10)Evidence dir on master:
/var/lib/base/e2e/ac-verified-20260728T022856Z/(VERIFIED_OK, canonical envelope, status).Ops note:
eval resultmust post compact JSON bytes (pretty-printed body → HTTP 422result_invalid). Guest logs are on Phalaprod5container channeldstack-orchestrator-1(base64 JSON lines), not serial.Post-merge ops (done on master, not in this PR)
PRISM_CHECKPOINT_UPLOAD_ENABLED=false+ publisher guard hotpatchTest plan
Summary by CodeRabbit