feat(agent-challenge): NO_PHALA host pipeline with unattested weight push - #52
Conversation
Opt-in host execution while Phala CVMs are disabled. Results are explicitly marked unattested and cannot be forged as attested. Attested path stays byte-identical when the flag is off. Contradiction with attestation flags fails closed at startup.
… push Drive the dual-flag-off analysis+own_runner path under NO_PHALA through scores and authenticated raw-weight push, with unattested provenance on completion metadata and CRITICAL push logs. Attested gates stay untouched.
📝 WalkthroughWalkthroughAdds an explicit ChangesNO_PHALA execution contract
Result marking and unattested evaluation
Raw-weight delivery
Configuration and database
Application lifecycle
OpenRouter LLM provider
Estimated code review effort: 4 (Complex) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant AgentChallenge as Agent Challenge App
participant RawWeightPushLoop as Push Loop Task
participant RawWeightStore as Durable Store
participant Master as Master Endpoint
AgentChallenge->>AgentChallenge: startup: create_app()
AgentChallenge->>RawWeightPushLoop: start background task
loop Every 30s or configured interval
RawWeightPushLoop->>RawWeightStore: get pending or fresh epoch
RawWeightPushLoop->>RawWeightStore: record_pending(payload, epoch, digest)
RawWeightPushLoop->>Master: POST /raw-weights (signed)
alt Exact match acknowledged
Master-->>RawWeightPushLoop: {accepted:true, epoch, revision, digest, snapshot_id}
RawWeightPushLoop->>RawWeightStore: acknowledge() → advance cursor
else Mismatch or error
Master-->>RawWeightPushLoop: error or digest mismatch
RawWeightPushLoop->>RawWeightStore: keep pending for retry
end
end
AgentChallenge->>RawWeightPushLoop: cancel during shutdown
🚥 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
🧹 Nitpick comments (4)
packages/challenges/agent-challenge/tests/conftest.py (1)
68-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the blanket try/except now that the ledger table is always created.
Database.init()importsagent_challenge.evaluation.raw_weight_push(andcore/models.pyre-exports the model), soraw_weight_push_ledgeris always inBase.metadatabeforecreate_all. Swallowing the error here also masks a worse failure mode: a statement error inside the openengine.begin()transaction leaves the connection needing a rollback, so the followingdelete(...)calls would fail with a confusingPendingRollbackErrorrather than the original cause.♻️ Proposed simplification
- # Optional table (registered when raw_weight_push is imported). - try: - await connection.exec_driver_sql("DELETE FROM raw_weight_push_ledger") - except Exception: - pass # table created on first Database.init after raw_weight_push import + await connection.exec_driver_sql("DELETE FROM raw_weight_push_ledger")🤖 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/conftest.py` around lines 68 - 72, Remove the blanket try/except around the raw_weight_push_ledger deletion in the database cleanup fixture. Since Database.init() always registers the model before schema creation, execute the DELETE directly and allow any statement or transaction error to propagate; leave the subsequent cleanup deletes unchanged.packages/challenges/agent-challenge/src/agent_challenge/sdk/app_factory.py (1)
178-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTask naming hardcodes a caller's private closure name.
_background_task_namematches the literal_run_raw_weight_push, the name of a nested closure inagent_challenge/api/app.py. Renaming that closure silently downgrades the task name tochallenge-background-taskand breakstest_raw_weight_push_lifespan.py, which asserts onraw-weight-push-loop. Prefer letting callers supply the name (e.g. accept(factory, name)pairs, or read an explicitfactory.task_nameattribute) instead of reverse-coupling the SDK to a caller's internal identifier.🤖 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/sdk/app_factory.py` around lines 178 - 184, The _background_task_name function is coupled to the caller’s private _run_raw_weight_push closure name. Remove this literal-name matching and update the task-registration flow to accept or read an explicit task name from the caller, preserving raw-weight-push-loop for that task while retaining the default challenge-background-task name when no name is supplied.packages/challenges/agent-challenge/tests/test_ac_raw_weight_push.py (1)
189-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the durable-retry path.
The suite verifies ack matching and mismatch, but not the headline promise in the module docstring: after a timeout/transport error, the pending row is retained and the next
push_oncere-sends the exact same bytes (same epoch/revision/digest) rather than minting a new snapshot. Worth one test with a transport that raiseshttpx.ConnectTimeoutfirst, then acks.🤖 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_ac_raw_weight_push.py` around lines 189 - 230, Add an async test alongside test_mismatched_ack_digest_does_not_advance_cursor covering the durable retry path: configure the transport to raise httpx.ConnectTimeout on the first request, then return a valid matching acknowledgment; assert the pending row is retained after the failure and that the next push_once succeeds while reusing the exact same serialized payload, epoch, revision, and payload digest rather than creating a new snapshot.packages/challenges/agent-challenge/src/agent_challenge/evaluation/no_phala.py (1)
85-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnrecognized boolean tokens silently resolve to "off" with no warning.
_parse_bool_envreturnsNonefor unrecognized tokens, and callers dobool(parsed)→False. An operator typo (e.g.NO_PHALA=tru) silently disables the mode instead of surfacing a misconfiguration. The direction is safe (fails to "off"), but a warning would help operators notice the typo instead of assuming NO_PHALA is active when it silently isn't.♻️ Proposed enhancement
def resolve_no_phala_from_environ( environ: Mapping[str, str] | None = None, ) -> bool: env = os.environ if environ is None else environ if CHALLENGE_NO_PHALA_ENV in env: parsed = _parse_bool_env(env.get(CHALLENGE_NO_PHALA_ENV)) + if parsed is None: + logger.warning( + "%s=%r is not a recognized boolean token; treating as off", + CHALLENGE_NO_PHALA_ENV, env.get(CHALLENGE_NO_PHALA_ENV), + ) return bool(parsed) if NO_PHALA_ENV in env: parsed = _parse_bool_env(env.get(NO_PHALA_ENV)) + if parsed is None: + logger.warning( + "%s=%r is not a recognized boolean token; treating as off", + NO_PHALA_ENV, env.get(NO_PHALA_ENV), + ) return bool(parsed) return False🤖 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/no_phala.py` around lines 85 - 96, Update _parse_bool_env and its caller so unrecognized non-empty boolean tokens emit a warning identifying the invalid value before preserving the current safe false/off behavior. Keep recognized tokens and unset values unchanged, and reuse the module’s existing logging mechanism rather than adding a new one.
🤖 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/src/agent_challenge/evaluation/own_runner_backend.py`:
- Around line 1207-1217: Update the NO_PHALA branch in _emit_job_result to catch
ArtifactProvenanceError from _maybe_no_phala_artifact_proof or
mark_result_unattested, then use the existing fail-closed result emission path
(emit_failclosed_result) so a genuine ZIP hash mismatch still emits a result
line instead of propagating. Preserve the current successful artifact-proof flow
and mirror the attested path’s error handling.
- Around line 626-661: Update _maybe_no_phala_artifact_proof so it returns None
when PHALA_AGENT_HASH_ENV is unset or blank, instead of assigning executed as
expected. Only call build_guest_artifact_proof when an independently declared
expected hash is available; preserve the existing artifact and digest validation
behavior otherwise.
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/raw_weight_push.py`:
- Around line 374-384: Update the pending-payload handling around
RawWeightPushRequest validation to inspect the restored payload’s freshness
timestamps before assigning it to raw_bytes. If the pending payload is expired,
discard it by clearing pending, payload, and raw_bytes so the normal path
rebuilds a fresh revision; otherwise preserve the existing byte-for-byte retry
behavior for still-fresh pending payloads.
- Around line 418-422: Update the revision selection in the rebuild path around
_next_revision so that, when rebuilding a discarded or corrupted pending entry
in the same epoch, the next revision is seeded from the latest outstanding
revision. Use pending.revision together with cursor.revision (taking the
maximum) before recording the new pending payload, while preserving
force_revision behavior.
---
Nitpick comments:
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/no_phala.py`:
- Around line 85-96: Update _parse_bool_env and its caller so unrecognized
non-empty boolean tokens emit a warning identifying the invalid value before
preserving the current safe false/off behavior. Keep recognized tokens and unset
values unchanged, and reuse the module’s existing logging mechanism rather than
adding a new one.
In `@packages/challenges/agent-challenge/src/agent_challenge/sdk/app_factory.py`:
- Around line 178-184: The _background_task_name function is coupled to the
caller’s private _run_raw_weight_push closure name. Remove this literal-name
matching and update the task-registration flow to accept or read an explicit
task name from the caller, preserving raw-weight-push-loop for that task while
retaining the default challenge-background-task name when no name is supplied.
In `@packages/challenges/agent-challenge/tests/conftest.py`:
- Around line 68-72: Remove the blanket try/except around the
raw_weight_push_ledger deletion in the database cleanup fixture. Since
Database.init() always registers the model before schema creation, execute the
DELETE directly and allow any statement or transaction error to propagate; leave
the subsequent cleanup deletes unchanged.
In `@packages/challenges/agent-challenge/tests/test_ac_raw_weight_push.py`:
- Around line 189-230: Add an async test alongside
test_mismatched_ack_digest_does_not_advance_cursor covering the durable retry
path: configure the transport to raise httpx.ConnectTimeout on the first
request, then return a valid matching acknowledgment; assert the pending row is
retained after the failure and that the next push_once succeeds while reusing
the exact same serialized payload, epoch, revision, and payload digest rather
than creating a new snapshot.
🪄 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: 07a166ef-7110-4050-b3ff-3e9497c11489
📒 Files selected for processing (20)
packages/challenges/agent-challenge/docs/no-phala-mode.mdpackages/challenges/agent-challenge/src/agent_challenge/api/app.pypackages/challenges/agent-challenge/src/agent_challenge/core/models.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/no_phala.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/own_runner_backend.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/raw_weight_push.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/runner.pypackages/challenges/agent-challenge/src/agent_challenge/sdk/app_factory.pypackages/challenges/agent-challenge/src/agent_challenge/sdk/config.pypackages/challenges/agent-challenge/src/agent_challenge/sdk/db.pypackages/challenges/agent-challenge/src/agent_challenge/sdk/schemas.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/eval.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/phala.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/review.pypackages/challenges/agent-challenge/tests/conftest.pypackages/challenges/agent-challenge/tests/test_ac_raw_weight_push.pypackages/challenges/agent-challenge/tests/test_health.pypackages/challenges/agent-challenge/tests/test_no_phala_mode.pypackages/challenges/agent-challenge/tests/test_no_phala_pipeline.pypackages/challenges/agent-challenge/tests/test_raw_weight_push_lifespan.py
| def _maybe_no_phala_artifact_proof() -> dict[str, str | bool] | None: | ||
| """Build guest_artifact_proof for NO_PHALA host runs when ZIP is available. | ||
|
|
||
| Returns None when the artifact path is missing (caller still marks | ||
| unattested without the triple). When the path is present, enforces | ||
| expected == download == executed against the plan/declared agent hash. | ||
| """ | ||
|
|
||
| from agent_challenge.evaluation.no_phala import ( | ||
| ArtifactProvenanceError, | ||
| build_guest_artifact_proof, | ||
| ) | ||
|
|
||
| artifact_path = resolve_agent_artifact_path() | ||
| if artifact_path is None or not Path(artifact_path).is_file(): | ||
| return None | ||
| try: | ||
| executed = agent_artifact_sha256(artifact_path) | ||
| except ValueError: | ||
| return None | ||
| expected = (os.environ.get(PHALA_AGENT_HASH_ENV) or "").strip().lower() | ||
| if not expected: | ||
| # No plan hash declared — use the executed digest as the expected pin | ||
| # so provenance still records download==executed for the host ZIP. | ||
| expected = executed | ||
| download = executed # host-local: download path is the same file we execute | ||
| try: | ||
| return build_guest_artifact_proof( | ||
| expected_hash=expected, | ||
| download_hash=download, | ||
| executed_hash=executed, | ||
| ) | ||
| except ArtifactProvenanceError: | ||
| raise | ||
|
|
||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Trivial guest_artifact_proof when no declared hash is available.
When CHALLENGE_PHALA_AGENT_HASH is unset, expected/download/executed are all set to the same locally-computed digest, so the built proof always shows match: true without checking against anything independent. This could misrepresent an unverified artifact as provenance-checked. Consider omitting guest_artifact_proof entirely (return None) when no independently-declared hash exists, rather than emitting a self-referential trivial match.
🛡️ Proposed fix
expected = (os.environ.get(PHALA_AGENT_HASH_ENV) or "").strip().lower()
if not expected:
- # No plan hash declared — use the executed digest as the expected pin
- # so provenance still records download==executed for the host ZIP.
- expected = executed
+ # No independently-declared plan hash — there is nothing to verify
+ # against, so omit the proof rather than emit a trivially-true match.
+ return None
download = executed # host-local: download path is the same file we execute📝 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.
| def _maybe_no_phala_artifact_proof() -> dict[str, str | bool] | None: | |
| """Build guest_artifact_proof for NO_PHALA host runs when ZIP is available. | |
| Returns None when the artifact path is missing (caller still marks | |
| unattested without the triple). When the path is present, enforces | |
| expected == download == executed against the plan/declared agent hash. | |
| """ | |
| from agent_challenge.evaluation.no_phala import ( | |
| ArtifactProvenanceError, | |
| build_guest_artifact_proof, | |
| ) | |
| artifact_path = resolve_agent_artifact_path() | |
| if artifact_path is None or not Path(artifact_path).is_file(): | |
| return None | |
| try: | |
| executed = agent_artifact_sha256(artifact_path) | |
| except ValueError: | |
| return None | |
| expected = (os.environ.get(PHALA_AGENT_HASH_ENV) or "").strip().lower() | |
| if not expected: | |
| # No plan hash declared — use the executed digest as the expected pin | |
| # so provenance still records download==executed for the host ZIP. | |
| expected = executed | |
| download = executed # host-local: download path is the same file we execute | |
| try: | |
| return build_guest_artifact_proof( | |
| expected_hash=expected, | |
| download_hash=download, | |
| executed_hash=executed, | |
| ) | |
| except ArtifactProvenanceError: | |
| raise | |
| def _maybe_no_phala_artifact_proof() -> dict[str, str | bool] | None: | |
| """Build guest_artifact_proof for NO_PHALA host runs when ZIP is available. | |
| Returns None when the artifact path is missing (caller still marks | |
| unattested without the triple). When the path is present, enforces | |
| expected == download == executed against the plan/declared agent hash. | |
| """ | |
| from agent_challenge.evaluation.no_phala import ( | |
| ArtifactProvenanceError, | |
| build_guest_artifact_proof, | |
| ) | |
| artifact_path = resolve_agent_artifact_path() | |
| if artifact_path is None or not Path(artifact_path).is_file(): | |
| return None | |
| try: | |
| executed = agent_artifact_sha256(artifact_path) | |
| except ValueError: | |
| return None | |
| expected = (os.environ.get(PHALA_AGENT_HASH_ENV) or "").strip().lower() | |
| if not expected: | |
| # No independently-declared plan hash — there is nothing to verify | |
| # against, so omit the proof rather than emit a trivially-true match. | |
| return None | |
| download = executed # host-local: download path is the same file we execute | |
| try: | |
| return build_guest_artifact_proof( | |
| expected_hash=expected, | |
| download_hash=download, | |
| executed_hash=executed, | |
| ) | |
| except ArtifactProvenanceError: | |
| raise |
🤖 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/own_runner_backend.py`
around lines 626 - 661, Update _maybe_no_phala_artifact_proof so it returns None
when PHALA_AGENT_HASH_ENV is unset or blank, instead of assigning executed as
expected. Only call build_guest_artifact_proof when an independently declared
expected hash is available; preserve the existing artifact and digest validation
behavior otherwise.
| # Temporary NO_PHALA host mode: mark the legacy line unattested and | ||
| # attach guest_artifact_proof when the agent ZIP provenance is known. | ||
| # Attested path below is untouched when NO_PHALA is off. | ||
| from agent_challenge.evaluation.no_phala import ( | ||
| is_no_phala_enabled, | ||
| mark_result_unattested, | ||
| ) | ||
|
|
||
| if is_no_phala_enabled(): | ||
| artifact_proof = _maybe_no_phala_artifact_proof() | ||
| payload = mark_result_unattested(payload, artifact_proof=artifact_proof) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unhandled ArtifactProvenanceError crashes the NO_PHALA emit path on a genuine hash mismatch.
Unlike the attested path below (which catches AttestationEmissionError and calls emit_failclosed_result(...) for a graceful fail-closed result), a genuine ZIP hash mismatch here propagates an uncaught ArtifactProvenanceError out of _emit_job_result, so no result line is ever emitted for the exact case this mechanism is meant to catch.
🐛 Proposed fix
+ from agent_challenge.evaluation.no_phala import ArtifactProvenanceError
from agent_challenge.evaluation.no_phala import (
is_no_phala_enabled,
mark_result_unattested,
)
if is_no_phala_enabled():
- artifact_proof = _maybe_no_phala_artifact_proof()
- payload = mark_result_unattested(payload, artifact_proof=artifact_proof)
+ try:
+ artifact_proof = _maybe_no_phala_artifact_proof()
+ except ArtifactProvenanceError as exc:
+ logger.critical("NO_PHALA artifact provenance mismatch: %s", exc)
+ emit_failclosed_result(total=len(list(task_ids)))
+ return 1
+ payload = mark_result_unattested(payload, artifact_proof=artifact_proof)
emit_benchmark_result_line(payload)
return 0📝 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.
| # Temporary NO_PHALA host mode: mark the legacy line unattested and | |
| # attach guest_artifact_proof when the agent ZIP provenance is known. | |
| # Attested path below is untouched when NO_PHALA is off. | |
| from agent_challenge.evaluation.no_phala import ( | |
| is_no_phala_enabled, | |
| mark_result_unattested, | |
| ) | |
| if is_no_phala_enabled(): | |
| artifact_proof = _maybe_no_phala_artifact_proof() | |
| payload = mark_result_unattested(payload, artifact_proof=artifact_proof) | |
| from agent_challenge.evaluation.no_phala import ArtifactProvenanceError | |
| from agent_challenge.evaluation.no_phala import ( | |
| is_no_phala_enabled, | |
| mark_result_unattested, | |
| ) | |
| if is_no_phala_enabled(): | |
| try: | |
| artifact_proof = _maybe_no_phala_artifact_proof() | |
| except ArtifactProvenanceError as exc: | |
| logger.critical("NO_PHALA artifact provenance mismatch: %s", exc) | |
| emit_failclosed_result(total=len(list(task_ids))) | |
| return 1 | |
| payload = mark_result_unattested(payload, artifact_proof=artifact_proof) | |
| emit_benchmark_result_line(payload) | |
| return 0 |
🤖 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/own_runner_backend.py`
around lines 1207 - 1217, Update the NO_PHALA branch in _emit_job_result to
catch ArtifactProvenanceError from _maybe_no_phala_artifact_proof or
mark_result_unattested, then use the existing fail-closed result emission path
(emit_failclosed_result) so a genuine ZIP hash mismatch still emits a result
line instead of propagating. Preserve the current successful artifact-proof flow
and mirror the attested path’s error handling.
| if pending is not None: | ||
| # Retry exact previous bytes after timeout/restart (no new revision). | ||
| try: | ||
| pending_bytes = str(pending["canonical_payload"]).encode("utf-8") | ||
| payload = RawWeightPushRequest.model_validate_json(pending_bytes) | ||
| raw_bytes = payload.canonical_bytes() | ||
| except Exception: # noqa: BLE001 - corrupt pending is rebuilt | ||
| pending = None | ||
| payload = None | ||
| raw_bytes = None | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Stale pending payload is retried forever after its freshness window expires.
The pending payload is replayed byte-for-byte, so computed_at/expires_at stay at the original values. Once more than freshness_seconds (default 300s) of failures/downtime elapse, every retry posts an already-expired snapshot that master will reject on freshness, and pending is only cleared on a successful ack — delivery is wedged permanently. Add a staleness escape so an expired pending is discarded and rebuilt with a fresh revision.
🛠️ Proposed staleness escape
if pending is not None:
# Retry exact previous bytes after timeout/restart (no new revision).
try:
pending_bytes = str(pending["canonical_payload"]).encode("utf-8")
payload = RawWeightPushRequest.model_validate_json(pending_bytes)
+ expires_at = datetime.fromisoformat(
+ str(payload.expires_at).replace("Z", "+00:00")
+ )
+ if expires_at <= now:
+ # Expired snapshot would be rejected on freshness forever.
+ raise ValueError("pending payload expired")
raw_bytes = payload.canonical_bytes()
- except Exception: # noqa: BLE001 - corrupt pending is rebuilt
+ except Exception: # noqa: BLE001 - corrupt/expired pending is rebuilt
pending = None
payload = None
raw_bytes = None📝 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.
| if pending is not None: | |
| # Retry exact previous bytes after timeout/restart (no new revision). | |
| try: | |
| pending_bytes = str(pending["canonical_payload"]).encode("utf-8") | |
| payload = RawWeightPushRequest.model_validate_json(pending_bytes) | |
| raw_bytes = payload.canonical_bytes() | |
| except Exception: # noqa: BLE001 - corrupt pending is rebuilt | |
| pending = None | |
| payload = None | |
| raw_bytes = None | |
| if pending is not None: | |
| # Retry exact previous bytes after timeout/restart (no new revision). | |
| try: | |
| pending_bytes = str(pending["canonical_payload"]).encode("utf-8") | |
| payload = RawWeightPushRequest.model_validate_json(pending_bytes) | |
| expires_at = datetime.fromisoformat( | |
| str(payload.expires_at).replace("Z", "+00:00") | |
| ) | |
| if expires_at <= now: | |
| # Expired snapshot would be rejected on freshness forever. | |
| raise ValueError("pending payload expired") | |
| raw_bytes = payload.canonical_bytes() | |
| except Exception: # noqa: BLE001 - corrupt/expired pending is rebuilt | |
| pending = None | |
| payload = None | |
| raw_bytes = None |
🤖 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/raw_weight_push.py`
around lines 374 - 384, Update the pending-payload handling around
RawWeightPushRequest validation to inspect the restored payload’s freshness
timestamps before assigning it to raw_bytes. If the pending payload is expired,
discard it by clearing pending, payload, and raw_bytes so the normal path
rebuilds a fresh revision; otherwise preserve the existing byte-for-byte retry
behavior for still-fresh pending payloads.
| revision = ( | ||
| int(force_revision) | ||
| if force_revision is not None | ||
| else self._next_revision(cursor, resolved_epoch) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Prism's equivalent revision logic for how pending revisions are handled.
fd -t f 'raw_weight_push*.py' packages/challenges/prism --exec cat -n {}Repository: BaseIntelligence/base
Length of output: 39895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the Agent Challenge client implementation plus its revision/pending tests.
fd -t f 'raw_weight_push.*\.py|test.*raw.*weight|.*weight.*test.*\.py' packages/challenges/agent-challenge -x sh -c '
echo "===== FILE: $1 ====="
wc -l "$1"
rg -n "_next_revision|record_pending|get_pending|push_once|pending" "$1"
' sh {}
echo "===== candidate files ====="
fd -t f 'raw_weight_push.*\.py|test.*raw.*weight|.*weight.*test.*\.py' packages/challenges/agent-challenge
echo "===== focused agent file outline/sections ====="
agent_file=$(fd -t f 'raw_weight_push.py' packages/challenges/agent-challenge | head -n1)
if [ -n "${agent_file:-}" ]; then
ast-grep outline "$agent_file" || true
sed -n '240,445p' "$agent_file" | cat -n
fiRepository: BaseIntelligence/base
Length of output: 15126
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
class PushCursor:
def __init__(self, epoch, revision):
self.epoch = epoch
self.revision = revision
def next_revision(cursor, epoch):
if cursor is None:
return 1
if cursor.epoch == epoch:
return cursor.revision + 1
return 1
cursor = PushCursor(epoch=10, revision=5)
pending = {"epoch": 10, "revision": 6, "payload_digest": "aaa"}
revision = next_revision(cursor, 10)
# A rejected/discard-like rebuild that does not persist pending would record:
rebuild = {"epoch": 10, "revision": revision, "payload_digest": "bbb"}
# A later retry with unchanged persisted pending state reuses pending if not rebuilt.
retry = pending
print({"cursor": (cursor.epoch, cursor.revision), "pending_before_rebuild": (pending["epoch"], pending["revision"], pending["payload_digest"]), "rebuilt": (rebuild["epoch"], rebuild["revision"], rebuild["payload_digest"]), "retry_reuses": (retry["epoch"], retry["revision"])})
PYRepository: BaseIntelligence/base
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in Path("packages/challenges/agent-challenge").rglob("test_ac_raw_weight_push.py"):
text = p.read_text()
print(f"===== {p} =====")
for needle in ["build_corrupt_pending", "discard_pending_identity", "corrupt pending", "_next_revision", "revision"]:
idx = text.find(needle)
print(f"{needle}: {idx}")
start = text.find("build_corrupt_pending")
if start == -1:
start = text.find("discard_pending_identity")
if start != -1:
end = text.find("\n def ", start + 1)
print(text[start: end if end != -1 else start + 2000])
PYRepository: BaseIntelligence/base
Length of output: 349
Seed rebuilt revisions from the latest outstanding revision.
When pending is discarded/corrupted and rebuilt in the same epoch, _next_revision only sees the acknowledged cursor, so it reuses the same revision number with a different digest. Seed the rebuilt revision from pending.revision/max(cursor.revision, pending.revision) before recording the new pending payload.
🤖 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/raw_weight_push.py`
around lines 418 - 422, Update the revision selection in the rebuild path around
_next_revision so that, when rebuilding a discarded or corrupted pending entry
in the same epoch, the next revision is seeded from the latest outstanding
revision. Use pending.revision together with cursor.revision (taking the
maximum) before recording the new pending payload, while preserving
force_revision behavior.
Add OpenRouterReviewProvider for analyzer LLM review when NO_PHALA is on, with key resolution (env then opencode auth.json), operator config keys, and fail-closed cost/error handling. Gateway path stays default when mode is off. Document embed.env keys and cover selection/parsing with unit tests.
Production mounts the challenge token via shared_token_file; raw-weight push now resolves it the same way auth does so the loop does not skip. NO_PHALA + cli docker backend runs own_runner in-process to avoid nested DooD path mismatch on master embed. Default epoch_seconds matches master sealer (360s).
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/challenges/agent-challenge/tests/test_raw_weight_push_lifespan.py (1)
208-208: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse an HTTPS fixture URL for the token-bearing client.
Although this test does not send a request today,
RawWeightPushClientstores the shared token with this base URL. Usinghttps://master.testprevents future push-capable tests from transmitting the token over cleartext.Proposed fix
- master_base_url="http://master.test", + master_base_url="https://master.test",🤖 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_raw_weight_push_lifespan.py` at line 208, Update the master_base_url fixture used to construct the token-bearing RawWeightPushClient to use the HTTPS scheme while preserving the existing host and test behavior.Source: Linters/SAST tools
packages/challenges/agent-challenge/src/agent_challenge/analyzer/openrouter_review_provider.py (1)
98-122: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScavenging unrelated local credential stores widens the secret surface.
~/.factory/*.json(and the recursive key hunt) makes the challenge service pick up any developer credential file it happens to find, with no operator intent and no audit trail of which source was used. Recommend limiting resolution to the explicit setting, the two env vars, and at most the documentedopencode/auth.json, and logging only the source name (never the value).🤖 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/analyzer/openrouter_review_provider.py` around lines 98 - 122, Restrict credential resolution to the explicit setting, the two supported environment variables, and the documented opencode/auth.json source; remove the factory-root candidate scanning and recursive JSON key discovery used by _key_from_factory_dir and _key_from_json_file_shallow. Track the selected source and log only its source name, never the credential value, while preserving the existing precedence order.
🤖 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/src/agent_challenge/analyzer/openrouter_review_provider.py`:
- Around line 242-250: Update the response parsing around choices and message so
the first choices element must be a mapping before calling .get or passing it to
_parse_provider_tool_calls. Treat malformed elements such as strings as an empty
message and preserve the existing LlmProviderResponse construction and
fail-closed behavior.
In
`@packages/challenges/agent-challenge/src/agent_challenge/evaluation/runner.py`:
- Around line 1401-1412: Update the NO_PHALA execution path in the runner around
_run_no_phala_host_terminal_bench to forward
_terminal_bench_stream_env(plan.attempt_id), matching the Docker branch’s
per-attempt streaming environment injection and preserving real-time SSE log
ingestion.
- Around line 2078-2097: Update the host-local subprocess setup around env and
cwd construction to prevent validator environment and workspace code from
reaching the host process. Build env from an explicit allowlist of required
control variables plus _terminal_bench_env(miner_env, gateway), without
inheriting all of os.environ; retain BASE_AGENT_PATH, remove the
workspace-derived PYTHONPATH, and run subprocess.run from a neutral directory
while allowing own_runner to import the agent inside its sandbox.
In `@packages/challenges/agent-challenge/src/agent_challenge/sdk/config.py`:
- Around line 372-381: Update the validation_alias for llm_cost_limit_usd to
include the documented CHALLENGE_LLM_COST_LIMIT_USD and CHALLENGE_LLM_COST_LIMIT
environment names, while preserving the existing aliases and default behavior.
In
`@packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py`:
- Around line 37-59: Replace all plaintext secret-like test fixtures with
identifier-only placeholders, preserving each test’s assertions and precedence
behavior. Update
packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py
ranges 37-59, 62-123, 126-189, and 192-254, plus
packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py
range 555-575; use names, digests, or SHAs consistently for environment values,
provider/header fixtures, failure-path keys, gateway tokens, and OpenRouter
tokens.
---
Nitpick comments:
In
`@packages/challenges/agent-challenge/src/agent_challenge/analyzer/openrouter_review_provider.py`:
- Around line 98-122: Restrict credential resolution to the explicit setting,
the two supported environment variables, and the documented opencode/auth.json
source; remove the factory-root candidate scanning and recursive JSON key
discovery used by _key_from_factory_dir and _key_from_json_file_shallow. Track
the selected source and log only its source name, never the credential value,
while preserving the existing precedence order.
In `@packages/challenges/agent-challenge/tests/test_raw_weight_push_lifespan.py`:
- Line 208: Update the master_base_url fixture used to construct the
token-bearing RawWeightPushClient to use the HTTPS scheme while preserving the
existing host and test behavior.
🪄 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: e61a0dab-a80b-4e6a-9369-278ade34910e
📒 Files selected for processing (10)
packages/challenges/agent-challenge/docs/no-phala-mode.mdpackages/challenges/agent-challenge/src/agent_challenge/analyzer/lifecycle.pypackages/challenges/agent-challenge/src/agent_challenge/analyzer/openrouter_review_provider.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/raw_weight_push.pypackages/challenges/agent-challenge/src/agent_challenge/evaluation/runner.pypackages/challenges/agent-challenge/src/agent_challenge/sdk/config.pypackages/challenges/agent-challenge/src/agent_challenge/selfdeploy/cli.pypackages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.pypackages/challenges/agent-challenge/tests/test_openrouter_review_provider.pypackages/challenges/agent-challenge/tests/test_raw_weight_push_lifespan.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/challenges/agent-challenge/docs/no-phala-mode.md
- packages/challenges/agent-challenge/src/agent_challenge/evaluation/raw_weight_push.py
| choices = data.get("choices") | ||
| message = choices[0].get("message", {}) if isinstance(choices, list) and choices else {} | ||
| return LlmProviderResponse( | ||
| content=str(message.get("content") or ""), | ||
| tool_calls=_parse_provider_tool_calls(message), | ||
| raw_response=_redacted_response(data), | ||
| usage=data.get("usage") if isinstance(data.get("usage"), Mapping) else None, | ||
| cost=data.get("cost") if isinstance(data.get("cost"), Mapping) else None, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Malformed choices raises AttributeError instead of failing closed.
choices[0] is only checked for list-ness, not element type; a response like {"choices": ["x"]} escapes as AttributeError, bypassing the LlmProvider* classification the reviewer relies on.
🛡️ Proposed fix
- choices = data.get("choices")
- message = choices[0].get("message", {}) if isinstance(choices, list) and choices else {}
+ choices = data.get("choices")
+ first = choices[0] if isinstance(choices, list) and choices else None
+ raw_message = first.get("message") if isinstance(first, Mapping) else None
+ message: Mapping[str, Any] = raw_message if isinstance(raw_message, Mapping) else {}📝 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.
| choices = data.get("choices") | |
| message = choices[0].get("message", {}) if isinstance(choices, list) and choices else {} | |
| return LlmProviderResponse( | |
| content=str(message.get("content") or ""), | |
| tool_calls=_parse_provider_tool_calls(message), | |
| raw_response=_redacted_response(data), | |
| usage=data.get("usage") if isinstance(data.get("usage"), Mapping) else None, | |
| cost=data.get("cost") if isinstance(data.get("cost"), Mapping) else None, | |
| ) | |
| choices = data.get("choices") | |
| first = choices[0] if isinstance(choices, list) and choices else None | |
| raw_message = first.get("message") if isinstance(first, Mapping) else None | |
| message: Mapping[str, Any] = raw_message if isinstance(raw_message, Mapping) else {} | |
| return LlmProviderResponse( | |
| content=str(message.get("content") or ""), | |
| tool_calls=_parse_provider_tool_calls(message), | |
| raw_response=_redacted_response(data), | |
| usage=data.get("usage") if isinstance(data.get("usage"), Mapping) else None, | |
| cost=data.get("cost") if isinstance(data.get("cost"), Mapping) else None, | |
| ) |
🤖 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/analyzer/openrouter_review_provider.py`
around lines 242 - 250, Update the response parsing around choices and message
so the first choices element must be a mapping before calling .get or passing it
to _parse_provider_tool_calls. Treat malformed elements such as strings as an
empty message and preserve the existing LlmProviderResponse construction and
fail-closed behavior.
| if _np() and settings.docker_backend in {"cli", "docker"}: | ||
| run = await asyncio.to_thread( | ||
| _run_no_phala_host_terminal_bench, | ||
| job=job, | ||
| task=task, | ||
| plan=plan, | ||
| agent_workspace=agent_workspace, | ||
| miner_env=miner_env, | ||
| gateway=gateway, | ||
| execution_backend=execution_backend, | ||
| timeout_seconds=settings.evaluation_timeout_seconds, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Host path drops _terminal_bench_stream_env(plan.attempt_id).
The Docker branch injects the per-attempt log-stream env; the NO_PHALA branch does not, so real-time SSE log ingest silently stops in this mode. Forward it via _run_no_phala_host_terminal_bench if streaming is expected to keep working.
🤖 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/runner.py`
around lines 1401 - 1412, Update the NO_PHALA execution path in the runner
around _run_no_phala_host_terminal_bench to forward
_terminal_bench_stream_env(plan.attempt_id), matching the Docker branch’s
per-attempt streaming environment injection and preserving real-time SSE log
ingestion.
| env = { | ||
| **dict(os.environ), | ||
| **_terminal_bench_env(miner_env, gateway), | ||
| "PYTHONPATH": f"{agent_workspace}:{os.environ.get('PYTHONPATH', '')}".rstrip(":"), | ||
| "DOCKER_HOST": os.environ.get("DOCKER_HOST") or "unix:///var/run/docker.sock", | ||
| "BASE_AGENT_PATH": str(agent_workspace), | ||
| } | ||
| if env["PYTHONPATH"].endswith(":"): | ||
| env["PYTHONPATH"] = env["PYTHONPATH"][:-1] | ||
|
|
||
| try: | ||
| proc = subprocess.run( | ||
| cmd, | ||
| cwd=str(agent_workspace), | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=timeout_seconds, | ||
| check=False, | ||
| env=env, | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Host-local path leaks the validator's whole environment and puts the miner workspace on sys.path of a host process.
Two distinct hazards in this env/cwd construction:
**dict(os.environ)hands the validator process environment (shared token,database_url,eval_result_signer_mnemonic, gateway tokens, OpenRouter key, …) toown_runnerand everything it spawns. The Docker path deliberately passes only_terminal_bench_env(miner_env, gateway); this path reverses that containment.cwd=agent_workspaceplusPYTHONPATH=agent_workspace:...means miner-supplied files in the extracted ZIP precede stdlib/site-packages forpython -m agent_challenge...— a submittedsubprocess.py/json.pyexecutes as the validator host user, outside any sandbox.
Pass an explicit allowlisted env (control vars + _terminal_bench_env), and keep the agent workspace off the interpreter path (use BASE_AGENT_PATH only, run from a neutral cwd, and let own_runner import the agent inside its own sandbox).
As per coding guidelines: "Do not accept code that exfiltrates secrets, tokens, SSH keys, cookies, environment variables, or private files" and "Do not accept code that disables sandboxing, escapes containers, changes host permissions, or bypasses resource limits."
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 2088-2096: Command coming from incoming request
Context: subprocess.run(
cmd,
cwd=str(agent_workspace),
capture_output=True,
text=True,
timeout=timeout_seconds,
check=False,
env=env,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 2088-2096: Use of unsanitized data to create processes
Context: subprocess.run(
cmd,
cwd=str(agent_workspace),
capture_output=True,
text=True,
timeout=timeout_seconds,
check=False,
env=env,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
🤖 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/runner.py`
around lines 2078 - 2097, Update the host-local subprocess setup around env and
cwd construction to prevent validator environment and workspace code from
reaching the host process. Build env from an explicit allowlist of required
control variables plus _terminal_bench_env(miner_env, gateway), without
inheriting all of os.environ; retain BASE_AGENT_PATH, remove the
workspace-derived PYTHONPATH, and run subprocess.run from a neutral directory
while allowing own_runner to import the agent inside its sandbox.
Source: Coding guidelines
| # Optional USD spend ceiling for OpenRouter analyzer calls; fail closed when | ||
| # exceeded. Env: CHALLENGE_LLM_COST_LIMIT / CHALLENGE_LLM_COST_LIMIT_USD | ||
| llm_cost_limit_usd: float | None = Field( | ||
| default=None, | ||
| validation_alias=AliasChoices( | ||
| "llm_cost_limit_usd", | ||
| "LLM_COST_LIMIT", | ||
| "llm_cost_limit", | ||
| ), | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
pydantic-settings validation_alias env_prefix not applied AliasChoices
💡 Result:
In Pydantic Settings, the env_prefix configuration is generally ignored for fields that have a validation_alias (or alias) explicitly defined [1][2][3][4]. This behavior is intentional, as the explicit alias is treated as the source of truth for the environment variable name, overriding the default prefixing logic [3]. Regarding AliasChoices and env_prefix: 1. The env_prefix does not automatically apply to the strings defined inside AliasChoices [5][3]. If you define a field with validation_alias=AliasChoices('a', 'b'), Pydantic will look for environment variables named exactly a and b, not PREFIX_a or PREFIX_b [5][3]. 2. If you need the prefix to apply to your aliases, you must manually include the prefix within the alias strings (e.g., AliasChoices('MY_PREFIX_a', 'MY_PREFIX_b')) or use the env_prefix_target configuration setting if you are on a recent version of Pydantic Settings [6][1][4]. 3. The env_prefix_target setting, available in newer versions, allows you to control how the prefix is applied. By setting env_prefix_target='all', you can attempt to apply the prefix to both variable names and aliases, though its effectiveness with complex AliasChoices remains a known point of discussion in the community [6][1][4]. If you find that your aliases are not picking up the expected environment variables, it is because Pydantic prioritizes the defined alias over the env_prefix [1][3]. To achieve your desired configuration, you may need to explicitly define the full variable names including the prefix within the AliasChoices list, or implement a custom EnvSettingsSource by overriding the get_field_value method to manually handle the prefixing logic [7].
Citations:
- 1: https://pydantic.dev/docs/validation/2.12/concepts/pydantic_settings/
- 2:
populate_by_nameandenv_prefixare ignored when alias is used pydantic/pydantic-settings#144 - 3: Ignore env_prefix if validation_alias is set pydantic/pydantic-settings#25
- 4: https://pydantic.dev/docs/validation/dev/concepts/pydantic_settings/
- 5: There is no way to apply
env_prefixto aliases pydantic/pydantic-settings#748 - 6: https://pydantic.dev/docs/validation/latest/concepts/pydantic_settings
- 7: Give users the ability to override environment variables using SettingsConfigDict pydantic/pydantic-settings#231
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate config.py =="
fd -a 'config\.py$' . | sed 's#^\./##'
echo "== relevant config.py section =="
config_path="$(fd 'config\.py$' packages/challenges/agent-challenge/src/agent_challenge | head -n1 || true)"
if [ -n "$config_path" ]; then
sed -n '330,390p' "$config_path" | cat -n
echo "== env_prefix references =="
rg -n "env_prefix|CHALLENGE_LLM_COST_LIMIT|CHALLENGE_LLM_COST_LIMIT_USD|LLM_COST_LIMIT|master_base_url" "$config_path" packages/challenges/agent-challenge/src/agent_challenge || true
fi
echo "== pydantic dependency versions =="
for f in pyproject.toml pnpm-lock.yaml package-lock.json poetry.lock requirements.txt; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f" | rg -n "pydantic|pydantic-settings|version|\\$\\s*$" | head -n 120; }
doneRepository: BaseIntelligence/base
Length of output: 4633
🏁 Script executed:
#!/bin/bash
set -euo pipefail
config_path="packages/challenges/agent-challenge/src/agent_challenge/sdk/config.py"
echo "== config.py llm_cost_limit section =="
sed -n '35,110p' "$config_path" | cat -n
echo "== llm_cost_limit section =="
sed -n '360,385p' "$config_path" | cat -n
echo "== tests mentioning llm cost/env names =="
rg -n "llm_cost_limit|LLM_COST_LIMIT|CHALLENGE_LLM_COST_LIMIT|llm_cost_limit_usd|CHALLENGE_LLM_COST_LIMIT_USD|env_prefix" packages/challenges/agent-challenge/tests packages/challenges/agent-challenge/src/agent_challenge -g '*.py' || true
echo "== pyproject pydantic versions =="
sed -n '1,110p' pyproject.toml | cat -nRepository: BaseIntelligence/base
Length of output: 17039
Add the documented CHALLENGE_* aliases for llm_cost_limit_usd.
With an explicit validation_alias, env_prefix is not applied to these choices, so llm_cost_limit_usd only reads llm_cost_limit_usd, LLM_COST_LIMIT, and llm_cost_limit. Setting CHALLENGE_LLM_COST_LIMIT_USD / CHALLENGE_LLM_COST_LIMIT will be ignored, leaving the documented spend-ceiling configuration ineffective.
🛠️ Proposed fix
llm_cost_limit_usd: float | None = Field(
default=None,
validation_alias=AliasChoices(
"llm_cost_limit_usd",
+ "CHALLENGE_LLM_COST_LIMIT_USD",
+ "CHALLENGE_LLM_COST_LIMIT",
"LLM_COST_LIMIT",
"llm_cost_limit",
),
)📝 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.
| # Optional USD spend ceiling for OpenRouter analyzer calls; fail closed when | |
| # exceeded. Env: CHALLENGE_LLM_COST_LIMIT / CHALLENGE_LLM_COST_LIMIT_USD | |
| llm_cost_limit_usd: float | None = Field( | |
| default=None, | |
| validation_alias=AliasChoices( | |
| "llm_cost_limit_usd", | |
| "LLM_COST_LIMIT", | |
| "llm_cost_limit", | |
| ), | |
| ) | |
| # Optional USD spend ceiling for OpenRouter analyzer calls; fail closed when | |
| # exceeded. Env: CHALLENGE_LLM_COST_LIMIT / CHALLENGE_LLM_COST_LIMIT_USD | |
| llm_cost_limit_usd: float | None = Field( | |
| default=None, | |
| validation_alias=AliasChoices( | |
| "llm_cost_limit_usd", | |
| "CHALLENGE_LLM_COST_LIMIT_USD", | |
| "CHALLENGE_LLM_COST_LIMIT", | |
| "LLM_COST_LIMIT", | |
| "llm_cost_limit", | |
| ), | |
| ) |
🤖 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/sdk/config.py` around
lines 372 - 381, Update the validation_alias for llm_cost_limit_usd to include
the documented CHALLENGE_LLM_COST_LIMIT_USD and CHALLENGE_LLM_COST_LIMIT
environment names, while preserving the existing aliases and default behavior.
| def test_resolve_key_order_explicit_then_env_then_opencode( | ||
| tmp_path: Path, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| monkeypatch.delenv("CHALLENGE_OPENROUTER_API_KEY", raising=False) | ||
| monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) | ||
|
|
||
| assert resolve_openrouter_api_key(explicit=" explicit-key ", home=tmp_path) == "explicit-key" | ||
|
|
||
| monkeypatch.setenv("CHALLENGE_OPENROUTER_API_KEY", "challenge-key") | ||
| monkeypatch.setenv("OPENROUTER_API_KEY", "plain-key") | ||
| env = dict(**__import__("os").environ) | ||
| assert resolve_openrouter_api_key(explicit=None, environ=env, home=tmp_path) == "challenge-key" | ||
|
|
||
| monkeypatch.delenv("CHALLENGE_OPENROUTER_API_KEY", raising=False) | ||
| env = dict(**__import__("os").environ) | ||
| assert resolve_openrouter_api_key(explicit=None, environ=env, home=tmp_path) == "plain-key" | ||
|
|
||
| monkeypatch.delenv("OPENROUTER_API_KEY", raising=False) | ||
| auth = tmp_path / ".local" / "share" / "opencode" / "auth.json" | ||
| auth.parent.mkdir(parents=True) | ||
| payload = {"openrouter": {"type": "api", "key": "file-key"}} | ||
| auth.write_text(json.dumps(payload), encoding="utf-8") | ||
| assert resolve_openrouter_api_key(explicit=None, environ={}, home=tmp_path) == "file-key" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Replace plaintext secret fixtures with identifier-only placeholders.
The tests embed complete key/token-like values. Even when synthetic, this violates the repository rule and risks normalizing secrets in source or triggering secret scanners.
packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L37-L59: use identifier-style placeholders for precedence assertions.packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L62-L123: replace the provider/header fixture with an identifier-only placeholder.packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L126-L189: replace failure-path key fixtures likewise.packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L192-L254: replace gateway/OpenRouter token fixtures with names or digests.packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py#L555-L575: replace the OpenRouter environment fixture with an identifier-only placeholder.
As per coding guidelines, “Never log, document, or include evidence containing private keys, wallet mnemonics, API tokens, or full secret values; only names, digests, and SHAs are permitted.”
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 57-57: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
📍 Affects 2 files
packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L37-L59(this comment)packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L62-L123packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L126-L189packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py#L192-L254packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py#L555-L575
🤖 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_openrouter_review_provider.py`
around lines 37 - 59, Replace all plaintext secret-like test fixtures with
identifier-only placeholders, preserving each test’s assertions and precedence
behavior. Update
packages/challenges/agent-challenge/tests/test_openrouter_review_provider.py
ranges 37-59, 62-123, 126-189, and 192-254, plus
packages/challenges/agent-challenge/tests/test_eval_deploy_token_handoff.py
range 555-575; use names, digests, or SHAs consistently for environment values,
provider/header fixtures, failure-path keys, gateway tokens, and OpenRouter
tokens.
Source: Coding guidelines
1 participant
Summary
Temporary NO_PHALA mode lets the master validator run the full agent-challenge product loop on the host while Phala CVMs are disabled. This PR completes that path: dual-flag-off analysis (AST + LLM) → host
own_runnerbenchmark → scores → authenticated raw-weight push, with unforgeable unattested provenance everywhere scores/weights are recorded.What NO_PHALA is (and is not)
Is: opt-in host-local execution (
NO_PHALA/CHALLENGE_NO_PHALA, default OFF). Master runs jobs via existingown_runnerDocker path. Phala client/deploy refuse. Results hard-markedattested:false/attestation_status:unattested/execution_mode:no_phala_host. ZIP hash provenance (expected == download == executed) still enforced.Is not / does not prove: TEE / Intel TDX, DCAP quote, RTMR / compose_hash / KMS bind, measurement allowlist, or host integrity. Scores and weights from this mode are host-trust only — not TEE-attested.
Full pipeline now supported
With attestation flags off (required by NO_PHALA contradiction) the pre-Phala offline chain runs:
queue_submission_analysisrun_worker_once→run_next_analysisown_runner_backendEvaluationJobphala_attestation_enabled=falsetb_completedmetadata61651c2f) + CRITICAL log when NO_PHALA pushesAttested path files have zero intentional diff for gates:
attestation.py,plan_scoring.py,direct_result.pyuntouched.Safety design
PhalaCloudClient/ eval+reviewdeploy()refuse in-modemark_result_unattestedhard-codesattested=Falseand strips proof-looking keysEnable / disable on master
Confirm:
curl -sS http://127.0.0.1:18081/health→"no_phala":true,"attestation_mode":"no_phala_host".Validated output (local)
Health + mode banner + weight push loop (real):
Submit → analysis chain (real):
Live LLM review needs the BASE master LLM gateway (
{root}/llm/v1+X-Gateway-Token), not raw OpenRouter. Without it the worker correctly entersllm_standby/ fail-closed pause. Full AST→LLM→TB→score→push is covered by automated tests with a static reviewer + mock HTTP.Phala CVMs:
npx phala@latest cvms list --json→total: 0.Tests
test_no_phala_pipeline.py,test_ac_raw_weight_push.py,test_raw_weight_push_lifespan.pyTest plan
tb_queued+ AST/LLM rowstb_completedmetadata unattestedget_weightswith flags off/health+ push loop + submit analysis phaseCI
Notes
keyrelease/.Summary by CodeRabbit
/health+/versionvisibility.