feat(greenfield): close OpenClaw operations and media parity - #421
Open
mira-2026 wants to merge 13 commits into
Open
Conversation
mira-2026
changed the title
mira-2026
added
status: needs-review
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (6)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used🧠 Learnings (11)📓 Common learnings📚 Learning: 2026-07-22T14:45:35.974ZApplied to files:
📚 Learning: 2026-08-07T18:48:18.623ZApplied to files:
📚 Learning: 2026-08-04T04:04:49.381ZApplied to files:
📚 Learning: 2026-08-01T06:32:15.434ZApplied to files:
📚 Learning: 2026-08-07T18:47:59.017ZApplied to files:
📚 Learning: 2026-07-14T11:31:42.796ZApplied to files:
📚 Learning: 2026-08-05T14:24:30.572ZApplied to files:
📚 Learning: 2026-07-20T22:09:46.707ZApplied to files:
📚 Learning: 2026-08-07T17:05:36.638ZApplied to files:
📚 Learning: 2026-08-07T18:47:49.639ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change implements protected OpenClaw configuration backups, durable Gateway restarts, bounded workspace-file prefixes, and secure local-history media projection and delivery. It updates runtime composition, browser controls, contracts, audits, parity fixtures, and tests. ChangesOpenClaw operations and file access
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
actor Operator
participant OpenClawSettingsPanel
participant OpenClawSettingsService
participant ConfigurationBackupTicketStore
participant ConfigurationBackupRawHttp
participant WorkspaceFileReader
Operator->>OpenClawSettingsPanel: Request configuration backup
OpenClawSettingsPanel->>OpenClawSettingsService: createConfigurationBackup
OpenClawSettingsService->>WorkspaceFileReader: Read exact secret-bearing configuration
WorkspaceFileReader-->>OpenClawSettingsService: Configuration bytes
OpenClawSettingsService->>ConfigurationBackupTicketStore: Issue actor-bound one-shot ticket
ConfigurationBackupTicketStore-->>OpenClawSettingsService: Ticket metadata and URL
OpenClawSettingsService-->>OpenClawSettingsPanel: Backup result without bytes
OpenClawSettingsPanel->>ConfigurationBackupRawHttp: GET ticket URL
ConfigurationBackupRawHttp->>ConfigurationBackupTicketStore: Consume ticket
ConfigurationBackupTicketStore-->>ConfigurationBackupRawHttp: Backup bytes
ConfigurationBackupRawHttp-->>OpenClawSettingsPanel: No-store backup response
sequenceDiagram
actor Operator
participant OpenClawSettingsPanel
participant OpenClawSettingsService
participant RestartQueue
participant JobRepository
participant GatewayLifecycle
Operator->>OpenClawSettingsPanel: Confirm Gateway restart
OpenClawSettingsPanel->>OpenClawSettingsService: restartGateway with idempotency key
OpenClawSettingsService->>RestartQueue: Persist or reconcile restart run
RestartQueue->>JobRepository: Store exclusive single-attempt job
RestartQueue->>JobRepository: Enqueue and poll durable run
JobRepository->>GatewayLifecycle: Execute fixed Gateway restart
GatewayLifecycle-->>JobRepository: Restart result
JobRepository-->>RestartQueue: Terminal run state
RestartQueue-->>OpenClawSettingsService: Restarted result
OpenClawSettingsService-->>OpenClawSettingsPanel: Operation status
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
greenfield/src/server/domains/jobs/workerRuntime.ts (1)
402-409: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPass the resolved restart action definition to the coordinator.
The resolver adds
openclaw.gateway.restartwhenopenClawGatewayexists. The coordinator below still receives onlyjobActionDefinitions.The restart definition cannot be in
jobActionDefinitions. If it were,createJobWorkerActionResolverwould append it again and reject duplicate action keys. A queued restart job can therefore have an executor but no coordinator action definition.Create one conditional action-definition list. Pass that same list to both the resolver and
createCoordinator.🤖 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 `@greenfield/src/server/domains/jobs/workerRuntime.ts` around lines 402 - 409, In the worker runtime flow around createJobWorkerActionResolver and createCoordinator, build one conditional action-definition list that includes the resolved openclaw.gateway.restart definition when openClawGateway exists without adding it to jobActionDefinitions. Pass this shared list to both the resolver and coordinator, preserving the resolver’s existing behavior and preventing duplicate action keys.
🧹 Nitpick comments (14)
greenfield/src/contracts/openClawSettings.ts (1)
758-774: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFreeze the raw HTTP contract list and keep the
satisfiesconformance check.
openClawSettingsRawHttpContractsis exported as a mutableRawHttpContract[]. Every other contract list in this repository, includingworkspaceFileRawHttpContractsingreenfield/src/contracts/files.ts(lines 558 and 597), usesas const satisfies readonly RawHttpContract[]. Apply the same shape here so the registry input stays immutable and the literal conformance check runs.♻️ Proposed refactor
/** One-shot, same-origin secret-bearing configuration export. */ -export const openClawSettingsRawHttpContracts = (["GET", "HEAD"] as const).map( - (method): RawHttpContract => ({ - access: controlAccess, - method, - path: "/api/openclaw-settings/configuration-backups/:ticketId", - rangeRequests: "none", - requestBody: { kind: "none" }, - response: { - contentTypes: ["application/json"], - kind: "binary", - maximumBytes: openClawConfigurationBackupMaximumBytes, - transfer: "buffered", - }, - statusCodes: [200, 400, 401, 403, 404, 405, 410, 429, 500, 503], - summary: `${method === "HEAD" ? "Inspects" : "Consumes"} one actor-bound no-store OpenClaw configuration export ticket.`, - }) -); +export const openClawSettingsRawHttpContracts = [ + ...(["GET", "HEAD"] as const).map((method): RawHttpContract => ({ + access: controlAccess, + method, + path: "/api/openclaw-settings/configuration-backups/:ticketId", + rangeRequests: "none", + requestBody: { kind: "none" }, + response: { + contentTypes: ["application/json"], + kind: "binary", + maximumBytes: openClawConfigurationBackupMaximumBytes, + transfer: "buffered", + }, + statusCodes: [200, 400, 401, 403, 404, 405, 410, 429, 500, 503], + summary: `${method === "HEAD" ? "Inspects" : "Consumes"} one actor-bound no-store OpenClaw configuration export ticket.`, + })), +] as const satisfies readonly RawHttpContract[];🤖 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 `@greenfield/src/contracts/openClawSettings.ts` around lines 758 - 774, Update the exported openClawSettingsRawHttpContracts declaration to use an immutable literal tuple with `as const satisfies readonly RawHttpContract[]`, matching the pattern used by other contract lists such as workspaceFileRawHttpContracts. Preserve the existing mapped GET/HEAD contract contents while ensuring the readonly conformance check remains active.greenfield/src/worker/openClaw/gatewayLifecycle.test.ts (1)
20-34: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the environment exactly instead of denylisting two secret names.
toMatchObjectat Line 20 permits extra environment keys. Lines 33-34 then check only thatOPENCLAW_GATEWAY_TOKENandMOLTBOOK_API_KEYare absent. A third leaked secret would pass this test. The environment for the restart process is fixed, so assert the exact key set.💚 Proposed stricter assertion
const environment = (calls[0] as { environment: Record<string, string> }) .environment; - expect(Object.keys(environment)).not.toContain("OPENCLAW_GATEWAY_TOKEN"); - expect(Object.keys(environment)).not.toContain("MOLTBOOK_API_KEY"); + expect(environment).toEqual({ + HOME: "/home/dashboard", + OPENCLAW_NO_RESPAWN: "1", + PATH: "/usr/local/bin:/usr/bin:/bin", + });🤖 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 `@greenfield/src/worker/openClaw/gatewayLifecycle.test.ts` around lines 20 - 34, Update the restart environment assertion in the gateway lifecycle test to require the exact fixed environment object/key set, replacing the partial toMatchObject and two-name denylist checks. Preserve the expected HOME, OPENCLAW_NO_RESPAWN, and PATH values while ensuring no additional environment keys are accepted.greenfield/src/server/domains/openClawSettings/configurationBackup.test.ts (1)
132-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the read-result fail-closed branches.
createWorkspaceFileOpenClawConfigurationBackupSourcerejects three conditions that only the read result can carry:result.sourceSizeBytes !== undefined,result.truncated === true, andresult.bytes.byteLength !== node.sizeBytes. The current cases override the node only, plus one revision mismatch. A regression that drops those three checks would still pass this suite.
readerFixturealready acceptsresultOverrides, so the additions are small.💚 Proposed additional cases
const mismatched = readerFixture({}, { revision: "b".repeat(64) }); const mismatchFailure = await createWorkspaceFileOpenClawConfigurationBackupSource(mismatched.reader) .read() .catch((error: unknown) => error); expect(mismatchFailure).toMatchObject({ reason: "invalid-source" }); + + for (const resultOverrides of [ + { sourceSizeBytes: bytes.byteLength + 1 }, + { truncated: true as const }, + { bytes: bytes.subarray(0, bytes.byteLength - 1) }, + ]) { + const fixture = readerFixture({}, resultOverrides); + const failure = await createWorkspaceFileOpenClawConfigurationBackupSource( + fixture.reader + ) + .read() + .catch((error: unknown) => error); + expect(failure).toMatchObject({ reason: "invalid-source" }); + expect(fixture.calls).toHaveLength(2); + }🤖 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 `@greenfield/src/server/domains/openClawSettings/configurationBackup.test.ts` around lines 132 - 156, Add cases to the “rejects redacted, truncated, oversized, or inconsistent source metadata” test using readerFixture’s resultOverrides for each read-result-only condition: a defined result.sourceSizeBytes, result.truncated set to true, and result.bytes with a byteLength different from node.sizeBytes. Assert each rejects with OpenClawConfigurationBackupError reason "invalid-source" and preserves the expected reader-call behavior.greenfield/src/server/domains/openClawSettings/configurationBackupTickets.ts (1)
250-298: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate the actor before generating a ticket ID.
issuecallsactorKey(actor)only at Line 289, inside thetickets.setargument. An invalid actor therefore fails after the capacity check, ID generation, and result parsing have run. State stays consistent, because the throw happens beforetickets.setand before thestoredBytesupdate. Moving theactorKey(actor)call to the top ofissuemakes the fail-closed order obvious and avoids consuming a generated ID.♻️ Proposed reordering
issue(actor: OpenClawConfigurationBackupActor, bytes: Uint8Array) { + const key = actorKey(actor); const at = now(); sweepExpired(at); scheduleExpiry(at); @@ const stored = Uint8Array.from(bytes); tickets.set(ticketId, { - actorKey: actorKey(actor), + actorKey: key, bytes: stored, expiresAtMs, ticketId, });🤖 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 `@greenfield/src/server/domains/openClawSettings/configurationBackupTickets.ts` around lines 250 - 298, Update issue in the OpenClaw configuration backup ticket store to call actorKey(actor) at the start of the method, before sweeping, capacity checks, ID generation, or result parsing. Reuse the validated actor key in the tickets.set entry instead of invoking actorKey(actor) there, while preserving the existing successful issuance behavior.greenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.test.ts (1)
194-210: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert
content-dispositionfor active and unknown content.
greenfield/docs/architecture/greenfield-rewrite/data-and-security.mdstates that SVG, HTML, unknown, and other active content stays download-only. This test verifies thecontent-typedowngrade toapplication/octet-stream, but never asserts the disposition header. A regression that keeps the MIME downgrade and drops the attachment disposition would still pass.Add the disposition assertions alongside the existing MIME checks.
💚 Proposed additional assertions
expect(text.headers.get("content-type")).toBe("text/plain"); expect(active.headers.get("content-type")).toBe("application/octet-stream"); expect(spoofed.headers.get("content-type")).toBe("application/octet-stream"); + expect(active.headers.get("content-disposition")).toContain("attachment"); + expect(spoofed.headers.get("content-disposition")).toContain("attachment");🤖 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 `@greenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.test.ts` around lines 194 - 210, Extend the test “only advertises sniffed passive media and allowlisted UTF-8 text” to assert that active SVG and unknown spoofed content responses include the download-only content-disposition header, alongside their existing application/octet-stream content-type checks. Keep the text response assertions unchanged.greenfield/src/server/test/system/serverFoundation.test.ts (1)
301-328: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the pass-through branch of the backup handler.
The stub returns
undefinedfor non-backup paths at Lines 302-308, but no request in this test reaches that branch. The??chain ingreenfield/src/app/server.tsdepends onundefinedfalling through to the chat handler. Add one request to an unrelated path in the same test. That asserts both precedence and pass-through.💚 Proposed additional assertions
expect(response.status).toBe(200); expect(await response.text()).toBe("configuration-backup"); expect(observed).toEqual([ "backup:/api/openclaw-settings/configuration-backups/fixture-ticket", ]); + + const passthrough = await fetch(new URL("/other", server.url)); + expect(await passthrough.text()).toBe("chat-fallback"); + expect(observed).toEqual([ + "backup:/api/openclaw-settings/configuration-backups/fixture-ticket", + "chat:/other", + ]); });🤖 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 `@greenfield/src/server/test/system/serverFoundation.test.ts` around lines 301 - 328, Extend the test around the existing openclaw backup request to fetch an unrelated path that does not match the backup prefix, exercising the undefined return from openClawConfigurationBackupRawHttpHandler and the subsequent chat-handler fallback. Assert the fallback response and update observed-call expectations to verify backup-handler precedence and pass-through behavior.greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.test.ts (1)
58-83: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose every ticket store created by
fixture.These stores retain secret bytes and schedule expiry timers until disposal or expiry. Add test teardown for all fixture-created stores.
🤖 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 `@greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.test.ts` around lines 58 - 83, Update the fixture helper to ensure every createOpenClawConfigurationBackupTicketStore instance is disposed during test teardown, including stores created across multiple fixture calls. Register cleanup through the test framework’s teardown mechanism while preserving the existing handler and tickets return values.greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.ts (1)
111-169: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCopy
bytesbefore enqueueing it.The default
ReadableStreamcontroller retains the enqueuedUint8Array. A pendingBun.serveresponse write can therefore observe the zeroed buffer afterrelease()runs. Enqueuebytes.slice(), then zero the original buffer before returning. Retainrelease()for lease cleanup and aborts.🤖 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 `@greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.ts` around lines 111 - 169, Update secretDownloadBody so pull enqueues a copied buffer via bytes.slice() rather than the original bytes, then zero the original buffer before returning from the first pull. Preserve release() for lease cleanup and abort handling without allowing it to zero the buffer retained by the stream.greenfield/src/server/domains/openClawSettings/restartQueue.ts (1)
105-121: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap the final contract parse so every failure stays sanitized.
terminalResultclassifies each failure asOpenClawGatewayRestartQueueError, except the last step. Line 117 callsv.parse, which throws a rawValiError. That error type is not part ofOpenClawGatewayRestartQueueErrorReason, so the caller cannot classify it, andgreenfield/src/server/domains/openClawSettings/routes.tsmaps it to a generic internal error instead ofSERVICE_UNAVAILABLE. The class comment states that persisted job diagnostics remain inside Jobs; a raw schema error breaks that guarantee.Use
v.safeParseand reuse theunknown-outcomereason.♻️ Proposed fix for the unsanitized parse
- return v.parse(restartOpenClawGatewayResultSchema, { - ...result.output, - jobRunId: run.id, - }); + const contract = v.safeParse(restartOpenClawGatewayResultSchema, { + ...result.output, + jobRunId: run.id, + }); + if (!contract.success) { + throw new OpenClawGatewayRestartQueueError("unknown-outcome"); + } + return contract.output;🤖 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 `@greenfield/src/server/domains/openClawSettings/restartQueue.ts` around lines 105 - 121, Update terminalResult to validate the final restartOpenClawGatewayResultSchema contract with v.safeParse instead of v.parse. If that validation fails, throw OpenClawGatewayRestartQueueError with the existing unknown-outcome reason; otherwise return the parsed output with jobRunId preserved.greenfield/src/app/dashboardServer.ts (1)
514-517: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the synchronous disposals so later cleanup still runs.
Every asynchronous disposal above records its failure and continues. The two new synchronous calls do not. If
openClawConfigurationBackupTickets.dispose()throws,openClawLocalHistoryMediaFetcher,chatAttachmentStore, andchatMediaReferencesare never disposed, and the recordedfailureis replaced by the new throw. Wrap the synchronous disposals in the same pattern.♻️ Proposed fix for disposal ordering
- openClawConfigurationBackupTickets?.dispose(); - openClawLocalHistoryMediaFetcher?.dispose(); - chatAttachmentStore?.dispose(); - chatMediaReferences?.dispose(); + for (const disposeSync of [ + () => openClawConfigurationBackupTickets?.dispose(), + () => openClawLocalHistoryMediaFetcher?.dispose(), + () => chatAttachmentStore?.dispose(), + () => chatMediaReferences?.dispose(), + ]) { + try { + disposeSync(); + } catch (error) { + failure ??= error; + } + }🤖 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 `@greenfield/src/app/dashboardServer.ts` around lines 514 - 517, Update the synchronous disposal calls for openClawConfigurationBackupTickets, openClawLocalHistoryMediaFetcher, chatAttachmentStore, and chatMediaReferences to use the same guarded failure-recording pattern as the preceding asynchronous disposals. Ensure each disposal runs independently, later cleanup continues after an exception, and the existing recorded failure is preserved rather than replaced.greenfield/src/worker/openClaw/gatewayLifecycle.ts (1)
80-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPreserve the failure cause so a broken restart is diagnosable.
Line 103 catches every spawn failure and throws a fixed message with no
cause. The worker therefore cannot log why the restart failed.ENOENT,EACCES, and abort all produce the same output.The executable path amplifies this. Line 84 infers
homeDirectoryfrompath.dirname(openClawRoot), and line 85 derives<home>/.local/bin/openclaw.requiredOpenClawRootdoes not verify that this executable exists. If a deployment places the OpenClaw root outside a home directory, every restart fails with the same generic message and no diagnostic.Keep the fixed message and attach the cause. Consider a construction-time existence check so a misconfiguration fails at worker startup instead of at the first operator-triggered restart.
♻️ Proposed fix to retain the cause
let exitCode: number; try { exitCode = await restartProcess.run(argv, environment, operationSignal); - } catch { - throw new Error("OpenClaw Gateway restart process failed"); + } catch (error) { + throw new Error("OpenClaw Gateway restart process failed", { + cause: error, + }); } if (exitCode !== 0) { - throw new Error("OpenClaw Gateway restart process failed"); + throw new Error("OpenClaw Gateway restart process failed", { + cause: new Error(`exit code ${exitCode}`), + }); }🤖 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 `@greenfield/src/worker/openClaw/gatewayLifecycle.ts` around lines 80 - 110, Update the restart method in createFixedOpenClawGatewayLifecycle to preserve the caught process error as the cause when throwing the existing “OpenClaw Gateway restart process failed” message; retain the nonzero-exit behavior unchanged. Do not infer or add unrelated executable validation unless the existing construction-time helpers support it.greenfield/src/browser/settings/SettingsRoute.test.tsx (1)
866-953: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for a declined confirmation dialog.
Every restart and discard test mocks
globalThis.confirmto returntrue. The guards ingreenfield/src/browser/settings/OpenClawSettingsPanel.tsx(lines 202-208 and 232-240) are the only protection against an accidental second Gateway restart, and no test proves that a declined dialog suppresses the action.Add one case where
confirmreturnsfalse. Assert that noopenClawSettings.restartGatewaymutation is issued and that the recovery key is unchanged.Also applies to: 1020-1099
🤖 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 `@greenfield/src/browser/settings/SettingsRoute.test.tsx` around lines 866 - 953, Add a test covering a declined confirmation in the restart flow, using globalThis.confirm mocked to return false. After attempting the restart, assert that no openClawSettings.restartGateway mutation is issued and the recovery key in sessionStorage remains unchanged; apply equivalent coverage to the discard-recovery-key flow referenced by the existing tests.greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts (1)
503-517: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
localHistoryMediaMaximumfor the per-array bound.Line 503 hardcodes
32while line 517 useslocalHistoryMediaMaximum, which is also32. The two bounds must stay equal. Use the named constant in both places so a future change to the limit cannot diverge.♻️ Proposed refactor
- if ([canonical, paths, urls, types].some((values) => values.length > 32)) { + if ( + [canonical, paths, urls, types].some( + (values) => values.length > localHistoryMediaMaximum + ) + ) { return undefined; }🤖 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 `@greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts` around lines 503 - 517, Replace the hardcoded 32 in the per-array length check within the media validation flow with localHistoryMediaMaximum, keeping the existing slots check and rejection behavior unchanged.greenfield/src/server/rawHttp/chatMedia.test.ts (1)
1304-1306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the exact dispatched request instead of a substring absence.
Line 1306 checks that the serialized local requests do not contain
"/home/". The request is built fromsegments: ["history", "diagram.png"], so that string can never appear regardless of the implementation. The assertion cannot fail and does not prove that filesystem paths are absent. Assert the exact dispatched object so a future change that adds a path field fails the test.♻️ Proposed refactor
expect(managedRequests).toHaveLength(1); expect(localRequests).toHaveLength(1); - expect(JSON.stringify(localRequests)).not.toContain("/home/"); + expect(localRequests[0]).toEqual({ + ...sharedRequest, + source: { + kind: "openclaw-local-history", + segments: ["history", "diagram.png"], + }, + });🤖 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 `@greenfield/src/server/rawHttp/chatMedia.test.ts` around lines 1304 - 1306, Replace the ineffective substring assertion in the test around managedRequests and localRequests with an exact assertion on the dispatched local request object, including its expected segments and all relevant fields. Ensure the assertion would fail if a filesystem path such as “/home/” were added.
🤖 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 `@greenfield/src/app/dashboardServer.ts`:
- Around line 749-768: Keep openClawFileRoot out of the shared writable Files
surface created by createDescriptorWorkspaceFileReader for
workspaceFilesService. Configure that reader with only workspaceFileRoot, and
use a separate backup-only reader for OpenClaw configuration if required; ensure
its manifest entries are non-writable and secret reveal remains denied.
In
`@greenfield/src/server/domains/openClawSettings/configurationBackupTickets.ts`:
- Around line 230-236: The plaintext configuration bytes lack clear erasure
ownership across the export flow. In
greenfield/src/server/domains/openClawSettings/configurationBackupTickets.ts:230-236,
update consume/resolve so the buffer removed by remove(record, false) is tracked
and zeroed after use, or explicitly make the raw HTTP handler erase it in
finally. In
greenfield/src/server/domains/openClawSettings/configurationBackupSource.ts:42-96,
ensure the Uint8Array returned by read is zeroed by the calling service on both
issue success and failure paths. In
greenfield/docs/architecture/greenfield-rewrite/data-and-security.md:562-568,
name the component responsible for erasure or add the missing erase there.
In
`@greenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.ts`:
- Around line 303-327: Replace the synchronous Fs.readSync loop in readExact
with asynchronous FileHandle.read calls, making readExact, readPrefix, and both
fetch call sites await the reads. Update OpenFile to retain and expose the final
FileHandle from Fs.promises.open, and pass that handle through the prefix and
selected-range reads while preserving abort checks and exact-length validation.
- Around line 478-481: Remove the early rangeNotSatisfiableResponse(0) return in
the parsedRange handling within the media fetcher, leaving unparsable
request.range values as undefined so the existing logic serves the complete
representation with status 200. Preserve range processing for successfully
parsed ranges.
In `@greenfield/src/server/platform/files/descriptorWorkspaceFileReader.ts`:
- Around line 554-572: Update the caller of stablePrefixBytes to pass
Math.min(workspaceFileLimits.maximumTextPreviewBytes, entry.maximumSizeBytes) as
the requested prefix size. Preserve the existing consistency check by not
clamping to numberSize(stat), ensuring the requested size remains below the
source size.
In `@greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts`:
- Around line 834-853: Update the visibleText helper so parsed.overflow
contributes to directiveOverflow only when acceptsDirectiveMedia is true,
matching the existing candidate collection gate. Preserve overflow tracking for
assistant messages that accept directive media while ignoring discarded
candidates from other roles.
- Around line 518-534: Update resolveMediaFactsWithPrecedence so each slot
resolves the path and URL independently before choosing the candidate. In the
loop using canonicalPath, canonicalUrl, legacyPath, and legacyUrl, select the
resolved path first and only fall back to the resolved URL, preserving both
values when a canonical URL and legacy path coexist.
In `@greenfield/src/server/rawHttp/chatMedia.ts`:
- Around line 717-739: Update the refresh initialization around
mediaReferenceRefresh so the deadline callback also clears the shared slot when
it fires, guarded to remove only the current refresh. Declare or capture the
in-flight refresh state before creating deadlineHandle as needed, while
preserving the existing work-finally cleanup and cooldown behavior.
---
Outside diff comments:
In `@greenfield/src/server/domains/jobs/workerRuntime.ts`:
- Around line 402-409: In the worker runtime flow around
createJobWorkerActionResolver and createCoordinator, build one conditional
action-definition list that includes the resolved openclaw.gateway.restart
definition when openClawGateway exists without adding it to
jobActionDefinitions. Pass this shared list to both the resolver and
coordinator, preserving the resolver’s existing behavior and preventing
duplicate action keys.
---
Nitpick comments:
In `@greenfield/src/app/dashboardServer.ts`:
- Around line 514-517: Update the synchronous disposal calls for
openClawConfigurationBackupTickets, openClawLocalHistoryMediaFetcher,
chatAttachmentStore, and chatMediaReferences to use the same guarded
failure-recording pattern as the preceding asynchronous disposals. Ensure each
disposal runs independently, later cleanup continues after an exception, and the
existing recorded failure is preserved rather than replaced.
In `@greenfield/src/browser/settings/SettingsRoute.test.tsx`:
- Around line 866-953: Add a test covering a declined confirmation in the
restart flow, using globalThis.confirm mocked to return false. After attempting
the restart, assert that no openClawSettings.restartGateway mutation is issued
and the recovery key in sessionStorage remains unchanged; apply equivalent
coverage to the discard-recovery-key flow referenced by the existing tests.
In `@greenfield/src/contracts/openClawSettings.ts`:
- Around line 758-774: Update the exported openClawSettingsRawHttpContracts
declaration to use an immutable literal tuple with `as const satisfies readonly
RawHttpContract[]`, matching the pattern used by other contract lists such as
workspaceFileRawHttpContracts. Preserve the existing mapped GET/HEAD contract
contents while ensuring the readonly conformance check remains active.
In `@greenfield/src/server/domains/openClawSettings/configurationBackup.test.ts`:
- Around line 132-156: Add cases to the “rejects redacted, truncated, oversized,
or inconsistent source metadata” test using readerFixture’s resultOverrides for
each read-result-only condition: a defined result.sourceSizeBytes,
result.truncated set to true, and result.bytes with a byteLength different from
node.sizeBytes. Assert each rejects with OpenClawConfigurationBackupError reason
"invalid-source" and preserves the expected reader-call behavior.
In
`@greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.test.ts`:
- Around line 58-83: Update the fixture helper to ensure every
createOpenClawConfigurationBackupTicketStore instance is disposed during test
teardown, including stores created across multiple fixture calls. Register
cleanup through the test framework’s teardown mechanism while preserving the
existing handler and tickets return values.
In
`@greenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.ts`:
- Around line 111-169: Update secretDownloadBody so pull enqueues a copied
buffer via bytes.slice() rather than the original bytes, then zero the original
buffer before returning from the first pull. Preserve release() for lease
cleanup and abort handling without allowing it to zero the buffer retained by
the stream.
In
`@greenfield/src/server/domains/openClawSettings/configurationBackupTickets.ts`:
- Around line 250-298: Update issue in the OpenClaw configuration backup ticket
store to call actorKey(actor) at the start of the method, before sweeping,
capacity checks, ID generation, or result parsing. Reuse the validated actor key
in the tickets.set entry instead of invoking actorKey(actor) there, while
preserving the existing successful issuance behavior.
In `@greenfield/src/server/domains/openClawSettings/restartQueue.ts`:
- Around line 105-121: Update terminalResult to validate the final
restartOpenClawGatewayResultSchema contract with v.safeParse instead of v.parse.
If that validation fails, throw OpenClawGatewayRestartQueueError with the
existing unknown-outcome reason; otherwise return the parsed output with
jobRunId preserved.
In
`@greenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.test.ts`:
- Around line 194-210: Extend the test “only advertises sniffed passive media
and allowlisted UTF-8 text” to assert that active SVG and unknown spoofed
content responses include the download-only content-disposition header,
alongside their existing application/octet-stream content-type checks. Keep the
text response assertions unchanged.
In `@greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts`:
- Around line 503-517: Replace the hardcoded 32 in the per-array length check
within the media validation flow with localHistoryMediaMaximum, keeping the
existing slots check and rejection behavior unchanged.
In `@greenfield/src/server/rawHttp/chatMedia.test.ts`:
- Around line 1304-1306: Replace the ineffective substring assertion in the test
around managedRequests and localRequests with an exact assertion on the
dispatched local request object, including its expected segments and all
relevant fields. Ensure the assertion would fail if a filesystem path such as
“/home/” were added.
In `@greenfield/src/server/test/system/serverFoundation.test.ts`:
- Around line 301-328: Extend the test around the existing openclaw backup
request to fetch an unrelated path that does not match the backup prefix,
exercising the undefined return from openClawConfigurationBackupRawHttpHandler
and the subsequent chat-handler fallback. Assert the fallback response and
update observed-call expectations to verify backup-handler precedence and
pass-through behavior.
In `@greenfield/src/worker/openClaw/gatewayLifecycle.test.ts`:
- Around line 20-34: Update the restart environment assertion in the gateway
lifecycle test to require the exact fixed environment object/key set, replacing
the partial toMatchObject and two-name denylist checks. Preserve the expected
HOME, OPENCLAW_NO_RESPAWN, and PATH values while ensuring no additional
environment keys are accepted.
In `@greenfield/src/worker/openClaw/gatewayLifecycle.ts`:
- Around line 80-110: Update the restart method in
createFixedOpenClawGatewayLifecycle to preserve the caught process error as the
cause when throwing the existing “OpenClaw Gateway restart process failed”
message; retain the nonzero-exit behavior unchanged. Do not infer or add
unrelated executable validation unless the existing construction-time helpers
support it.
🪄 Autofix
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: e7911898-bf0b-46d3-9031-9a25b0613f1e
⛔ Files ignored due to path filters (10)
greenfield/docs/generated/configuration.mdis excluded by!**/generated/**and included by**/*greenfield/docs/generated/procedures.mdis excluded by!**/generated/**and included by**/*greenfield/docs/generated/raw-http.mdis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/files.list.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/files.prepareContent.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/files.prepareReveal.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/openClawSettings.createConfigurationBackup.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/openClawSettings.createConfigurationBackup.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/openClawSettings.restartGateway.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/openClawSettings.restartGateway.output.schema.jsonis excluded by!**/generated/**and included by**/*
📒 Files selected for processing (82)
greenfield/.bun-browser-test-timings.jsongreenfield/.bun-test-timings.jsongreenfield/docs/architecture/greenfield-rewrite/application-architecture.mdgreenfield/docs/architecture/greenfield-rewrite/data-and-security.mdgreenfield/docs/architecture/greenfield-rewrite/progress.mdgreenfield/docs/architecture/greenfield-rewrite/runtime-and-delivery.mdgreenfield/scripts/audits/openclaw/fixtures/2026.7.2-beta.7/chat.jsongreenfield/scripts/audits/openclaw/fixtures/2026.7.2-beta.7/manifest.jsongreenfield/scripts/audits/openclaw/sourceAudit.tsgreenfield/scripts/audits/openclaw/sourceAuditSchemas.tsgreenfield/scripts/documentation/jsonSchema.tsgreenfield/src/app/dashboardServer.test.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/app/developmentWorker.tsgreenfield/src/app/server.tsgreenfield/src/app/worker.test.tsgreenfield/src/app/worker.tsgreenfield/src/browser/files/WorkspaceFileEditorPane.test.tsxgreenfield/src/browser/files/WorkspaceFileEditorPane.tsxgreenfield/src/browser/files/WorkspaceFileTable.test.tsxgreenfield/src/browser/files/WorkspaceFileTable.tsxgreenfield/src/browser/files/workspaceFilePresentation.tsgreenfield/src/browser/files/workspaceFileTransfers.test.tsgreenfield/src/browser/files/workspaceFileTransfers.tsgreenfield/src/browser/settings/OpenClawSettingsPanel.tsxgreenfield/src/browser/settings/SettingsRoute.test.tsxgreenfield/src/browser/settings/openClawSettingsMutations.tsgreenfield/src/browser/settings/openClawSettingsOperations.tsgreenfield/src/contracts/chatMedia.tsgreenfield/src/contracts/contractRegistry.tsgreenfield/src/contracts/files.test.tsgreenfield/src/contracts/files.tsgreenfield/src/contracts/openClawSettings.test.tsgreenfield/src/contracts/openClawSettings.tsgreenfield/src/server/domains/files/ports.tsgreenfield/src/server/domains/files/rawHttp.test.tsgreenfield/src/server/domains/files/rawHttp.tsgreenfield/src/server/domains/files/service.test.tsgreenfield/src/server/domains/files/service.tsgreenfield/src/server/domains/jobs/actionExecutors.test.tsgreenfield/src/server/domains/jobs/actionExecutors.tsgreenfield/src/server/domains/jobs/actionRegistry.test.tsgreenfield/src/server/domains/jobs/actionRegistry.tsgreenfield/src/server/domains/jobs/workerRuntime.test.tsgreenfield/src/server/domains/jobs/workerRuntime.tsgreenfield/src/server/domains/openClawSettings/configurationBackup.test.tsgreenfield/src/server/domains/openClawSettings/configurationBackup.tsgreenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.test.tsgreenfield/src/server/domains/openClawSettings/configurationBackupRawHttp.tsgreenfield/src/server/domains/openClawSettings/configurationBackupSource.tsgreenfield/src/server/domains/openClawSettings/configurationBackupTickets.tsgreenfield/src/server/domains/openClawSettings/operationAudit.tsgreenfield/src/server/domains/openClawSettings/procedures.test.tsgreenfield/src/server/domains/openClawSettings/restartQueue.test.tsgreenfield/src/server/domains/openClawSettings/restartQueue.tsgreenfield/src/server/domains/openClawSettings/routes.tsgreenfield/src/server/domains/openClawSettings/service.test.tsgreenfield/src/server/domains/openClawSettings/service.tsgreenfield/src/server/domains/security/authenticationSession.tsgreenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.test.tsgreenfield/src/server/platform/chat/descriptorOpenClawLocalHistoryMediaFetcher.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.test.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.tsgreenfield/src/server/platform/files/descriptorWorkspaceFileReader.test.tsgreenfield/src/server/platform/files/descriptorWorkspaceFileReader.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.tsgreenfield/src/server/platform/observability/structuredLogger.test.tsgreenfield/src/server/platform/observability/structuredLogger.tsgreenfield/src/server/rawHttp/chatMedia.test.tsgreenfield/src/server/rawHttp/chatMedia.tsgreenfield/src/server/test/support/requestContext.tsgreenfield/src/server/test/system/serverFoundation.test.tsgreenfield/src/server/trpc/procedureErrorPolicy.tsgreenfield/src/shared/configuration/applicationConfigurationRegistry.tsgreenfield/src/shared/openClawGatewayLifecycle.tsgreenfield/src/test/integration/openclaw/sourceAudit.test.tsgreenfield/src/test/parity/fixtures/greenfield-contracts.jsongreenfield/src/test/parity/fixtures/legacy-endpoints.jsongreenfield/src/test/parity/parityInventory.test.tsgreenfield/src/worker/openClaw/gatewayLifecycle.test.tsgreenfield/src/worker/openClaw/gatewayLifecycle.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0531d440b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e4764e662
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50a03e3db2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4323b1e7cc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f90a8c11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 551071ed5f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (2)
greenfield/src/app/dashboardServer.ts (1)
396-404: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDo not advance the legacy fallback offset when the refresh read nothing.
In
"targeted"mode with ambiguous routing,sessionsis empty andsessionsReadis0, but this block still advanceslegacyFallbackSessionOffsetby1. Repeated targeted misses then shift the legacy rotation start without reading any session. The rotation still covers every session over time, so the impact is fairness drift only. Gate the advance onsessionsRead > 0.♻️ Proposed change
if ( attachmentId !== undefined && routedSessions.length === 0 && + sessionsRead > 0 && fallbackSessionCount > 0 ) { legacyFallbackSessionOffset = - (legacyFallbackSessionOffset + Math.max(1, sessionsRead)) % - fallbackSessionCount; + (legacyFallbackSessionOffset + sessionsRead) % fallbackSessionCount; }🤖 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 `@greenfield/src/app/dashboardServer.ts` around lines 396 - 404, Update the legacy fallback offset advancement condition in the targeted-session fallback block so it only runs when sessionsRead > 0, while preserving the existing attachmentId, routedSessions, and fallbackSessionCount guards and rotation calculation.greenfield/src/server/rawHttp/chatMedia.ts (1)
260-324: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge
waitForRefreshandwaitForSharedRefresh.The two helpers share the same abort-listener, single-settle, and error-normalization logic. Only the
onSettledcallback differs. MakeonSettledan optional parameter of one helper and delete the duplicate. This removes about thirty duplicated lines and keeps one abort contract to maintain.🤖 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 `@greenfield/src/server/rawHttp/chatMedia.ts` around lines 260 - 324, Merge waitForRefresh and waitForSharedRefresh into a single helper by adding an optional onSettled callback parameter to waitForRefresh. Invoke it during the single-settle cleanup when provided, preserve the existing abort handling and error normalization, update shared-refresh callers to pass the callback, and delete waitForSharedRefresh.
🤖 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 `@greenfield/src/app/dashboardServer.ts`:
- Around line 396-404: Update the legacy fallback offset advancement condition
in the targeted-session fallback block so it only runs when sessionsRead > 0,
while preserving the existing attachmentId, routedSessions, and
fallbackSessionCount guards and rotation calculation.
In `@greenfield/src/server/rawHttp/chatMedia.ts`:
- Around line 260-324: Merge waitForRefresh and waitForSharedRefresh into a
single helper by adding an optional onSettled callback parameter to
waitForRefresh. Invoke it during the single-settle cleanup when provided,
preserve the existing abort handling and error normalization, update
shared-refresh callers to pass the callback, and delete waitForSharedRefresh.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a12b8139-b1a9-4d55-ba77-ab730c419271
📒 Files selected for processing (11)
greenfield/docs/architecture/greenfield-rewrite/application-architecture.mdgreenfield/docs/architecture/greenfield-rewrite/data-and-security.mdgreenfield/docs/architecture/greenfield-rewrite/runtime-and-delivery.mdgreenfield/src/app/dashboardServer.test.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.test.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.tsgreenfield/src/server/rawHttp/chatMedia.test.tsgreenfield/src/server/rawHttp/chatMedia.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- greenfield/docs/architecture/greenfield-rewrite/runtime-and-delivery.md
- greenfield/docs/architecture/greenfield-rewrite/application-architecture.md
- greenfield/docs/architecture/greenfield-rewrite/data-and-security.md
- greenfield/src/server/platform/chat/inMemoryChatMediaReferences.test.ts
- greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts
- greenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 421
File: greenfield/src/app/dashboardServer.ts:734-753
Timestamp: 2026-08-12T04:22:28.232Z
Learning: In `greenfield/src/server/platform/files/openClawFileRootConfiguration.ts`, the reviewed `openclaw-config` root intentionally remains non-writable while its fixed manifest permits replacement of only `openclaw.json` and `hooks/transforms/agentmail.ts`. `openclaw.json` replacement requires the existing actor-bound secret-reveal and CAS revision controls. The OpenClaw configuration backup is independently restricted to the exact `openclaw-config/openclaw.json` locator.
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 0
File: :0-0
Timestamp: 2026-07-22T14:45:35.974Z
Learning: In `src/components/features/chat/transport/openClawRuntimeAdapter.ts`, media on a Synthetic `toolUse` assistant turn is provider-turn-scoped rather than tool-call-scoped. When one turn contains multiple tool calls, exactly one sibling tool draft must own the media to preserve it without rendering duplicate images or attachments. `src/test/openClawAdapterVariants.test.ts` covers this behavior in the `owns turn-scoped Synthetic media once across multiple tool drafts` regression.
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 365
File: backend/src/gateway.ts:1727-1736
Timestamp: 2026-08-01T06:32:15.434Z
Learning: For `backend/src/gateway.ts`, installed OpenClaw v2026.6.11 accepts only `sessionKey`, `agentId`, `message`, and `label` in the public `chat.inject` schema. It rejects unknown properties such as `idempotencyKey`. The Dashboard task notification flow does not retry `sendSessionControlEvent` after a partial inject-then-wake failure; it logs the failure and keeps the durable control notice visible.
📚 Learning: 2026-07-22T14:45:35.974Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 0
File: :0-0
Timestamp: 2026-07-22T14:45:35.974Z
Learning: In `src/components/features/chat/transport/openClawRuntimeAdapter.ts`, media on a Synthetic `toolUse` assistant turn is provider-turn-scoped rather than tool-call-scoped. When one turn contains multiple tool calls, exactly one sibling tool draft must own the media to preserve it without rendering duplicate images or attachments. `src/test/openClawAdapterVariants.test.ts` covers this behavior in the `owns turn-scoped Synthetic media once across multiple tool drafts` regression.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/server/rawHttp/chatMedia.ts
📚 Learning: 2026-08-07T18:48:18.623Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 398
File: greenfield/src/server/database/validation/notifications.ts:48-49
Timestamp: 2026-08-07T18:48:18.623Z
Learning: In `greenfield/src/server/database/validation/notifications.ts`, Drizzle Valibot generated notification schemas retain nullable and optional behavior for the `reportId: uuidV7TextSchema` and `source: () => monitoringReportSourceSchema` refinements. Regression tests in `greenfield/src/server/database/validation/notifications.test.ts` cover `null` values for both fields in insert and select schemas, plus omission of both fields in the insert schema.
Applied to files:
greenfield/src/app/dashboardServer.test.ts
📚 Learning: 2026-08-04T04:04:49.381Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 379
File: src/app/server.ts:3-26
Timestamp: 2026-08-04T04:04:49.381Z
Learning: In the Mira-Dashboard TypeScript repository, `src/app/server.ts:createServer` is the side-by-side rewrite composition and is not connected to the legacy production backend startup path in `backend/src/serverStart.ts`. The legacy backend owns separate dynamic readiness handling. Do not recommend wiring rewrite `ReadinessState` or `ReadinessController` into `backend/src/serverStart.ts` unless a pull request explicitly integrates the rewrite architecture. Rewrite readiness promotion must remain gated on future configuration, checksum-verified database startup, and declared critical dependencies.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/app/dashboardServer.ts
📚 Learning: 2026-08-01T06:32:15.434Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 365
File: backend/src/gateway.ts:1727-1736
Timestamp: 2026-08-01T06:32:15.434Z
Learning: For `backend/src/gateway.ts`, installed OpenClaw v2026.6.11 accepts only `sessionKey`, `agentId`, `message`, and `label` in the public `chat.inject` schema. It rejects unknown properties such as `idempotencyKey`. The Dashboard task notification flow does not retry `sendSessionControlEvent` after a partial inject-then-wake failure; it logs the failure and keeps the durable control notice visible.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/server/rawHttp/chatMedia.ts
📚 Learning: 2026-08-07T18:47:59.017Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 398
File: greenfield/src/server/domains/monitoring/routeEffects.ts:21-63
Timestamp: 2026-08-07T18:47:59.017Z
Learning: In the Greenfield TypeScript application, `greenfield/package.json` and `greenfield/bun.lock` pin `effect` to `4.0.0-beta.104`. In this version, `Effect.runPromise` rejects with the original typed domain-error instance, so `instanceof` checks in `greenfield/src/server/domains/monitoring/routeEffects.ts` remain valid. The route-level assertions in `greenfield/src/server/domains/monitoring/procedures.test.ts` cover the monitoring tRPC mappings, including `BAD_REQUEST`, `CONFLICT`, `NOT_FOUND`, and `PRECONDITION_FAILED`.
Applied to files:
greenfield/src/app/dashboardServer.test.ts
📚 Learning: 2026-07-14T11:31:42.796Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 277
File: src/test/pageBehavior.test.tsx:1887-1912
Timestamp: 2026-07-14T11:31:42.796Z
Learning: In `src/test/pageBehavior.test.tsx` (Bun test suite for Mira Dashboard pages), test-local overrides of `globalThis.fetch` via `Object.defineProperty` do not need their own `try/finally` restoration. The suite's `afterEach` hook unconditionally restores `globalThis.fetch` to `originalGlobals.fetch` (and resets WebSocket, requestAnimationFrame, cancelAnimationFrame), and `beforeEach` installs a fresh fetch mock before every test, so isolation is already guaranteed even if a test throws.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.ts
📚 Learning: 2026-08-05T14:24:30.572Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 386
File: src/app/server.ts:176-206
Timestamp: 2026-08-05T14:24:30.572Z
Learning: In `src/app/server.ts`, Bun 1.4.0-canary.1 supports `new Request(request, { body, signal })` after the input request body was consumed, because an explicit `init.body` replaces rather than reuses the consumed body. The POST bootstrap system test exercises this bounded-body reconstruction path successfully. Retain this construction because rebuilding only from URL, method, and headers can lose Request metadata or change header-guard behavior.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.ts
📚 Learning: 2026-07-20T22:09:46.707Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 309
File: src/test/chatScroll.test.tsx:540-548
Timestamp: 2026-07-20T22:09:46.707Z
Learning: In `src/components/features/chat/useChatScroll.ts`, `handleUserScrollIntent()` calls `cancelBottomFollow()` without requiring `messagesContainerReference.current`, so tests may intentionally invoke it before attaching the scroll container to cancel an initial queued structural follow. When `composerLayoutKey` changes, the hook invokes `scrollToBottom()` synchronously before scheduling deferred structural settling; tests should assert that immediate bottom alignment before flushing animation frames when validating transient-overlap prevention.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.ts
📚 Learning: 2026-08-07T17:05:36.638Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 397
File: greenfield/src/server/domains/agents/service.test.ts:225-239
Timestamp: 2026-08-07T17:05:36.638Z
Learning: In Bun test files, write rejection assertions as `expect(promise).rejects...` without `await`. The repository's installed matcher types return `void`, and ESLint's `typescript(await-thenable)` rule rejects awaiting these matcher assertions.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.ts
📚 Learning: 2026-08-07T18:47:49.639Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 398
File: greenfield/src/server/domains/monitoring/catalogErrors.ts:3-3
Timestamp: 2026-08-07T18:47:49.639Z
Learning: In the greenfield TypeScript application, use the pinned Effect version 4.0.0-beta.104 API. Preserve `Schema.Literals` for readonly literal tuples and arrays, and do not replace it with variadic `Schema.Literal(...)` unless the replacement has been validated against the pinned Effect version.
Applied to files:
greenfield/src/app/dashboardServer.test.tsgreenfield/src/server/rawHttp/chatMedia.test.tsgreenfield/src/server/platform/chat/inMemoryChatMediaReferences.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/server/rawHttp/chatMedia.ts
📚 Learning: 2026-08-12T04:22:28.232Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 421
File: greenfield/src/app/dashboardServer.ts:734-753
Timestamp: 2026-08-12T04:22:28.232Z
Learning: In `greenfield/src/server/platform/files/openClawFileRootConfiguration.ts`, the reviewed `openclaw-config` root intentionally remains non-writable while its fixed manifest permits replacement of only `openclaw.json` and `hooks/transforms/agentmail.ts`. `openclaw.json` replacement requires the existing actor-bound secret-reveal and CAS revision controls. The OpenClaw configuration backup is independently restricted to the exact `openclaw-config/openclaw.json` locator.
Applied to files:
greenfield/src/app/dashboardServer.ts
📚 Learning: 2026-07-31T19:55:25.605Z
Learnt from: mira-2026
Repo: rajohan/Mira-Dashboard PR: 364
File: backend/src/services/dockerUpdater.ts:124-136
Timestamp: 2026-07-31T19:55:25.605Z
Learning: For `backend/src/services/dockerUpdater.ts`, files under the configured Docker roots are within the high-trust operator boundary. In production, `/opt/docker` is mode `0750` and `/opt/docker/apps` is mode `0775`; both are owned by `ubuntu:ubuntu`, the `ubuntu` group has no additional members, and the only container bind of `/opt/docker` is read-only. Do not report a compose-file TOCTOU issue based only on an authorized operator modifying a managed compose file after `managedComposePath` validation: retaining a read file descriptor does not prevent same-UID in-place writes and breaks normal Docker Compose relative include, environment, and project semantics. The applicable protections are rejection of symlinks, hard links, non-regular files, and canonical paths outside configured Docker roots.
Applied to files:
greenfield/src/app/dashboardServer.ts
🔇 Additional comments (12)
greenfield/src/server/platform/chat/inMemoryChatMediaReferences.ts (6)
10-15: LGTM!
79-122: LGTM!
256-281: LGTM!
288-343: LGTM!
386-418: LGTM!Also applies to: 420-440
345-384: 🔒 Security & PrivacyNo change required: local-media reads are symlink-safe. The fetcher opens each path component relative to an anchored root descriptor with
O_NOFOLLOWand validates each descriptor withisSafeDescendant.> Likely an incorrect or invalid review comment.greenfield/src/app/dashboardServer.test.ts (1)
215-239: LGTM!Also applies to: 420-454, 456-492, 494-526, 528-564
greenfield/src/app/dashboardServer.ts (1)
272-318: LGTM!Also applies to: 320-395, 553-599, 825-886, 943-949, 967-973, 1050-1062, 1136-1153, 1169-1203, 1349-1351
greenfield/src/server/rawHttp/chatMedia.test.ts (1)
13-15: LGTM!Also applies to: 37-37, 122-127, 727-811, 813-884, 918-924, 934-1033, 1035-1094, 1096-1166
greenfield/src/server/rawHttp/chatMedia.ts (3)
25-31: LGTM!Also applies to: 197-209, 220-258, 827-1059, 1163-1163
1185-1197: 🩺 Stability & AvailabilityNo change needed.
proxyMediareleasesdownloadLeasein afinallyblock that covers all returns and thrown errors.> Likely an incorrect or invalid review comment.
1239-1245: 🔒 Security & PrivacyDo not raise this issue. All stored
gateway-managedsources validateupstreamAttachmentIdas a UUID v4, which cannot contain path separators or URL delimiters.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bbe175051
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca988b9063
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf2f9af362
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
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.
Stack
Summary
No new browser media route, path-query API, or media inventory is introduced.
Security and concurrency boundaries
GET, non-consuming onHEAD, and actively zero expired, consumed, and in-flight buffersmedia/root and enforces no-follow traversal, ownership/device/mode/link checks, stable-file checks, MIME policy, ranges, and a 16 MiB ceilingMEDIA:authority; directives are stripped case-insensitively so host paths cannot reach the browserParity
Verification
3e4764e662caa37a1b9d0ba47994dc6e07279e27git diff --check0531d440b535cf3e76eee2e282dce8cbf1801165: OpenClaw source audit 18/18 tests, 241 assertions, 83 hash-pinned artifacts, live pinned hostMATCHbun run test:coverage: 2,637 Bun/backend + 800 browser tests, 93.16% coverage (117,727/126,373)