Skip to content

Workspaces: add persistent notes widget#101829

Open
100yenadmin wants to merge 2 commits into
openclaw:mainfrom
100yenadmin:feat/dashboard-notes
Open

Workspaces: add persistent notes widget#101829
100yenadmin wants to merge 2 commits into
openclaw:mainfrom
100yenadmin:feat/dashboard-notes

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Workspaces had persistent per-widget state after #101329, but no trusted built-in editor that used the contract safely. The legacy Notes implementation targeted the removed dashboard stack and could overwrite edits during hydration or write without a loaded concurrency version.

Why This Change Was Made

This stacked rebuild adds builtin:notes to the current Workspaces registry and consumes the recovered widget-state contract through a trusted per-widget accessor. Every save uses optimistic concurrency, missing versions never degrade to unguarded writes, early edits queue until hydration completes, and conflicts surface visibly. Debounced writes are canceled on disconnect/unmount; reconnect retries hydration without performing network work while unmounted.

User Impact

Workspace users can keep persistent notes inside a widget without losing early input to a late load. Concurrent edits are detected instead of silently overwritten, read-only/offline views cannot mutate state, and transient hydration failures can recover after reconnect.

Generated locale synchronization remains intentionally separate: four new English keys require authenticated translation sync for the repository's 20 generated locales. No generated locale was hand-edited or filled with fallback-only output.

Evidence

  • Stacked on the recovered Workspaces widget-state contract and rebased onto upstream main at 843e3c787806389cdaac884300e671fba658dab6 during the final recovery pass; the Notes commit range-diffs identically and its companion differs only in current-main import context.
  • Final-head focused Vitest: 61 extension + 83 UI tests passed.
  • Scoped extension/UI oxfmt and oxlint: clean.
  • Final-head extension/UI/UI-test TypeScript checks passed.
  • Autoreview found two lifecycle defects (unmount-stranded debounce and non-retryable hydration); both were fixed with expanded tests.
  • Test-only state helpers were removed from the public export surface; the remaining pairing-store dead-export findings originate from current upstream main.
  • git diff --check: clean.
  • Known external gate: pnpm ui:i18n:check reports 12 pending keys across 20 generated locales: four introduced here plus eight already present on the frozen main snapshot. Authenticated translation sync remains required; no fallback-only locale output was committed.

@100yenadmin
100yenadmin requested a review from a team as a code owner July 7, 2026 17:16
@100yenadmin

100yenadmin commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

On the dependency-guard check: stacked on write-back (#101329) → Wave 1 (#101098), so the cumulative diff inherits Wave 1's single new-plugin pnpm-lock.yaml importer entry. This PR adds no new dependency. Verified green locally by the submitting orchestrator: UI dashboard 98 tests, ui:i18n:check clean, pnpm ui:build, full type-aware oxlint.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@100yenadmin 100yenadmin mentioned this pull request Jul 7, 2026
Open
24 tasks
@github-actions github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: ed69acd55a07eb07a1e9bc7a2463e400875e4728

@clawsweeper

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 8:55 AM ET / 12:55 UTC.

Summary
This PR adds SQLite-backed per-widget state and a built-in persistent notes editor to Workspaces.

PR surface: Source +672, Tests +647, Docs +6. Total +1325 across 23 files.

Reproducibility: yes. for the remaining defect at source level: on the unchanged reviewed head, mounted notes hydrate through a cached state accessor without an evidenced broadcast-driven rehydration path. The feature itself is a new capability rather than a current-main bug report.

Review metrics: 2 noteworthy metrics.

  • Persistent API Surface: 1 capability and 2 gateway methods added. The custom-widget contract becomes a compatibility and security boundary for future plugin consumers.
  • Stored Data Model: 1 SQLite table added. The widget_state schema and row lifecycle require explicit upgrade and ownership review before merge.

Stored data model
Persistent data-model change detected: database schema: extensions/workspaces/src/manifest.test.ts, database schema: extensions/workspaces/src/store.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix broadcast-driven refresh and generated locale synchronization.
  • [P1] Add redacted real-browser proof for persistence, conflict, offline or read-only behavior, reconnect, and cross-client refresh.
  • Rebase to a notes-only diff after the persistence contract is accepted, or adopt a maintainer-approved trusted-only boundary.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Only tests, checks, and CI are supplied; add a redacted browser recording or linked live artifact showing save and reload persistence, conflict handling, disconnected read-only behavior, reconnect recovery, and cross-client refresh. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The cumulative branch couples the notes consumer to a new persistent custom-widget capability, two gateway methods, and a SQLite schema before the open base contract is independently accepted.
  • [P1] The permanent trust model for approved sandboxed widgets reading and writing parent-bound persistent state is a maintainer security and product choice that green tests do not settle.
  • [P1] Landing the stacked commits out of order or without a notes-only rebase would obscure ownership and upgrade review of the stored-data contract.

Maintainer options:

  1. Land Persistence Before Notes (recommended)
    Approve and land the canonical widget-state contract, then rebase this branch to a notes-only diff with refresh, locale, and live-proof gaps resolved.
  2. Narrow to Trusted-Only Storage
    Remove the custom-widget capability and API from this branch and keep persistence private to the built-in notes implementation.
  3. Pause for Boundary Design
    Do not merge the cumulative branch until maintainers explicitly accept the persistent sandbox bridge and stored-data contract.

Next step before merge

  • [P1] A maintainer must approve the permanent persistence and security boundary and stacked landing order, and the contributor must supply real-browser proof; this is not a safe automated repair lane yet.

Maintainer decision needed

  • Question: Should Workspaces permanently expose the proposed state:persist capability and widget-state gateway and storage contract to approved custom widgets before the built-in notes editor lands?
  • Rationale: This creates a durable plugin capability, sandbox-to-parent trust boundary, gateway API, and stored-data contract; implementation tests cannot choose that permanent product and security boundary or landing order.
  • Likely owner: steipete — The supplied timeline provides the strongest available maintainer-side routing signal for the active security-boundary review, while 100yenadmin owns the implementation history.
  • Options:
    • Approve Contract First (recommended): Review and land Workspaces: persist widget state in SQLite #101329 first, then rebase this PR to the built-in notes consumer and focused fixes and proof.
    • Keep Notes Trusted-Only: Narrow persistence to a private built-in accessor and defer the custom-widget state:persist contract to separate design approval.
    • Pause Persistent Notes: Defer both changes until maintainers select another Workspaces persistence model.

Security
Cleared: No concrete sandbox escape, cross-widget identity bypass, secret exposure, or supply-chain regression is shown; the new persistent capability still requires maintainer acceptance as a permanent security boundary.

Review findings

  • [P2] Refresh mounted notes on widget-state broadcasts — ui/src/lib/workspace/widgets/context.ts:18-21
  • [P2] Synchronize the generated locale files — ui/src/i18n/locales/en.ts:2193-2196
Review details

Best possible solution:

Approve and land the widget-state contract separately, then rebase this PR to a notes-only consumer that refreshes safely on broadcasts, includes generated locales, and demonstrates the full workflow in a real browser.

Do we have a high-confidence way to reproduce the issue?

Yes for the remaining defect at source level: on the unchanged reviewed head, mounted notes hydrate through a cached state accessor without an evidenced broadcast-driven rehydration path. The feature itself is a new capability rather than a current-main bug report.

Is this the best way to solve the issue?

No, not in its current cumulative form. Optimistic concurrency and parent-bound identity are strong primitives, but the maintainable path is an accepted persistence base followed by a notes-only PR with broadcast refresh, locale synchronization, and live proof.

Full review comments:

  • [P2] Refresh mounted notes on widget-state broadcasts — ui/src/lib/workspace/widgets/context.ts:18-21
    The notes editor hydrates once and keeps its binding keyed by the cached per-client state accessor, but the plugin.workspaces.widget-state.changed path does not invalidate or rehydrate that mounted editor. A write from another client therefore leaves the visible note stale until remount; subscribe or invalidate by widget ID while preserving dirty and conflict handling.
    Confidence: 0.96
  • [P2] Synchronize the generated locale files — ui/src/i18n/locales/en.ts:2193-2196
    The PR adds four English notes keys without the generated locale updates required by the UI i18n contract, and the PR body confirms ui:i18n:check remains pending for these keys. Run the authenticated translation synchronization before merge.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0d1573e9a2cc.

Label changes

Label justifications:

  • P2: This is a substantial but non-urgent Workspaces feature with bounded web UI and plugin impact.
  • merge-risk: 🚨 compatibility: The PR adds a persistent manifest capability, gateway methods, and SQLite schema that future widgets and upgrades may depend on.
  • merge-risk: 🚨 security-boundary: Approved sandboxed widgets gain a new parent-mediated persistent read and write capability whose permanent trust boundary needs maintainer acceptance.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only tests, checks, and CI are supplied; add a redacted browser recording or linked live artifact showing save and reload persistence, conflict handling, disconnected read-only behavior, reconnect recovery, and cross-client refresh. Updating the PR body should trigger re-review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +672, Tests +647, Docs +6. Total +1325 across 23 files.

View PR surface stats
Area Files Added Removed Net
Source 15 687 15 +672
Tests 7 653 6 +647
Docs 1 8 2 +6
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 23 1348 23 +1325

What I checked:

  • Re-review continuity: The current head exactly matches the latest completed ClawSweeper review, and the supplied timeline shows no later commit, so its two unresolved findings remain applicable. (ui/src/lib/workspace/widgets/context.ts:18, ed69acd55a07)
  • Cross-client refresh gap: The new per-client widget-state accessor and notes lifecycle provide initial hydration but no evidenced invalidation or rehydration when plugin.workspaces.widget-state.changed is broadcast, leaving mounted notes stale after another client writes. (ui/src/lib/workspace/widgets/context.ts:18, ed69acd55a07)
  • Generated locale gap: Four English notes strings are added while the PR body explicitly reports pending generated-locale keys and leaves authenticated translation synchronization for separate work. (ui/src/i18n/locales/en.ts:2193, ed69acd55a07)
  • Persistence contract dependency: The branch carries the same state:persist capability, gateway methods, and SQLite widget-state contract tracked by the still-open base PR Workspaces: persist widget state in SQLite #101329. (extensions/workspaces/src/store.ts:64, b5d4ebaef8f2)
  • Proof remains test-only: The PR body supplies focused tests, type and lint checks, and CI results, but no after-fix browser recording, live output, logs, or linked artifact demonstrating the changed workflow. (ed69acd55a07)

Likely related people:

  • 100yenadmin: This person authored the broader Workspaces roadmap and multiple persistence, widget, and control-surface proposals beyond this PR, providing the strongest available implementation-domain signal. (role: feature-history contributor; confidence: medium; commits: b5d4ebaef8f2, ed69acd55a07; files: extensions/workspaces/src/store.ts, ui/src/lib/workspace/widgets/notes.ts, ui/src/lib/workspace/widgets/context.ts)
  • steipete: The timeline records an explicit mention and subscription while the compatibility and security-boundary review signals were active. (role: review routing candidate; confidence: low; files: extensions/workspaces/src/store.ts, ui/src/lib/workspace/bridge.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (14 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-07T23:57:22.771Z sha f6a71c1 :: needs real behavior proof before merge. :: [P2] Refetch notes on widget-state broadcasts | [P2] Preserve hidden widgets in the UI model | [P3] Remove the local carve plan
  • reviewed 2026-07-08T04:34:18.879Z sha f6a71c1 :: needs real behavior proof before merge. :: [P2] Refetch notes on widget-state broadcasts | [P2] Preserve hidden widgets in the UI model | [P3] Remove the local carve plan
  • reviewed 2026-07-09T23:31:40.825Z sha 224bad3 :: needs real behavior proof before merge. :: [P2] Refetch notes on widget-state broadcasts | [P2] Preserve hidden widgets in the UI model | [P3] Remove the local carve plan
  • reviewed 2026-07-10T08:39:41.544Z sha 73180ee :: needs real behavior proof before merge. :: [P2] Move dashboard runtime state into SQLite | [P2] Refetch notes on widget-state broadcasts | [P2] Preserve hidden widgets in the UI model | [P3] Remove the local carve plan
  • reviewed 2026-07-13T03:14:37.175Z sha df76cdd :: needs real behavior proof before merge. :: [P2] Refresh mounted notes when widget state changes | [P2] Synchronize the generated locale files
  • reviewed 2026-07-13T07:39:12.869Z sha 2bb670a :: needs real behavior proof before merge. :: [P2] Invalidate cached notes state on widget-state broadcasts | [P2] Synchronize the generated locale files
  • reviewed 2026-07-13T09:47:44.290Z sha ee58f50 :: needs real behavior proof before merge. :: [P2] Refresh mounted notes after widget-state broadcasts | [P2] Synchronize the generated locale files
  • reviewed 2026-07-13T12:38:07.598Z sha ed69acd :: needs real behavior proof before merge. :: [P2] Refresh mounted notes on widget-state broadcasts | [P2] Synchronize the generated locale files

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 7, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 7, 2026
Open
@100yenadmin

100yenadmin commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the P1 file-binding contract issue flagged here. resolveBinding (ui/src/lib/dashboard/index.ts) now calls dashboard.data.read with { binding } — the gateway's readParams whitelist accepts only that key, so the previous { path, pointer } call threw at runtime — and it returns the server's already-pointer-resolved data verbatim (the server applies the JSON pointer in resolveFileBinding; the client had been re-applying it, a double-resolve). The unit test had encoded the wrong contract (mocked the whole doc, asserted { path, pointer }); it's rewritten to assert the real gateway shape so a regression fails. The fix is in this PR's diff now and at the base #101098 the series stacks on.

The dependency-guard red and the large cumulative diff are both inherent to the base-first split (this stacks on the unmerged #101098; no new package enters the resolved graph — just the new plugin's lockfile importer entry needing the standard head-SHA authorization).

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@github-actions github-actions Bot mentioned this pull request Jul 7, 2026
Open
@100yenadmin 100yenadmin mentioned this pull request Jul 12, 2026
Open
@100yenadmin
100yenadmin force-pushed the feat/dashboard-notes branch from 73180ee to df76cdd Compare July 13, 2026 02:40
@100yenadmin 100yenadmin changed the title Dashboard: stateful notes widget — the first consumer of widget write-back Workspaces: add persistent notes widget Jul 13, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation plugin: workspaces Agent-composable Workspaces plugin labels Jul 13, 2026
@github-actions github-actions Bot removed the dependencies-changed PR changes dependency-related files label Jul 13, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 13, 2026
@100yenadmin
100yenadmin force-pushed the feat/dashboard-notes branch 2 times, most recently from 2bb670a to ee58f50 Compare July 13, 2026 09:03
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 13, 2026
@100yenadmin
100yenadmin force-pushed the feat/dashboard-notes branch from ee58f50 to ed69acd Compare July 13, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: web-ui App: web-ui docs Improvements or additions to documentation merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. plugin: workspaces Agent-composable Workspaces plugin rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@100yenadmin