Add 100/100 hybrid app handoff gate - #504
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a hybrid QA handoff gate: a Python orchestrator runs deterministic web/native smokes and a short Codex playtest on the same clean SHA, captures indexed evidence via an enhanced exporter, validates hooks with a Node probe, and writes ChangesHandoff Gate Implementation & Tests
Sequence Diagram(s)sequenceDiagram
participant CLI as Handoff Gate CLI
participant WebGate as Web Scripted Gate
participant NativeGate as Native Provider Gate
participant Hooks as app_handoff_hooks.js
participant Exporter as export_app_evidence.py
participant Finalize as finalize_handoff
CLI->>WebGate: run_web_scripted(args)
WebGate->>Exporter: export evidence (manifest + files)
CLI->>NativeGate: run_native_provider_gate(provider)
NativeGate->>Hooks: run_hook_probe(base_url)
Hooks-->>NativeGate: hook report (ok/missing, details)
NativeGate->>Exporter: export evidence (play-state, manifest)
CLI->>Finalize: finalize_handoff(run_id, gates, expected_sha)
Finalize-->>CLI: write handoff.json (status, handoff_score, blockers)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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: 3
🧹 Nitpick comments (1)
qa/test_export_app_evidence.py (1)
151-157: 💤 Low valueConsider defensive assertions for review_entrypoint file categories.
The
assertInchecks assumereview["files"][category]keys exist. If the exporter omits a category, the test raisesKeyErrorinstead of a clear assertion failure. Adding explicit existence checks would improve test readability on failure.♻️ Optional defensive assertion pattern
review = payload["review_entrypoint"] self.assertEqual(review["schema"], "worldos.app-evidence-review-entrypoint.v1") self.assertEqual(review["failure_bucket"], "") +self.assertIn("screenshots", review["files"]) +self.assertIn("app_status_snapshots", review["files"]) +self.assertIn("session_surface_snapshots", review["files"]) +self.assertIn("moves", review["files"]) +self.assertIn("provider_trace", review["files"]) +self.assertIn("network_logs", review["files"]) +self.assertIn("action_logs", review["files"]) self.assertIn("run-dir/screenshots/beat-001.png", review["files"]["screenshots"]) self.assertIn("app-status.json", review["files"]["app_status_snapshots"]) self.assertIn("session-surface.json", review["files"]["session_surface_snapshots"]) self.assertIn("local-files/moves.jsonl", review["files"]["moves"]) self.assertIn("run-dir/scripted-provider/summary.json", review["files"]["provider_trace"]) self.assertIn("run-dir/network.ndjson", review["files"]["network_logs"]) self.assertIn("run-dir/actions.ndjson", review["files"]["action_logs"])🤖 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 `@qa/test_export_app_evidence.py` around lines 151 - 157, The test assumes keys exist under review["files"] which can raise KeyError; update the assertions in qa/test_export_app_evidence.py to first assert the presence of each category key (e.g., "screenshots", "app_status_snapshots", "session_surface_snapshots", "moves", "provider_trace", "network_logs", "action_logs") in review["files"] and then use the existing assertIn checks to verify specific file entries like "run-dir/screenshots/beat-001.png" in review["files"]["screenshots"]; this makes failures show clear assertion errors instead of KeyError and points directly to the missing category or file.
🤖 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 `@qa/app_handoff_gate.py`:
- Around line 79-87: The run_logged function can raise subprocess.TimeoutExpired
and currently propagates, so wrap the subprocess.run call in a try/except that
catches subprocess.TimeoutExpired (referencing run_logged and the proc variable)
and in the except block write a clear timeout message and the partial output to
the same log file, write a synthetic exit marker like "[timeout]" or "[exit
124]" and return a non-zero code (e.g., 124) instead of re-raising; ensure the
log_path.open context is used for these writes so the log is flushed and the
function returns normally so downstream code (like writing handoff.json) still
runs.
- Around line 389-391: The code currently ignores the result of
subprocess.run(cmd, cwd=ROOT, ...) and then treats read_json(out /
"manifest.json") as if export succeeded; update the block so you capture the
CompletedProcess (e.g., result = subprocess.run(...)), check result.returncode
and on non‑zero either raise an exception or return an error status (include
result.stdout/stderr in the log/error), and only call read_json if the process
succeeded and the manifest file exists; also validate that read_json returns a
valid dict (raise or propagate an error if parsing fails) so the gate does not
silently pass when the exporter failed (refer to variables cmd, ROOT, out, and
function read_json).
- Around line 552-575: The exported manifest currently uses the initial
smoke_json verdict before later checks; move the export_evidence call (and
assignment to gate.evidence_manifest) so it occurs after validate_app_status and
evidence_gap_count (i.e., after the failure checks and after
gate.fail/gate.pass_ decisions), or alternatively re-run export_evidence with
the final verdict right before returning; update references to export_evidence,
gate.evidence_manifest, validate_app_status, evidence_gap_count, gate.fail, and
gate.pass_ to ensure manifest_path/manifest reflect the final gate outcome.
---
Nitpick comments:
In `@qa/test_export_app_evidence.py`:
- Around line 151-157: The test assumes keys exist under review["files"] which
can raise KeyError; update the assertions in qa/test_export_app_evidence.py to
first assert the presence of each category key (e.g., "screenshots",
"app_status_snapshots", "session_surface_snapshots", "moves", "provider_trace",
"network_logs", "action_logs") in review["files"] and then use the existing
assertIn checks to verify specific file entries like
"run-dir/screenshots/beat-001.png" in review["files"]["screenshots"]; this makes
failures show clear assertion errors instead of KeyError and points directly to
the missing category or file.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: eb5793c5-d200-49b5-856f-3c91e9d3e7fd
📒 Files selected for processing (9)
docs/AGENT_GRADE_APP_TESTABILITY.mdqa/SCORECARD.mdqa/app_handoff_gate.pyqa/app_handoff_hooks.jsqa/export_app_evidence.pyqa/test_app_handoff_gate.pyqa/test_export_app_evidence.pyqa/test_macos_app_static.pyqa/ui_playtest_app.sh
100yenadmin
force-pushed
the
codex/worldos-handoff-gate-100
branch
from
June 1, 2026 06:24
abf4f60 to
a4f00aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
qa/export_app_evidence.py (2)
678-688:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAllow
session-surfacesnapshot fallback after a live fetch error.When the live
/session-surfacecall fails, this code recordssources["session_surface"] = {"ok": False}. The later fallback only runs when the key is absent, so a validsession-surface.final.jsonalready copied fromrun-diris ignored andhandoff_gate.session_surface_okstays false.Suggested fix
- if "session_surface" not in sources: + if not (sources.get("session_surface") or {}).get("ok"): _surface, session_surface_snapshot = first_bundle_json(bundle, ("session-surface.final.json", "session-surface*.json")) if session_surface_snapshot: sources["session_surface_snapshot"] = {"path": session_surface_snapshot, "ok": True}Also applies to: 707-710
🤖 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 `@qa/export_app_evidence.py` around lines 678 - 688, The error handler that catches (OSError, urllib.error.URLError, ValueError) should not force sources["session_surface"]["ok"] = False because later logic only falls back when the key is absent; instead, in the except block update gaps and set sources["session_surface"]["url"] = surface_url but do not overwrite an existing sources["session_surface"]["ok"] value (remove setting ok=False or only set ok=False if the key did not previously exist). Apply the same change to the other similar block around lines 707-710 so handoff_gate.session_surface_ok can become true when a pre-existing session-surface.final.json was copied from run-dir.
655-666:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear the hard-fail exit code once the bundled
app-statussnapshot recovers the export.After the live
/app-statusfetch fails,exit_codestays1even whenfirst_bundle_json()finds a validapp-status.final.json.qa/app_handoff_gate.pyLine 406 treats any non-zero exporter exit as fatal, so recovered native bundles still get downgraded to an exporter failure.Suggested fix
if not app_status: app_status, app_status_snapshot = first_bundle_json(bundle, ("app-status.final.json", "app-status*.json")) if app_status_snapshot: sources["app_status_snapshot"] = {"path": app_status_snapshot, "ok": True} + exit_code = 0Also applies to: 703-706
🤖 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 `@qa/export_app_evidence.py` around lines 655 - 666, When a live /app-status fetch fails we currently set exit_code = 1 and never clear it even if first_bundle_json() later finds a valid bundled app-status.final.json; update the recovery path (the code that calls first_bundle_json() and sets sources["app_status"]) to reset exit_code back to 0 when a bundled snapshot is accepted (i.e., when you set sources["app_status"]["ok"] = True or otherwise mark the source recovered). Specifically, after the code path that records the bundled app-status (the place that uses first_bundle_json() to populate sources["app_status"]), explicitly set exit_code = 0 so recovered bundles do not leave a hard-fail exit; make the same change for the other identical block that sets sources["app_status"] in the second location noted.
♻️ Duplicate comments (1)
qa/app_handoff_gate.py (1)
396-430:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the synthesized failure manifest before returning.
On timeout, non-zero exit, or missing/invalid
manifest.json, this returns an in-memory failure payload but does not write it toout/manifest.json.gate.evidence_manifestthen points at a file that may not exist, and the later native check at Line 741 can reread{}and miss the exporter failure.Suggested fix
manifest_path = out / "manifest.json" + + def persist_failure(reason: str) -> tuple[str, dict[str, Any]]: + payload = { + "schema": "worldos.app-evidence.v1", + "evidence_gaps": [{"source": "export_app_evidence", "kind": "manifest", "path": str(manifest_path), "reason": reason}], + "failure": {"failure_bucket": "no_provider", "failure_detail": reason}, + "handoff_gate": {"ok": False, "blocking_reasons": [reason]}, + } + json_dump(manifest_path, payload) + return str(manifest_path), payload + try: proc = subprocess.run(cmd, cwd=ROOT, text=True, capture_output=True, check=False, timeout=60) except subprocess.TimeoutExpired as exc: - reason = f"export_app_evidence timed out after {exc.timeout}s" - return str(manifest_path), { - "schema": "worldos.app-evidence.v1", - "evidence_gaps": [{"source": "export_app_evidence", "kind": "manifest", "path": str(manifest_path), "reason": reason}], - "failure": {"failure_bucket": "no_provider", "failure_detail": reason}, - "handoff_gate": {"ok": False, "blocking_reasons": [reason]}, - } + return persist_failure(f"export_app_evidence timed out after {exc.timeout}s") if proc.returncode != 0: - reason = f"export_app_evidence exited {proc.returncode}: {(proc.stderr or proc.stdout)[-1000:]}" - return str(manifest_path), { - "schema": "worldos.app-evidence.v1", - "evidence_gaps": [{"source": "export_app_evidence", "kind": "manifest", "path": str(manifest_path), "reason": reason}], - "failure": {"failure_bucket": "no_provider", "failure_detail": reason}, - "handoff_gate": {"ok": False, "blocking_reasons": [reason]}, - } + return persist_failure(f"export_app_evidence exited {proc.returncode}: {(proc.stderr or proc.stdout)[-1000:]}") if not manifest_path.exists(): - reason = "export_app_evidence did not write manifest.json" - return str(manifest_path), { - "schema": "worldos.app-evidence.v1", - "evidence_gaps": [{"source": "export_app_evidence", "kind": "manifest", "path": str(manifest_path), "reason": reason}], - "failure": {"failure_bucket": "no_provider", "failure_detail": reason}, - "handoff_gate": {"ok": False, "blocking_reasons": [reason]}, - } + return persist_failure("export_app_evidence did not write manifest.json") manifest = read_json(manifest_path) if not manifest: - reason = "export_app_evidence wrote invalid or empty manifest.json" - return str(manifest_path), { - "schema": "worldos.app-evidence.v1", - "evidence_gaps": [{"source": "export_app_evidence", "kind": "manifest", "path": str(manifest_path), "reason": reason}], - "failure": {"failure_bucket": "no_provider", "failure_detail": reason}, - "handoff_gate": {"ok": False, "blocking_reasons": [reason]}, - } + return persist_failure("export_app_evidence wrote invalid or empty manifest.json")🤖 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 `@qa/app_handoff_gate.py` around lines 396 - 430, When export_app_evidence fails (the subprocess.run block, the proc.returncode != 0 branch, or when manifest_path doesn't exist or read_json(manifest_path) returns falsy), persist the failure payload to the same manifest_path before returning so gate.evidence_manifest points to a real file; construct the same dict you return (schema/evidence_gaps/failure/handoff_gate), ensure manifest_path.parent exists, write the JSON atomically (e.g., write to a temp file then rename) and flush, then return str(manifest_path) and the dict; update the branches that set reason (the TimeoutExpired except, the non-zero return code branch, the missing file branch, and the invalid manifest branch) to perform this write using the manifest_path and the local reason variable.
🤖 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 `@qa/app_handoff_gate.py`:
- Around line 556-560: The Codex gate currently only fails when
trace.failed_or_error_count > 0 but provider_trace_summary can report
failed_or_error_count==0 while the trace is missing; update the check after
calling provider_trace_summary (the trace variable returned by
provider_trace_summary) to also fail when the trace is absent/missing by testing
its existence flag (e.g. trace.get("trace_exists") or equivalent key returned by
provider_trace_summary) in the conditional that returns the Codex failure (the
branch that currently checks provider == "codex" and failed_or_error_count);
modify that conditional to fail if the trace is missing OR failed_or_error_count
> 0 and keep the same return tuple and payload fields (screenshots,
evidence_gaps, provider_trace).
---
Outside diff comments:
In `@qa/export_app_evidence.py`:
- Around line 678-688: The error handler that catches (OSError,
urllib.error.URLError, ValueError) should not force
sources["session_surface"]["ok"] = False because later logic only falls back
when the key is absent; instead, in the except block update gaps and set
sources["session_surface"]["url"] = surface_url but do not overwrite an existing
sources["session_surface"]["ok"] value (remove setting ok=False or only set
ok=False if the key did not previously exist). Apply the same change to the
other similar block around lines 707-710 so handoff_gate.session_surface_ok can
become true when a pre-existing session-surface.final.json was copied from
run-dir.
- Around line 655-666: When a live /app-status fetch fails we currently set
exit_code = 1 and never clear it even if first_bundle_json() later finds a valid
bundled app-status.final.json; update the recovery path (the code that calls
first_bundle_json() and sets sources["app_status"]) to reset exit_code back to 0
when a bundled snapshot is accepted (i.e., when you set
sources["app_status"]["ok"] = True or otherwise mark the source recovered).
Specifically, after the code path that records the bundled app-status (the place
that uses first_bundle_json() to populate sources["app_status"]), explicitly set
exit_code = 0 so recovered bundles do not leave a hard-fail exit; make the same
change for the other identical block that sets sources["app_status"] in the
second location noted.
---
Duplicate comments:
In `@qa/app_handoff_gate.py`:
- Around line 396-430: When export_app_evidence fails (the subprocess.run block,
the proc.returncode != 0 branch, or when manifest_path doesn't exist or
read_json(manifest_path) returns falsy), persist the failure payload to the same
manifest_path before returning so gate.evidence_manifest points to a real file;
construct the same dict you return (schema/evidence_gaps/failure/handoff_gate),
ensure manifest_path.parent exists, write the JSON atomically (e.g., write to a
temp file then rename) and flush, then return str(manifest_path) and the dict;
update the branches that set reason (the TimeoutExpired except, the non-zero
return code branch, the missing file branch, and the invalid manifest branch) to
perform this write using the manifest_path and the local reason variable.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fab6b424-3b59-4f16-b1db-a4c55769e55d
📒 Files selected for processing (9)
docs/AGENT_GRADE_APP_TESTABILITY.mdqa/SCORECARD.mdqa/app_handoff_gate.pyqa/app_handoff_hooks.jsqa/export_app_evidence.pyqa/test_app_handoff_gate.pyqa/test_export_app_evidence.pyqa/test_macos_app_static.pyqa/ui_playtest_app.sh
✅ Files skipped from review due to trivial changes (1)
- docs/AGENT_GRADE_APP_TESTABILITY.md
🚧 Files skipped from review as they are similar to previous changes (6)
- qa/SCORECARD.md
- qa/ui_playtest_app.sh
- qa/test_app_handoff_gate.py
- qa/test_macos_app_static.py
- qa/app_handoff_hooks.js
- qa/test_export_app_evidence.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
qa/app_handoff_gate.py (1)
728-729: 💤 Low valueAvoid redundant manifest re-read.
The
manifestvariable from line 715 is still in scope and contains the same data that would be read from disk. Re-reading adds I/O overhead and could theoretically get different data if something modified the file.Suggested simplification
- if gate.status == "passed" and evidence_gap_count(read_json(Path(gate.evidence_manifest))): + if gate.status == "passed" and evidence_gap_count(manifest): gate.fail("no_provider", "native evidence manifest has gaps")🤖 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 `@qa/app_handoff_gate.py` around lines 728 - 729, Replace the redundant disk re-read by using the in-scope manifest variable: when checking if gate.status == "passed" and calling evidence_gap_count, pass the already-loaded manifest (the variable manifest from earlier) instead of re-reading Path(gate.evidence_manifest); update the conditional to call evidence_gap_count(manifest) and keep the gate.fail("no_provider", "native evidence manifest has gaps") behavior unchanged.
🤖 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.
Nitpick comments:
In `@qa/app_handoff_gate.py`:
- Around line 728-729: Replace the redundant disk re-read by using the in-scope
manifest variable: when checking if gate.status == "passed" and calling
evidence_gap_count, pass the already-loaded manifest (the variable manifest from
earlier) instead of re-reading Path(gate.evidence_manifest); update the
conditional to call evidence_gap_count(manifest) and keep the
gate.fail("no_provider", "native evidence manifest has gaps") behavior
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e78462b-8f89-45ed-b4b3-6702b2e2480c
📒 Files selected for processing (4)
qa/app_handoff_gate.pyqa/export_app_evidence.pyqa/test_app_handoff_gate.pyqa/test_export_app_evidence.py
🚧 Files skipped from review as they are similar to previous changes (2)
- qa/test_export_app_evidence.py
- qa/export_app_evidence.py
This was referenced Jun 1, 2026
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
qa/app_handoff_gate.py, a single hybrid gate that scores web scripted smoke, builtdist/WorldOS.appscripted smoke, and a short built-app Codex-provider playtest on the same clean SHAmanifest.jsonwithreview_entrypointpointers for command, repo/commit, provider/gate kind, screenshots, app-status/session-surface snapshots, moves, traces, console/network/action logs, art status, and failure buckethandoff_score: 100is an implementation-velocity handoff signal, not a release verdict; full non-partial RRI remains separateLocal proof
Evidence root:
/Volumes/LEXAR/Codex/worldos-agent-grade-app-testability/handoff-local-20260601T063508Z-c3d37ae/handoff.jsonresult:passed100c3d37aefalsefalseGate results on the same SHA:
failed_or_error_count: 0, no evidence gapsEvidence manifests:
Tests
bash -n qa/ui_playtest_app.sh scripts/play_scripted_dm.shpython3 -m py_compile qa/app_handoff_gate.py qa/app_smoke_scripted.py qa/export_app_evidence.py qa/app_failure_buckets.pynode --check qa/app_handoff_hooks.jspython3 -m pytest qa/test_app_handoff_gate.py qa/test_export_app_evidence.py -q-> 13 passedpython3 -m pytest qa/test_app_failure_buckets.py qa/test_app_smoke_scripted.py qa/test_export_app_evidence.py qa/test_release_readiness.py qa/test_app_handoff_gate.py qa/test_macos_app_static.py qa/test_ui_playtest_app_buckets.py viewer/tests/test_openworlds_static.py -q-> 88 passed, 6 subtests passedFollow-ups
This does not run or replace the full five-persona RRI.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores