docs(openworlds): add fidelity and asset contract - #124
Conversation
📝 WalkthroughWalkthroughThis PR adds two OpenWorlds docs: an asset-provenance and repo-content policy, and a fidelity rollout plan for the exact web surface with staged integration and validation requirements. ChangesOpenWorlds Integration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md`:
- Around line 12-19: The file OPENWORLDS_DESIGN_ASSET_POLICY.md currently
contains host-specific absolute paths (e.g., entries like
`/Volumes/LEXAR/Codex/openworlds-design-2026-05-25/openworlds/Open Worlds.html`,
`/Volumes/LEXAR/.../styles.css`, `/Volumes/LEXAR/.../app.jsx`,
`/Volumes/LEXAR/.../chrome.jsx`, `/Volumes/LEXAR/.../screen-*.jsx`,
`/Volumes/LEXAR/.../camp-sidebar.jsx`, `/Volumes/LEXAR/.../toast.jsx`,
`/Volumes/LEXAR/.../tooltip.jsx`) that leak personal/machine info; replace each
absolute path with a repo-stable provenance reference: use a neutral artifact
identifier (artifact name), a content hash (SHA256 or similar), acquisition
date, and a repo-relative pointer or canonical storage location (e.g.,
artifacts/ or docs/ path), and remove any /Users or /Volumes segments; update
all similar occurrences (lines noted in the comment: 23-24, 27-33, 37-40, and
the referenced line 39) so every entry follows the artifact-name + hash +
acquisition-date + repo-relative path pattern.
In `@docs/OPENWORLDS_FIDELITY_PLAN.md`:
- Line 84: The Markdown line contains a bare "`#123`" which is being parsed as an
ATX heading; fix by either escaping the hash or moving it inline so it isn't
interpreted as a heading—for example replace the standalone "`#123` linking to the
fidelity contract PR..." with "\`#123` linking to the fidelity contract PR..." or
"linking to the fidelity contract PR (see `#123`)" so the token is treated as
plain text; update the line containing "`#123`" accordingly in
OPENWORLDS_FIDELITY_PLAN.md.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 422df6a3-8d0c-4788-9758-cd68748c8b65
📒 Files selected for processing (2)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.mddocs/OPENWORLDS_FIDELITY_PLAN.md
📜 Review details
🧰 Additional context used
🪛 markdownlint-cli2 (0.22.1)
docs/OPENWORLDS_FIDELITY_PLAN.md
[warning] 84-84: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🔇 Additional comments (2)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (1)
41-99: LGTM!docs/OPENWORLDS_FIDELITY_PLAN.md (1)
1-83: LGTM!Also applies to: 85-123
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (1)
79-79: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winPoint the policy at the actual browser-side enforcement for campaign mutation
- Enforcement exists:
viewer/server.pymakes the viewer read-only by only allowing writes viaPOST /move, which (1) is disabled when$CLAWDND_PLAYER_MOVESis unset, (2) appends sanitized player move intents to the$CLAWDND_PLAYER_MOVESJSONL sink (never campaign state), and (3) rejects moves targeting a non-live/othercampaignid;sanitize_move()also whitelistskindand forcesrole: "player".- The UI mirrors it:
viewer/dashboard.htmlgates the action layer onstate.liveand only sends moves throughfetch("/move")(no direct state writes).Update
docs/OPENWORLDS_DESIGN_ASSET_POLICY.mdaround line 79 to explicitly name/link this enforcement mechanism (and optionally the related contract test:servers/engine/tests/test_provider_contract_smoke.py).🤖 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 `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md` at line 79, Update the policy text to explicitly point at the browser-side enforcement implemented in viewer/server.py (the POST /move endpoint) and the UI gating in viewer/dashboard.html: mention that viewer/server.py makes the viewer read-only by only accepting sanitized player intents via POST /move (see sanitize_move() which whitelists kind and forces role:"player", appends to $CLAWDND_PLAYER_MOVES JSONL sink, and rejects moves for non-live/other campaign ids and when $CLAWDND_PLAYER_MOVES is unset), and note that viewer/dashboard.html only sends moves via fetch("/move") and gates the action layer on state.live; optionally reference the related contract test servers/engine/tests/test_provider_contract_smoke.py.docs/OPENWORLDS_FIDELITY_PLAN.md (2)
27-37: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConfirm viewer/server.py enforces the browser write boundary
do_POSTonly permitsPOST /moveandPOST /speak; anything else 404s.POST /movewrites only to the append-only$CLAWDND_PLAYER_MOVESsink (via_moves_path()), and returns a “read-only (no live game)” error without writing when that env var/target isn’t available or writable.- Moves are constrained by
sanitize_move(...)and also refused when acampaigntag doesn’t match the attached live campaign (#49).- No request handler path in
viewer/server.pywritessnapshot.json,play-state, orqa/state(those are used as read-only inputs for rendering/monitoring).Add a small integration test to assert that
POST /moveis the only endpoint that can create/append the moves log, and that attempts to hit other routes don’t changesnapshot.json/play-state/qa/state.
123-123: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueClarify "read-only adversarial agents" terminology.
Line 123 mentions "read-only adversarial agents" which may be unclear to future contributors.
Consider clarifying:
- Is this referring to automated testing tools?
- Is it referring to AI code review agents (like CodeRabbit)?
- Or is it a security review process?
Example clarification:
-Use CodeRabbit and read-only adversarial agents before merge. +Use CodeRabbit and read-only adversarial review agents (automated security/quality scanners) before merge.🤖 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 `@docs/OPENWORLDS_FIDELITY_PLAN.md` at line 123, The phrase "read-only adversarial agents" is ambiguous; update the sentence to explicitly define what you mean (e.g., automated testing/security scanners, AI code-review bots like CodeRabbit, or a manual security review) and give one or two examples and intent; for example replace or augment the phrase with "read-only adversarial agents (e.g., automated security scanners or AI review bots such as CodeRabbit) used to probe for vulnerabilities without modifying code" and note whether they run automatically in CI, are human-operated, or are merely advisory.
🤖 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.
Outside diff comments:
In `@docs/OPENWORLDS_DESIGN_ASSET_POLICY.md`:
- Line 79: Update the policy text to explicitly point at the browser-side
enforcement implemented in viewer/server.py (the POST /move endpoint) and the UI
gating in viewer/dashboard.html: mention that viewer/server.py makes the viewer
read-only by only accepting sanitized player intents via POST /move (see
sanitize_move() which whitelists kind and forces role:"player", appends to
$CLAWDND_PLAYER_MOVES JSONL sink, and rejects moves for non-live/other campaign
ids and when $CLAWDND_PLAYER_MOVES is unset), and note that
viewer/dashboard.html only sends moves via fetch("/move") and gates the action
layer on state.live; optionally reference the related contract test
servers/engine/tests/test_provider_contract_smoke.py.
In `@docs/OPENWORLDS_FIDELITY_PLAN.md`:
- Line 123: The phrase "read-only adversarial agents" is ambiguous; update the
sentence to explicitly define what you mean (e.g., automated testing/security
scanners, AI code-review bots like CodeRabbit, or a manual security review) and
give one or two examples and intent; for example replace or augment the phrase
with "read-only adversarial agents (e.g., automated security scanners or AI
review bots such as CodeRabbit) used to probe for vulnerabilities without
modifying code" and note whether they run automatically in CI, are
human-operated, or are merely advisory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31dd7ab6-95c7-4fc4-b2e3-f162fc9a5f11
📒 Files selected for processing (2)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.mddocs/OPENWORLDS_FIDELITY_PLAN.md
📜 Review details
🔇 Additional comments (10)
docs/OPENWORLDS_DESIGN_ASSET_POLICY.md (5)
10-18: LGTM!
19-51: LGTM!
53-56: LGTM!
57-78: LGTM!
81-112: LGTM!docs/OPENWORLDS_FIDELITY_PLAN.md (5)
1-25: LGTM!
39-81: LGTM!
82-90: LGTM!
92-103: LGTM!
109-118: ⚡ Quick winRemove the concern about missing validation script
script/build_and_run.shexists in the repo and is executable.- The script’s
usage/casehandling includes--verify|verify, so./script/build_and_run.sh --verifyis a valid invocation.
Summary
Adds the fidelity-first contract for integrating the OpenWorlds design into ClawDnD after PR #123 proved that a hand SwiftUI repaint is the wrong path.
This PR is intentionally docs-only. It defines:
/openworlds/architecture and engine-state boundaryRefs #113.
Refs #122.
Supersedes the implementation direction in #123, but does not close it yet. #123 should remain draft/unmerged until the viewer-hosted OpenWorlds replacement is open.
Why
The OpenWorlds export depends on the actual web implementation for its fidelity: CSS texture layers, parchment/walnut/brass treatment, typography, SVG ornaments, dense layout, and interaction states. Recreating that by hand in SwiftUI lost the design. This contract locks the safer direction before code/assets move into the repo.
Boundaries
/moveplayer-intent path.Validation
Local validation run from
/Volumes/LEXAR/repos/ClawDnD-openworlds-contract:Results:
viewer/server.pypy_compile passedAdversarial review:
/moveboundaries.Next PR
PR B should create a cleaned, audited
viewer/openworlds/bundle served byviewer/server.pyunder/openworlds/, with local runtime dependencies and no reference images.Summary by CodeRabbit