[codex] Stabilize WorldOS release gate evidence - #465
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR migrates the play surface to OpenWorlds, adds separate art-repo path resolution and wiring across the macOS app and viewer, and expands release-readiness/gating scripts and tests to model evidence gaps and partial-gate behavior. ChangesOpenWorlds migration, art repo feature, and release infrastructure
Sequence Diagram(s)sequenceDiagram
participant RootView
participant AppProcessService
participant RepositoryLocator
participant ProviderAdapters
participant ProviderProcess
RootView->>AppProcessService: startProviderSession(artRepoPath)
AppProcessService->>RepositoryLocator: resolve art repo path
RepositoryLocator-->>AppProcessService: resolved path
AppProcessService->>ProviderAdapters: build budgetEnvironment(with artRepoPath)
ProviderAdapters->>ProviderProcess: launch with WORLDOS_ART_REPO_ROOT / CLAWDND_ART_REPO_ROOT
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
WorldOS-GUI-RUNBOOK.md (1)
27-35:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
Line 27 and Line 80 use unlabeled fenced blocks, which will keep failing MD040 checks. Please tag them as
bashfor consistency with the rest of the docs.Also applies to: 80-85
🤖 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 `@WorldOS-GUI-RUNBOOK.md` around lines 27 - 35, The two unlabeled fenced code blocks (one starting with "cd /Users/lume/ClawDnD-val" and the other at the block around lines 80–85) must be tagged with a language identifier to satisfy MD040; update each triple-backtick fence to use ```bash so both blocks are labeled consistently with other examples in WorldOS-GUI-RUNBOOK.md and will pass the linter.scripts/install-desktop-shortcut.sh (1)
33-34:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale installed-filename output in post-install messaging.
The script installs
WorldOS OpenWorlds.commandandWorldOS OpenWorlds.webloc, but these lines still printWorldOS Dashboard.*, which is misleading during setup verification.Proposed fix
-echo " • 'WorldOS Dashboard.command' — double-click: starts the viewer + opens $URL" -echo " • 'WorldOS Dashboard.webloc' — clickable link to $URL (once the server is running)" +echo " • 'WorldOS OpenWorlds.command' — double-click: starts the viewer + opens $URL" +echo " • 'WorldOS OpenWorlds.webloc' — clickable link to $URL (once the server is running)"🤖 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 `@scripts/install-desktop-shortcut.sh` around lines 33 - 34, Update the post-install echo messages so they match the actual installed filenames: change the strings that currently print "WorldOS Dashboard.command" and "WorldOS Dashboard.webloc" to "WorldOS OpenWorlds.command" and "WorldOS OpenWorlds.webloc" respectively in the echo statements in install-desktop-shortcut.sh so the output correctly reflects the installed artifacts.
🤖 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/GUI_WORKBOOK.md`:
- Around line 22-25: The markdown table under the section header "## REAL bugs
(verified; the actual punch-list)" violates MD058 by being adjacent to paragraph
text; fix it by inserting a blank line before the table and a blank line after
the table block so the table is isolated from surrounding text, ensuring the
header and subsequent paragraphs are separated from the table.
In `@qa/release_readiness.py`:
- Around line 191-205: The code currently only flags mismatched or mixed
non-empty run_build_sha values; update the logic around
build_shas/persona_scores to also treat missing run_build_sha as an evidence
gap: after computing persona_scores (and before validating mixed or matching
SHAs) detect any persona p where p.get("run_build_sha") is falsy and append an
evidence_gaps entry (gate "native_gate", missing like "build_sha provenance",
detail listing personas missing run_build_sha e.g. ", ".join(p['persona'] for p
in missing)). Keep the existing mismatched (args.build_sha) and mixed-build
checks but ensure the missing-provenance check runs first so a missing
run_build_sha prevents passing the native_gate.
In `@qa/ui_playtest_app.sh`:
- Line 236: The guard relying on app_pid_for_bundle is ineffective because
app_pid_for_bundle can exit 0 even when it finds no PID; modify the
app_pid_for_bundle function so it returns a non-zero exit status when no
matching process is found (i.e., set a failure exit code and/or call return 1
when the PID variable is empty) and ensure callers like the check in the script
that does app_pid_for_bundle "$APP_BUNDLE" >/dev/null 2>&1 || { ... } will
correctly take the error branch; update any other call sites (the similar block
around the 564-573 check) to rely on this new non-zero exit behavior.
In `@script/build_and_run.sh`:
- Around line 66-75: The current wait_for_bundle_pid function returns the first
PID from bundle_pid which may be a pre-existing instance; change the logic to
validate candidate PIDs from bundle_pid before returning by ensuring the PID
belongs to the process started by this run (e.g., compare process start time
against this script's start time, or check the process command-line/environment
for a unique marker set when launching the new instance); implement a helper
like pid_belongs_to_current_launch(pid) and use it in wait_for_bundle_pid (and
the other similar block at lines 186-190) to skip PIDs that do not match the
current launch context and only return a PID that passes validation.
In `@viewer/monitor.html`:
- Around line 113-118: The user-facing copy still refers to "the dashboard"
while the link and flow have moved to OpenWorlds; update all visible text around
the OpenWorlds link created in the title variable (where heading, c.name/c.id
and the href "/openworlds/" are used) to say "OpenWorlds" instead of "dashboard"
and ensure any adjacent meta or help text that directs users to start a game is
changed to reference OpenWorlds (and any phrasing like "start a game in the
dashboard" is replaced with "start a game in OpenWorlds").
In `@viewer/tests/test_openworlds_static.py`:
- Around line 29-32: In setUp, after saving the existing environment values
(self._old_worldos_art_repo_root, self._old_clawdnd_art_repo_root,
self._old_worldos_repo_root, self._old_clawdnd_repo_root) clear those env vars
(os.environ.pop or del) so host-specific WORLDOS_ART_REPO_ROOT,
CLAWDND_ART_REPO_ROOT, WORLDOS_REPO_ROOT and CLAWDND_REPO_ROOT do not influence
tests; keep the existing tearDown restore logic that resets those four
attributes back into os.environ to avoid side effects.
---
Outside diff comments:
In `@scripts/install-desktop-shortcut.sh`:
- Around line 33-34: Update the post-install echo messages so they match the
actual installed filenames: change the strings that currently print "WorldOS
Dashboard.command" and "WorldOS Dashboard.webloc" to "WorldOS
OpenWorlds.command" and "WorldOS OpenWorlds.webloc" respectively in the echo
statements in install-desktop-shortcut.sh so the output correctly reflects the
installed artifacts.
In `@WorldOS-GUI-RUNBOOK.md`:
- Around line 27-35: The two unlabeled fenced code blocks (one starting with "cd
/Users/lume/ClawDnD-val" and the other at the block around lines 80–85) must be
tagged with a language identifier to satisfy MD040; update each triple-backtick
fence to use ```bash so both blocks are labeled consistently with other examples
in WorldOS-GUI-RUNBOOK.md and will pass the linter.
🪄 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: 4a94ee3a-b880-48ab-a0ca-eb6c876abc87
📒 Files selected for processing (37)
.gitignoreREADME.mdWorldOS-GUI-RUNBOOK.mdWorldOS-NORTH-STAR.mdWorldOS-OPERATING-GOAL.mdWorldOS-RUNBOOK.mddocs/MODEL-TIERING-STRATEGY.mddocs/OPENWORLDS_FIDELITY_PLAN.mdmacos/WorldOSApp/Sources/WorldOSApp/Models/ProviderModels.swiftmacos/WorldOSApp/Sources/WorldOSApp/Services/AppProcessService.swiftmacos/WorldOSApp/Sources/WorldOSApp/Services/ProviderAdapters.swiftmacos/WorldOSApp/Sources/WorldOSApp/Services/RepositoryLocator.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/CampaignsView.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/MonitorView.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/PlayView.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/ProvidersView.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/RootView.swiftmacos/WorldOSApp/Sources/WorldOSApp/Views/SettingsView.swiftqa/GUI_WORKBOOK.mdqa/SCORECARD.mdqa/play_human.shqa/release_gate.shqa/release_readiness.pyqa/test_macos_app_static.pyqa/test_release_gate_static.pyqa/test_release_readiness.pyqa/ui_playtest_app.shscript/build_and_run.shscripts/install-desktop-shortcut.shscripts/play.shscripts/play_party.shviewer/monitor.htmlviewer/openworlds/screen-character.jsxviewer/server.pyviewer/tests/test_openworlds_static.pyworldos-dashboard.commandworldos-play.command
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/release_readiness.py`:
- Around line 275-276: The existence check treats every args.palette_source as a
filesystem path which falsely flags valid labels; update the check in
qa/release_readiness.py so that you only verify
Path(args.palette_source).exists() when the value looks like a path (e.g.
contains a path separator or is an absolute/relative path indicator) and
otherwise treat it as a label and skip the existence check; keep using the same
evidence_gaps append with gate "palette_live" when a real path is missing and
leave label values untouched (references: args.palette_source, evidence_gaps,
and the "palette_live" gate).
🪄 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: c02d62be-1c66-4898-a657-bc5f2fcd58fd
📒 Files selected for processing (4)
README.mdqa/SCORECARD.mdqa/release_readiness.pyqa/test_release_readiness.py
✅ Files skipped from review due to trivial changes (1)
- qa/SCORECARD.md
🚧 Files skipped from review as they are similar to previous changes (1)
- qa/test_release_readiness.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
origin/main, the canonical private-art checkout, the last measured gate build, and the last valid release gate are no longer conflated/openworlds/f5500acas partial/harness-contaminated evidence, not a release verdict, and adds a model-tiering proposal as to-test guidance onlyWhy
This is the takeover stabilization lane, not a release lane. The previous gate evidence could mix harness failures with product failures and could overstate partial evidence. The Mac app also needed an explicit code-root/art-root split so Lexar worktrees can be tested without moving the private-art checkout.
Related follow-up: #466 tracks the first clean non-partial five-persona RRI rerun after this gate hardening lands.
Validation
python3 -m unittest qa.test_macos_app_static qa.test_release_gate_static viewer.tests.test_openworlds_static qa.test_release_readiness-> 53 tests OKbash -n qa/ui_playtest_app.sh && bash -n qa/release_gate.sh && bash -n script/build_and_run.sh && bash -n scripts/install-desktop-shortcut.sh && node qa/playwright/screenshot_helper.test.js && git diff --check-> PASSswift build --package-path macos/WorldOSApp-> PASSWORLDOS_NO_STOP_EXISTING=1 WORLDOS_ART_REPO_ROOT=/Users/lume/ClawDnD-val WORLDOS_PREFER_LAUNCH_ROOTS=1 script/build_and_run.sh --verify-> launcheddist/WorldOS.appand cleaned up the launched app/worktree viewer PIDsWORLDOS_ART_REPO_ROOT=/Users/lume/ClawDnD-val qa/release_gate.sh --preflight-only-> PASS with expected takeover warnings: non-tip PR branch, occupied 8765, host swap pressureNot Release Evidence Yet
f5500acRRI remains preserved only as partial/harness-contaminated evidenceLicensing / CLA
CLA.mdand submit this contribution under the WorldOS Contributor License Agreement.Summary by CodeRabbit
New Features
Documentation
Tests