feat(greenfield): close logs maintenance parity - #416
Conversation
|
Warning Review limit reached
Next review available in: 53 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 (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds durable log-maintenance status, managed dry runs, safe rotation epochs, fail-closed log reads, expanded redaction, dashboard controls, chat activity projections, and ChangesLog maintenance lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant LogsBrowser
participant LogsService
participant JobQueue
participant JobRepository
participant Worker
participant SafeLogReader
Operator->>LogsBrowser: request maintenance
LogsBrowser->>LogsService: submit policy and dryRun
LogsService->>JobQueue: enqueue durable run
JobQueue->>JobRepository: persist payload and audit data
JobQueue->>Worker: execute maintenance
Worker->>JobRepository: persist execution result
LogsBrowser->>JobRepository: observe run status
LogsBrowser->>SafeLogReader: read log snapshot
SafeLogReader-->>LogsBrowser: verified snapshot or hidden data
Possibly related PRs
🚥 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 |
mira-2026
changed the title
mira-2026
added
status: waiting
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/worker/logs/managedLogRotation.ts (1)
549-565: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftA missing source file leaves the epoch permanently in
rotating.
openFileTargetreturnsundefinedwhen the source file does not exist. In that case the function returns themissingresult at Line 559 and never callspublishCopyTruncateEpochState. The persisted entry for that target stays in staterotating.
createLogRotationEpochProberejects any entry whose state is notcommitted, so every later read of that source fails closed, and every later maintenance run repeats the samemissingoutcome without clearing the entry. The source stays unreadable in the dashboard until an operator edits the projection file.Consider settling the epoch when recovery observes a missing source, for example by committing the pending epoch or by removing the entry, before returning the
missingresult.🤖 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/logs/managedLogRotation.ts` around lines 549 - 565, The missing-file branch in the rotation flow leaves the pending epoch in rotating; update the file === undefined handling in the function containing openFileTarget to settle the corresponding epoch before returning the missing result. Reuse the existing publishCopyTruncateEpochState or established epoch-removal mechanism, preserving the current missing response and ensuring subsequent probes see no unresolved rotating entry.
🧹 Nitpick comments (15)
greenfield/src/server/platform/logs/safeLogReader.ts (1)
228-245: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove
await predecessorinside thetryblock to keep the queue alive.
pendingis set beforeawait predecessorruns, and that await sits outside thetry. If the predecessor promise ever rejects, this call throws before thefinally, socompletion.resolve()never runs. The stored promise for thatsourceIdthen never settles, and every later read of that source waits forever.Today the chain cannot reject, because
completion.promiseis only resolved in thefinally. The guard protects the invariant against future changes.♻️ Proposed change
const predecessor = pending.get(sourceId) ?? Promise.resolve(); const completion = Promise.withResolvers<void>(); pending.set(sourceId, completion.promise); - await predecessor; try { + await predecessor.catch(() => {}); return await operation(); } finally { completion.resolve(); if (pending.get(sourceId) === completion.promise) pending.delete(sourceId); }🤖 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/logs/safeLogReader.ts` around lines 228 - 245, Move await predecessor inside the try block in createSourceReadCoordinator so any predecessor rejection still reaches the finally cleanup. Preserve completion.resolve() and the pending-map identity guard, ensuring the queued source operation cannot leave a permanently unsettled promise.greenfield/src/server/platform/logs/redaction.ts (2)
125-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
isSafeValueDelimiterin the unquoted branch.Line 135 repeats the character class that
isSafeValueDelimiteralready defines at Line 77. The two definitions must stay identical. A future change toisSafeValueDelimiterwill not reach this loop, so the unquoted value boundary and the fail-closed boundary can diverge.♻️ Proposed fix
if (consumesCompleteUnquotedValue(secret)) return value.length; let index = start; - while (index < value.length && !/[\r\n,;}&\]]/u.test(value[index]!)) { + while (index < value.length && !isSafeValueDelimiter(value[index]!)) { index += 1; } return index;🤖 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/logs/redaction.ts` around lines 125 - 139, Update the unquoted branch of sensitiveValueEnd to use the existing isSafeValueDelimiter helper instead of duplicating its character-class check, while preserving the current loop boundary and fail-closed behavior.
44-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider one shared scanner for the quote, escape, and bracket state.
structuredValueEndandmalformedValueSuffixEndimplement the same quote, escape, and bracket-stack state machine. They differ only in the stop condition:structuredValueEndstops when the stack empties, andmalformedValueSuffixEndstops on a safe delimiter at depth zero.quotedValueEndrepeats the escape handling a third time.Extracting a single scanner that takes a stop predicate removes the duplicated state handling and keeps the escape rules identical across all three paths.
🤖 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/logs/redaction.ts` around lines 44 - 106, Extract the shared quote, escape, and bracket-stack traversal from structuredValueEnd, malformedValueSuffixEnd, and quotedValueEnd into one scanner that accepts a stop predicate. Update each function to use this scanner while preserving its existing termination behavior: balanced structured values stop when the stack empties, malformed suffixes stop at safe delimiters at depth zero, and quoted values retain their current escape handling.greenfield/src/server/platform/logs/logRotationEpochProbe.test.ts (1)
121-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a noncanonical ordering case or narrow the test name.
The generated entries use
String(index).padStart(2, "0")for indices 0 to 64, so thesourceIdvalues are already sorted ascending. The fixture therefore only violateslogRotationEpochProjectionMaximumEntries. TheentriesAreCanonicalcheck ingreenfield/src/shared/logRotationEpochProjection.tsstays uncovered.Add a second projection whose entries are out of order, or duplicated, to cover the canonical rule.
💚 Proposed additional case
expect(probe.epoch("dashboard.web.stdout")).rejects.toThrow( "Log rotation epoch is unavailable" ); + + await writeProjection(projectionPath, [ + { + epoch, + sourceId: "dashboard.web.stdout", + state: "committed", + }, + { + epoch: otherEpoch, + sourceId: "dashboard.a.stdout", + state: "committed", + }, + ]); + expect(probe.epoch("dashboard.web.stdout")).rejects.toThrow( + "Log rotation epoch is unavailable" + ); });🤖 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/logs/logRotationEpochProbe.test.ts` around lines 121 - 135, Update the test named “rejects noncanonical and over-budget source inventories” to add a separate projection fixture with entries that are out of order or duplicated, then assert that probing it rejects with “Log rotation epoch is unavailable.” Keep the existing 65-entry fixture to cover the maximum-entry limit, and ensure the new case exercises the entriesAreCanonical validation.greenfield/src/server/domains/logs/service.ts (1)
47-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one status type instead of restating the shape.
This inline type repeats the return shape declared by
LogMaintenanceJobQueue.runStatusesingreenfield/src/server/domains/jobs/logMaintenanceQueue.tsLines 43-49, andrunStatusrestates it a third time at Lines 126-130. The three copies must change together. Export one named type and reuse it in all three places.greenfield/src/contracts/logs.tsalready ownsLogMaintenanceActiveRunandLogMaintenanceLastRun, so it is the natural home.♻️ Proposed shared type
// greenfield/src/contracts/logs.ts export type LogMaintenanceRunStatus = Readonly<{ readonly activeRun?: LogMaintenanceActiveRun; readonly lastRun?: LogMaintenanceLastRun; readonly policyId: LogMaintenancePolicyId; }>;export interface LogMaintenanceQueuePort { /** Returns active and latest terminal non-dry-run observations for fixed policies. */ - readonly runStatuses: (signal?: AbortSignal) => Promise< - readonly Readonly<{ - readonly activeRun?: LogMaintenanceActiveRun; - readonly lastRun?: LogMaintenanceLastRun; - readonly policyId: LogMaintenancePolicyId; - }>[] - >; + readonly runStatuses: ( + signal?: AbortSignal + ) => Promise<readonly LogMaintenanceRunStatus[]>;🤖 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/logs/service.ts` around lines 47 - 54, Define and export a shared LogMaintenanceRunStatus type in logs.ts using the existing activeRun, lastRun, and policyId fields, then replace the duplicated inline return shapes in the runStatuses declaration and runStatus method of LogMaintenanceJobQueue and the service runStatuses declaration with this named type.greenfield/src/server/domains/jobs/logMaintenanceQueue.ts (1)
190-234: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that dry-run terminal history is intentionally dropped.
runStatusesmergesmanagedDryRunSnapshot.activeRuninto thedocker-managedstatus but never readsmanagedDryRunSnapshot.lastRun. A completed dry run therefore disappears from status once it reaches a terminal state.greenfield/src/server/domains/logs/service.tsLine 47 states thatlastRuncovers non-dry-run observations only, andgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsLines 306-311 assert this behavior, so the intent is clear. Add a short comment at the merge site so a future reader does not treat the omission as a defect.♻️ Proposed comment
const statuses = realPolicyPayloads.map(({ payloadJson, policyId }) => { const snapshot = snapshotsByPayload.get(payloadJson); if (snapshot === undefined) throw queueFailure(); + // A managed dry run occupies the same single-flight slot, so its + // active record surfaces under `docker-managed`. Its terminal record + // is intentionally dropped: `lastRun` reports real maintenance only. return runStatus(🤖 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/logMaintenanceQueue.ts` around lines 190 - 234, Add a short explanatory comment at the docker-managed merge site in runStatuses, immediately around the preferredActiveRun call, documenting that managedDryRunSnapshot.lastRun is intentionally not used because dry-run terminal history is excluded from status observations. Preserve the existing behavior and logic unchanged.greenfield/src/server/domains/jobs/coordinator.test.ts (1)
1398-1415: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the whole cadence from
definition.defaultSchedule.The override sets
intervalMsfrom the definition but keepsscheduleKind: "interval",cronExpression: null,timeOfDay: null, andtimeZone: nullfromintervalSchedule. If the registered log-maintenance cadence changes todailyorcron, this fixture becomes an interval schedule withintervalMs: null, and the failure will point at cadence math instead of the cadence mapping. Map all cadence fields fromdefinition.defaultSchedule, or assert that the kind isintervalbefore building the schedule.♻️ Proposed guard
if (definition === undefined) throw new Error("Missing log-maintenance definition"); + if (definition.defaultSchedule.kind !== "interval") { + throw new Error("Expected an interval log-maintenance cadence"); + } const schedule = intervalSchedule({ @@ - intervalMs: - definition.defaultSchedule.kind === "interval" - ? definition.defaultSchedule.intervalMs - : null, + intervalMs: definition.defaultSchedule.intervalMs,The assertion itself is correct and covers the new
rejectWhenActionActivecontract.🤖 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/coordinator.test.ts` around lines 1398 - 1415, Update the schedule construction around intervalSchedule to derive every cadence field from definition.defaultSchedule, including schedule kind, interval, cron expression, time of day, and time zone, rather than overriding only intervalMs. Alternatively, explicitly assert that definition.defaultSchedule.kind is "interval" before constructing the fixture, so non-interval cadence changes fail at the mapping boundary.greenfield/src/server/domains/logs/service.test.ts (1)
118-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the
...fixture.servicespread and avoidas never.Two points in the new test:
...fixture.servicespreads aLogsServiceinstance into aLogsServiceDependenciesobject. Every real dependency is overridden below it, so the spread adds only unrelated keys (listSources,maintenanceStatus,search,tail,requestMaintenance). Remove it and pass the dependencies directly.as neveron Line 139 erases theLogMaintenanceActiveRunandLogMaintenanceLastRunshapes. If those contracts change, this fixture keeps compiling while asserting a shape the service can no longer produce.Item 2 needs a full
JobRunSummaryfixture, so a shared test helper is the cheaper path if one already exists.♻️ Proposed change for the spread
- const fixture = dependencies(); const service = createLogsService({ - ...fixture.service, auditWriter: { record: () => Promise.resolve() },🤖 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/logs/service.test.ts` around lines 118 - 145, Update the test setup for “projects an active run without hiding the latest terminal run” to construct the LogsServiceDependencies directly instead of spreading fixture.service, since every required dependency is already overridden. Replace the runStatuses `as never` cast with a properly typed JobRunSummary fixture, reusing an existing shared test helper if available, so the active and terminal run entries satisfy their declared contracts.greenfield/src/server/database/schema/jobRuns.ts (1)
209-222: 🚀 Performance & Scalability | 🔵 TrivialBoth new indexes store the full
payload_jsontext for every matching run.The index key lists include
table.payloadJson, andjob_runs_payload_json_check(Line 141) allows payloads up to 65,536 bytes. The terminal partial index matches every completed run for every action, not onlymaintenance.rotate-logs, so each terminal run duplicates its whole payload inside the index. Actions with large payloads will amplify database size and write cost.The key order and partial predicates match the two queries in
readActionPayloadRunSnapshots, so the index shape itself is correct. Consider one of these if payload sizes grow:
- Index a payload digest column instead of the raw text, and filter on the digest plus an equality check on
payload_json.- Narrow the terminal partial predicate to the action keys that need snapshot reads.
Measure index size on a representative database before deciding.
🤖 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/database/schema/jobRuns.ts` around lines 209 - 222, The new indexes in the schema should not blindly duplicate full payloadJson values for every matching run. Before finalizing job_runs_action_payload_active_idx and job_runs_action_payload_terminal_idx, measure their size and write impact on a representative database, then either index a payload digest while retaining payload_json equality filtering or narrow the terminal predicate to only action keys requiring snapshot reads; preserve the query-compatible key ordering and active/terminal behavior.greenfield/src/server/domains/jobs/repository.ts (1)
791-845: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInline run-state SQL literals are repeated in three places. The typed lists
terminalRunStateList(Line 518) and the implicit active-state pair exist, but each query re-types the state names inside a raw SQL fragment. The literals are required so SQLite can match the partial indexes, so the fix is to render one shared fragment from the typed lists rather than to switch toinArray.
greenfield/src/server/domains/jobs/repository.ts#L791-L845: replace the two inlineIN (...)fragments with sharedactiveStateFilterandterminalStateFiltervalues rendered from the typed state lists, and add a comment stating why literals are needed.greenfield/src/server/domains/jobs/repository.ts#L1321-L1333: use the sharedactiveStateFilterin theenqueueManualRunadmission guard.greenfield/src/server/domains/jobs/repository.ts#L1486-L1500: use the sharedactiveStateFilterin theenqueueNextDueScheduleadmission guard.🤖 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/repository.ts` around lines 791 - 845, In greenfield/src/server/domains/jobs/repository.ts lines 791-845, define shared activeStateFilter and terminalStateFilter SQL fragments rendered from the typed active and terminal state lists, replace both inline IN clauses, and document that literal SQL is required for SQLite partial-index matching. In lines 1321-1333 and 1486-1500, replace each repeated active-state SQL condition in enqueueManualRun and enqueueNextDueSchedule with the shared activeStateFilter; retain the existing query behavior.greenfield/docs/architecture/greenfield-rewrite/data-and-security.md (1)
204-205: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument each partial-index predicate.
Lines 204-205 identify both indexes as partial but omit their
WHEREpredicates. Readers cannot verify predicate and query alignment from this index plan. Add each predicate and its maintenance-action scope.🤖 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/docs/architecture/greenfield-rewrite/data-and-security.md` around lines 204 - 205, Update the active and terminal maintenance status index entries in the architecture document to include each partial index’s WHERE predicate and explicitly state the maintenance-action scope. Preserve the existing indexed columns and ordering while documenting the predicates needed to verify query alignment.greenfield/src/server/domains/jobs/actionExecutors.ts (1)
72-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
undefinedovervoidin the return union.
Promise<LogMaintenanceExecutionSummary | void>mixes a value type withvoid.voidmeans "ignore the result", so a union with it reads as a contradiction and weakens the narrowing at line 204, where the code testssummary === undefined. Useundefinedto state the same runtime contract precisely. The worker implementation already satisfies it, because its host branch ends with a barereturn;.♻️ Proposed type change
export interface LogMaintenanceExecutionPort { readonly run: ( policyId: LogMaintenancePolicyId, dryRun: boolean, signal?: AbortSignal - ) => Promise<LogMaintenanceExecutionSummary | void>; + ) => Promise<LogMaintenanceExecutionSummary | 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/domains/jobs/actionExecutors.ts` around lines 72 - 78, Update the LogMaintenanceExecutionPort.run return type to use Promise<LogMaintenanceExecutionSummary | undefined> instead of void, preserving the existing undefined result behavior and the summary === undefined narrowing.greenfield/src/contracts/logs.test.ts (1)
247-319: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a case for a mismatch between
result.dryRunandsummary.dryRun.The cases here cover a missing summary, a host summary,
ok: false, a non-zeroerrorcount, and acompletedAtMsthat precedessummary.finishedAtMs. No case pairsdryRun: trueat the result level withdryRun: falseinside the summary, or the reverse.That pair matters downstream.
greenfield/src/server/domains/jobs/logMaintenanceQueue.ts(lines 100-120) trusts a durable summary only when!parsedResult.output.dryRunandparsedResult.output.summary?.dryRun === falseboth hold. IflogMaintenanceJobResultSchemadoes not reject the mismatched pair, a dry-run summary can be persisted under a real-run result, and the queue projection then silently drops it instead of reporting the inconsistency.🧪 Proposed additional cases
expect( v.safeParse(logMaintenanceJobResultSchema, { ...result, completedAtMs: summary.finishedAtMs - 1, }).success ).toBe(false); + expect( + v.safeParse(logMaintenanceJobResultSchema, { + ...result, + summary: { ...summary, dryRun: true }, + }).success + ).toBe(false); + expect( + v.safeParse(logMaintenanceJobResultSchema, { + ...result, + dryRun: true, + summary: { ...summary, dryRun: false }, + }).success + ).toBe(false);🤖 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/logs.test.ts` around lines 247 - 319, Add test cases in the “accepts only coherent successful managed maintenance results” test for mismatched dryRun values between the top-level result and nested summary: result.dryRun true with summary.dryRun false, and result.dryRun false with summary.dryRun true. Assert both are rejected by logMaintenanceJobResultSchema, while preserving the existing valid matching case.greenfield/src/server/database/migrations/jobsSchema.test.ts (1)
2116-2179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the plan helper instead of re-implementing it twice.
Lines 2120-2145 and 2154-2179 repeat the logic of
expectUsesIndexWithoutTemporarySort(lines 285-302) with one addition: a check that the plan contains noSCAN job_runs. The same block calls that helper directly at lines 2146-2153, so one segment now uses two different styles for the same assertion.Extend the helper with the scan check and a two-parameter overload, then call it for all three plans. That removes about 40 lines and keeps future plan assertions consistent.
🤖 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/database/migrations/jobsSchema.test.ts` around lines 2116 - 2179, Extend expectUsesIndexWithoutTemporarySort to also reject SCAN job_runs and support queries requiring two bound parameters, preserving the existing one-parameter usage. Replace the duplicated active and terminal maintenance plan EXPLAIN assertions, plus the existing direct helper call, with helper calls for all three plans using their respective SQL, index names, parameters, and expected predicates.greenfield/src/browser/logs/logQueries.test.ts (1)
56-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the LogsBrowser fallback wiring.
LogsBrowser.tsxpasseslogMaintenanceRealtimeFallbackRefreshIntervalMstouseRealtimeQueryInvalidation, and the hook test covers interval scheduling. This test only checks the constant. Move the assertion to a LogsBrowser wiring test or remove it.🤖 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/logs/logQueries.test.ts` around lines 56 - 71, Remove the logMaintenanceRealtimeFallbackRefreshIntervalMs assertion from the refreshLogMaintenanceQueries test, since it only verifies the constant rather than LogsBrowser wiring. Add the assertion to a LogsBrowser test that verifies the constant is passed to useRealtimeQueryInvalidation, or omit it if no suitable wiring test exists.
🤖 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/migrations/20260804022252_dashboard-foundation/migration.sql`:
- Around line 1113-1114: Replace payload_json with the bounded payload_sha256
key in both job_runs active and terminal indexes, keeping action_key first and
the existing state/time/id ordering and predicates. Ensure
readActionPayloadRunSnapshots filters by payload_sha256 while retaining the
exact payload_json equality check for collision protection, and do not use
enqueue_sha256.
In `@greenfield/src/browser/logs/LogsBrowser.test.tsx`:
- Around line 601-637: Move the onlineManager.setOnline(false) call inside the
existing try block in the test, before renderBrowser, so the finally cleanup
always restores the process-global online state even when rendering throws. Keep
the current assertions and cleanup unchanged.
In `@greenfield/src/browser/logs/LogsView.test.tsx`:
- Around line 310-316: Update the download-click test around the
activatedDownload assertion to capture the clicked HTMLAnchorElement directly in
the click mock and assert against that captured anchor, rather than relying on
mock.contexts[0]; alternatively, replace the floating canary Bun version with a
fixed version that guarantees mock.contexts behavior.
In `@greenfield/src/browser/logs/LogsView.tsx`:
- Around line 267-278: Remove the aria-label override from the Button rendering
the “Clear buffer” action in LogsView, allowing its accessible name to derive
from the visible label while preserving the existing click behavior and disabled
state.
In `@greenfield/src/server/domains/jobs/logMaintenanceQueue.test.ts`:
- Around line 437-440: Guard fixture.runs[firstIndex] against undefined before
spreading it, matching the existing checks near the other indexed updates. Only
assign resultJson after confirming the indexed run exists, and preserve the
test’s intended assertion against an existing run rather than creating a partial
fallback record.
In `@greenfield/src/server/domains/logs/operationAudit.ts`:
- Line 18: Preserve the audit trail link between maintenance requests and their
queued runs: update the queued settlement flow around recordAttempt and settle
to carry the created run id, or propagate the caller requestId in the enqueue
audit metadata. Keep the pre-enqueue attempted event unchanged, and ensure the
queued audit event retains enough identifiers to correlate the request with its
run.
In `@greenfield/src/server/platform/logs/safeLogReader.test.ts`:
- Around line 311-333: Update the test around createSafeLogReader and the
failure returned by reader.tail to assert that error.message and error.stack do
not contain the corrupt marker content or temporary root path, replacing the
ineffective JSON.stringify(error) checks while preserving the existing
unavailable-reason assertion.
In `@greenfield/src/worker/logs/managedLogRotation.test.ts`:
- Around line 620-645: Await both managed log rotation runs using the existing
expectRejection helper before asserting file contents or lock-path state. Update
the two createManagedLogRotationEngine({ manifest }).run() rejection assertions
in the test so each run fully completes, including cleanup, while preserving the
expected "Managed log maintenance failed" error.
---
Outside diff comments:
In `@greenfield/src/worker/logs/managedLogRotation.ts`:
- Around line 549-565: The missing-file branch in the rotation flow leaves the
pending epoch in rotating; update the file === undefined handling in the
function containing openFileTarget to settle the corresponding epoch before
returning the missing result. Reuse the existing publishCopyTruncateEpochState
or established epoch-removal mechanism, preserving the current missing response
and ensuring subsequent probes see no unresolved rotating entry.
---
Nitpick comments:
In `@greenfield/docs/architecture/greenfield-rewrite/data-and-security.md`:
- Around line 204-205: Update the active and terminal maintenance status index
entries in the architecture document to include each partial index’s WHERE
predicate and explicitly state the maintenance-action scope. Preserve the
existing indexed columns and ordering while documenting the predicates needed to
verify query alignment.
In `@greenfield/src/browser/logs/logQueries.test.ts`:
- Around line 56-71: Remove the logMaintenanceRealtimeFallbackRefreshIntervalMs
assertion from the refreshLogMaintenanceQueries test, since it only verifies the
constant rather than LogsBrowser wiring. Add the assertion to a LogsBrowser test
that verifies the constant is passed to useRealtimeQueryInvalidation, or omit it
if no suitable wiring test exists.
In `@greenfield/src/contracts/logs.test.ts`:
- Around line 247-319: Add test cases in the “accepts only coherent successful
managed maintenance results” test for mismatched dryRun values between the
top-level result and nested summary: result.dryRun true with summary.dryRun
false, and result.dryRun false with summary.dryRun true. Assert both are
rejected by logMaintenanceJobResultSchema, while preserving the existing valid
matching case.
In `@greenfield/src/server/database/migrations/jobsSchema.test.ts`:
- Around line 2116-2179: Extend expectUsesIndexWithoutTemporarySort to also
reject SCAN job_runs and support queries requiring two bound parameters,
preserving the existing one-parameter usage. Replace the duplicated active and
terminal maintenance plan EXPLAIN assertions, plus the existing direct helper
call, with helper calls for all three plans using their respective SQL, index
names, parameters, and expected predicates.
In `@greenfield/src/server/database/schema/jobRuns.ts`:
- Around line 209-222: The new indexes in the schema should not blindly
duplicate full payloadJson values for every matching run. Before finalizing
job_runs_action_payload_active_idx and job_runs_action_payload_terminal_idx,
measure their size and write impact on a representative database, then either
index a payload digest while retaining payload_json equality filtering or narrow
the terminal predicate to only action keys requiring snapshot reads; preserve
the query-compatible key ordering and active/terminal behavior.
In `@greenfield/src/server/domains/jobs/actionExecutors.ts`:
- Around line 72-78: Update the LogMaintenanceExecutionPort.run return type to
use Promise<LogMaintenanceExecutionSummary | undefined> instead of void,
preserving the existing undefined result behavior and the summary === undefined
narrowing.
In `@greenfield/src/server/domains/jobs/coordinator.test.ts`:
- Around line 1398-1415: Update the schedule construction around
intervalSchedule to derive every cadence field from definition.defaultSchedule,
including schedule kind, interval, cron expression, time of day, and time zone,
rather than overriding only intervalMs. Alternatively, explicitly assert that
definition.defaultSchedule.kind is "interval" before constructing the fixture,
so non-interval cadence changes fail at the mapping boundary.
In `@greenfield/src/server/domains/jobs/logMaintenanceQueue.ts`:
- Around line 190-234: Add a short explanatory comment at the docker-managed
merge site in runStatuses, immediately around the preferredActiveRun call,
documenting that managedDryRunSnapshot.lastRun is intentionally not used because
dry-run terminal history is excluded from status observations. Preserve the
existing behavior and logic unchanged.
In `@greenfield/src/server/domains/jobs/repository.ts`:
- Around line 791-845: In greenfield/src/server/domains/jobs/repository.ts lines
791-845, define shared activeStateFilter and terminalStateFilter SQL fragments
rendered from the typed active and terminal state lists, replace both inline IN
clauses, and document that literal SQL is required for SQLite partial-index
matching. In lines 1321-1333 and 1486-1500, replace each repeated active-state
SQL condition in enqueueManualRun and enqueueNextDueSchedule with the shared
activeStateFilter; retain the existing query behavior.
In `@greenfield/src/server/domains/logs/service.test.ts`:
- Around line 118-145: Update the test setup for “projects an active run without
hiding the latest terminal run” to construct the LogsServiceDependencies
directly instead of spreading fixture.service, since every required dependency
is already overridden. Replace the runStatuses `as never` cast with a properly
typed JobRunSummary fixture, reusing an existing shared test helper if
available, so the active and terminal run entries satisfy their declared
contracts.
In `@greenfield/src/server/domains/logs/service.ts`:
- Around line 47-54: Define and export a shared LogMaintenanceRunStatus type in
logs.ts using the existing activeRun, lastRun, and policyId fields, then replace
the duplicated inline return shapes in the runStatuses declaration and runStatus
method of LogMaintenanceJobQueue and the service runStatuses declaration with
this named type.
In `@greenfield/src/server/platform/logs/logRotationEpochProbe.test.ts`:
- Around line 121-135: Update the test named “rejects noncanonical and
over-budget source inventories” to add a separate projection fixture with
entries that are out of order or duplicated, then assert that probing it rejects
with “Log rotation epoch is unavailable.” Keep the existing 65-entry fixture to
cover the maximum-entry limit, and ensure the new case exercises the
entriesAreCanonical validation.
In `@greenfield/src/server/platform/logs/redaction.ts`:
- Around line 125-139: Update the unquoted branch of sensitiveValueEnd to use
the existing isSafeValueDelimiter helper instead of duplicating its
character-class check, while preserving the current loop boundary and
fail-closed behavior.
- Around line 44-106: Extract the shared quote, escape, and bracket-stack
traversal from structuredValueEnd, malformedValueSuffixEnd, and quotedValueEnd
into one scanner that accepts a stop predicate. Update each function to use this
scanner while preserving its existing termination behavior: balanced structured
values stop when the stack empties, malformed suffixes stop at safe delimiters
at depth zero, and quoted values retain their current escape handling.
In `@greenfield/src/server/platform/logs/safeLogReader.ts`:
- Around line 228-245: Move await predecessor inside the try block in
createSourceReadCoordinator so any predecessor rejection still reaches the
finally cleanup. Preserve completion.resolve() and the pending-map identity
guard, ensuring the queued source operation cannot leave a permanently unsettled
promise.
🪄 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: 44737588-8233-4a89-bab5-f2cda493b74e
⛔ Files ignored due to path filters (4)
greenfield/docs/generated/procedures.mdis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/logs.maintenanceStatus.output.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/logs.requestMaintenance.input.schema.jsonis excluded by!**/generated/**and included by**/*greenfield/docs/generated/schemas/logs.requestMaintenance.output.schema.jsonis excluded by!**/generated/**and included by**/*
📒 Files selected for processing (55)
greenfield/docs/architecture/greenfield-rewrite/data-and-security.mdgreenfield/docs/architecture/greenfield-rewrite/progress.mdgreenfield/migrations/20260804022252_dashboard-foundation/migration.sqlgreenfield/migrations/20260804022252_dashboard-foundation/snapshot.jsongreenfield/scripts/documentation/jsonSchema.tsgreenfield/src/app/dashboardLogs.tsgreenfield/src/app/dashboardServer.test.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/browser/logs/LogMaintenancePanel.tsxgreenfield/src/browser/logs/LogsBrowser.test.tsxgreenfield/src/browser/logs/LogsBrowser.tsxgreenfield/src/browser/logs/LogsRoute.test.tsxgreenfield/src/browser/logs/LogsView.test.tsxgreenfield/src/browser/logs/LogsView.tsxgreenfield/src/browser/logs/logClient.test.tsgreenfield/src/browser/logs/logQueries.test.tsgreenfield/src/browser/logs/logQueries.tsgreenfield/src/browser/logs/stories/LogsView.stories.tsxgreenfield/src/browser/routes/logs.lazy.test.tsxgreenfield/src/contracts/logs.test.tsgreenfield/src/contracts/logs.tsgreenfield/src/server/database/migrations/jobsSchema.test.tsgreenfield/src/server/database/schema/jobRuns.tsgreenfield/src/server/domains/jobs/actionExecutors.test.tsgreenfield/src/server/domains/jobs/actionExecutors.tsgreenfield/src/server/domains/jobs/coordinator.test.tsgreenfield/src/server/domains/jobs/coordinator.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.tsgreenfield/src/server/domains/jobs/repository.test.tsgreenfield/src/server/domains/jobs/repository.tsgreenfield/src/server/domains/logs/operationAudit.test.tsgreenfield/src/server/domains/logs/operationAudit.tsgreenfield/src/server/domains/logs/service.test.tsgreenfield/src/server/domains/logs/service.tsgreenfield/src/server/domains/security/audit.tsgreenfield/src/server/domains/security/securityAuditLifecycle.test.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.test.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.tsgreenfield/src/server/platform/logs/redaction.test.tsgreenfield/src/server/platform/logs/redaction.tsgreenfield/src/server/platform/logs/safeLogReader.test.tsgreenfield/src/server/platform/logs/safeLogReader.tsgreenfield/src/server/platform/observability/structuredLogger.test.tsgreenfield/src/server/platform/observability/structuredLogger.tsgreenfield/src/shared/databaseMigrationManifest.tsgreenfield/src/shared/logRotationEpochProjection.tsgreenfield/src/test/parity/fixtures/frontend-routes.jsongreenfield/src/test/parity/fixtures/legacy-endpoints.jsongreenfield/src/test/parity/parityInventory.test.tsgreenfield/src/worker/logs/logMaintenanceExecutor.test.tsgreenfield/src/worker/logs/logMaintenanceExecutor.tsgreenfield/src/worker/logs/managedLogManifest.tsgreenfield/src/worker/logs/managedLogRotation.test.tsgreenfield/src/worker/logs/managedLogRotation.ts
💤 Files with no reviewable changes (1)
- greenfield/src/server/domains/security/audit.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: dashboard-checks
- GitHub Check: storybook
- GitHub Check: Analyze JavaScript and TypeScript
🧰 Additional context used
🧠 Learnings (2)
📚 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/shared/databaseMigrationManifest.tsgreenfield/src/server/database/schema/jobRuns.tsgreenfield/src/server/platform/observability/structuredLogger.tsgreenfield/src/server/platform/observability/structuredLogger.test.tsgreenfield/src/server/domains/jobs/coordinator.test.tsgreenfield/src/test/parity/parityInventory.test.tsgreenfield/src/app/dashboardServer.tsgreenfield/src/server/domains/jobs/coordinator.tsgreenfield/src/browser/logs/logClient.test.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.test.tsgreenfield/src/shared/logRotationEpochProjection.tsgreenfield/src/server/database/migrations/jobsSchema.test.tsgreenfield/src/server/domains/security/securityAuditLifecycle.test.tsgreenfield/scripts/documentation/jsonSchema.tsgreenfield/src/app/dashboardServer.test.tsgreenfield/src/browser/logs/logQueries.test.tsgreenfield/src/server/domains/logs/operationAudit.tsgreenfield/src/server/domains/jobs/actionExecutors.test.tsgreenfield/src/server/platform/logs/redaction.test.tsgreenfield/src/worker/logs/managedLogManifest.tsgreenfield/src/server/domains/jobs/repository.test.tsgreenfield/src/browser/logs/logQueries.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.tsgreenfield/src/app/dashboardLogs.tsgreenfield/src/server/domains/jobs/repository.tsgreenfield/src/worker/logs/logMaintenanceExecutor.test.tsgreenfield/src/contracts/logs.test.tsgreenfield/src/server/domains/logs/service.test.tsgreenfield/src/server/platform/logs/redaction.tsgreenfield/src/worker/logs/logMaintenanceExecutor.tsgreenfield/src/server/domains/logs/operationAudit.test.tsgreenfield/src/server/domains/jobs/actionExecutors.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/worker/logs/managedLogRotation.test.tsgreenfield/src/server/domains/logs/service.tsgreenfield/src/server/platform/logs/safeLogReader.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.tsgreenfield/src/worker/logs/managedLogRotation.tsgreenfield/src/server/platform/logs/safeLogReader.test.tsgreenfield/src/contracts/logs.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/server/platform/observability/structuredLogger.test.tsgreenfield/src/server/domains/jobs/coordinator.test.tsgreenfield/src/test/parity/parityInventory.test.tsgreenfield/src/browser/logs/logClient.test.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.test.tsgreenfield/src/server/database/migrations/jobsSchema.test.tsgreenfield/src/server/domains/security/securityAuditLifecycle.test.tsgreenfield/src/app/dashboardServer.test.tsgreenfield/src/browser/logs/logQueries.test.tsgreenfield/src/server/domains/jobs/actionExecutors.test.tsgreenfield/src/server/platform/logs/redaction.test.tsgreenfield/src/server/domains/jobs/repository.test.tsgreenfield/src/worker/logs/logMaintenanceExecutor.test.tsgreenfield/src/contracts/logs.test.tsgreenfield/src/server/domains/logs/service.test.tsgreenfield/src/server/domains/logs/operationAudit.test.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/worker/logs/managedLogRotation.test.tsgreenfield/src/server/platform/logs/safeLogReader.test.ts
🪛 ast-grep (0.45.0)
greenfield/src/server/platform/logs/redaction.ts
[warning] 12-12: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(([?&]${secretName}=)[^&#\\s]*, "giu")
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 6-9: Do not use variable for regular expressions
Context: new RegExp(
String.raw(?:["']?)(${secretName})(?:["']?)\s*[:=]\s*,
"giu"
)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🪛 OpenGrep (1.26.0)
greenfield/src/server/platform/logs/redaction.ts
[ERROR] 146-146: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
mira-2026
added
the
area: chat
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
greenfield/src/browser/chat/chatRuntimeStore.ts (1)
1372-1400: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCompute the segment merge only for truncated projections.
reconciledSegmentsuses the merge result only whenprojection.projectionTruncatedis true. For a complete projection the code takesexternalSegments(projection)and discardssegmentsandauthoritativeParts.
mergeExternalSegmentsandmergeTruncatedExternalPartsstill run on every install.mergeExternalSegmentscallsJSON.stringifytwice per matching segment. Complete projections are the common polling case, so this work is always discarded.Move both calls behind the truncation check.
♻️ Proposed refactor to skip the unused merge
- const segments = mergeExternalSegments( - externalSegments(existing), - externalSegments(projection), - newlyReplacedStreams - ); - const authoritativeParts = mergeTruncatedExternalParts( - existing.message.parts, - projection.message.parts, - newlyReplacedStreams - ); - const reconciledSegments = projection.projectionTruncated - ? reconcileExternalSegmentsWithAuthoritativeParts( - segments, - authoritativeParts - ) - : externalSegments(projection); + const reconciledSegments = projection.projectionTruncated + ? reconcileExternalSegmentsWithAuthoritativeParts( + mergeExternalSegments( + externalSegments(existing), + externalSegments(projection), + newlyReplacedStreams + ), + mergeTruncatedExternalParts( + existing.message.parts, + projection.message.parts, + newlyReplacedStreams + ) + ) + : externalSegments(projection);🤖 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/chat/chatRuntimeStore.ts` around lines 1372 - 1400, Update the projection reconciliation block to call mergeExternalSegments and mergeTruncatedExternalParts only when projection.projectionTruncated is true. For complete projections, use externalSegments(projection) directly without computing segments or authoritativeParts, while preserving the existing truncated reconciliation behavior.greenfield/src/browser/chat/chatContractAdapter.ts (1)
731-749: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace only the matched text part instead of the whole segment parts array.
Branch A locates the last text part with
findLast, then callsreplaceSegmentPartswith a single-element array. Every other part of that segment is dropped.Today assistant text segments are built by
upsertSinglePartand hold exactly one part, so no data is lost. ThefindLastcall implies the code expects several parts. The two assumptions conflict, and any future change that appends parts to an assistant segment loses them silently.♻️ Proposed refactor to preserve sibling parts
if (previous !== undefined && previousPart?.kind === "text") { - replaceSegmentParts(lastAssistantIndex, [ - { - ...previousPart, - text: - previousPart.text + - run.text.slice(renderedAssistantText.length), - }, - ]); + replaceSegmentParts( + lastAssistantIndex, + previous.message.parts.map((part) => + part === previousPart + ? { + ...previousPart, + text: + previousPart.text + + run.text.slice(renderedAssistantText.length), + } + : part + ) + ); }🤖 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/chat/chatContractAdapter.ts` around lines 731 - 749, Update the Branch A logic around lastAssistantIndex and previousPart so it replaces only the matched text part within the existing segment parts, preserving all sibling parts and their ordering. Build the replacement from previous.message.parts with the targeted text part updated, then pass the complete parts array to replaceSegmentParts instead of a single-element array.greenfield/src/browser/chat/chatViewProjection.ts (1)
348-352: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
canonicalAssistantProviderRunIdsclause inunanchoredis unreachable.The
ephemeralfilter at lines 319-323 already removes every assistant message whoseproviderRunIdappears incanonicalAssistantProviderRunIds. Every member ofephemeraltherefore satisfies!canonicalAssistantProviderRunIds.has(message.providerRunId ?? "")at line 410. The whole parenthesized condition always evaluates totrue, andgroupedExternalIds(lines 348-352) exists only to feed that dead condition.Remove both, or state the intended second guard explicitly if one is missing.
♻️ Proposed simplification
- const groupedExternalIds = new Set( - [...externalGroups.values()].flatMap((messages) => - messages.map((message) => message.id) - ) - ); const beforeCanonical = new Map<string, ChatDisplayMessage[]>(); @@ - const unanchored = ephemeral.filter( - (message) => - !anchoredExternalIds.has(message.id) && - (!groupedExternalIds.has(message.id) || - !canonicalAssistantProviderRunIds.has(message.providerRunId ?? "")) - ); + const unanchored = ephemeral.filter( + (message) => !anchoredExternalIds.has(message.id) + );Also applies to: 406-411
🤖 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/chat/chatViewProjection.ts` around lines 348 - 352, Remove the unused groupedExternalIds Set construction and simplify the unanchored condition near canonicalAssistantProviderRunIds so it no longer includes the unreachable guard. Preserve the remaining filtering behavior and update any references to groupedExternalIds accordingly.greenfield/src/browser/chat/ChatTranscript.tsx (1)
218-248: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing
timestampMsinstead of asserting it, and reduce the unconditional refresh.Two points on these effects:
- Line 233 uses the non-null assertion
message.timestampMs!. The outer ternary guarantees the value, but TypeScript cannot narrow it inside the nested closure. A localconst timestampMs = message.timestampMs;guard removes the assertion.- The effect at lines 218-221 runs whenever the
messagesarray identity changes.ChatBrowser.tsxline 493 builds that array inline on every render, so this schedules an extra state update and render pass per parent render. The expiry effect at lines 222-248 already refreshesnowMsat the exact boundary. Consider dropping the 0 ms refresh, or gating it on whether any control part carries anactivity.♻️ Proposed narrowing for line 224-239
const nextExpiry = messages - .flatMap((message) => - message.timestampMs === undefined - ? [] - : message.parts.flatMap((part) => { - if (part.kind !== "control" || part.activity === undefined) { - return []; - } - return [ - message.timestampMs! + - (part.activity === "running" - ? activeCompactionMaximumAgeMs - : completedCompactionMaximumAgeMs), - ]; - }) - ) + .flatMap((message) => { + const timestampMs = message.timestampMs; + if (timestampMs === undefined) return []; + return message.parts.flatMap((part) => + part.kind !== "control" || part.activity === undefined + ? [] + : [ + timestampMs + + (part.activity === "running" + ? activeCompactionMaximumAgeMs + : completedCompactionMaximumAgeMs), + ] + ); + })🤖 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/chat/ChatTranscript.tsx` around lines 218 - 248, Update the expiry calculation in the messages effect by assigning message.timestampMs to a local variable and guarding that value before using it, removing the non-null assertion. Also reduce the unconditional nowMs refresh effect tied to messages changes—remove it or gate it on messages containing an activity-bearing control part—while preserving the expiry effect’s boundary refresh behavior.
🤖 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/browser/chat/chatContractAdapter.ts`:
- Around line 750-779: In the Branch C insertion logic around the aggregate
assistant createSegment call, assign the aggregate segment the providerSequence
of the first assistant text segment being replaced rather than
Number.MAX_SAFE_INTEGER - 4. Preserve the existing splice position and segment
contents so chatRuntimeMessages sorting reflects the intended rendered order.
In `@greenfield/src/browser/chat/ChatMessageBubble.tsx`:
- Around line 500-514: Update the running-state label in the activity rendering
branch of ChatMessageBubble so it appends an ellipsis only when part.text does
not already end with one. Preserve the existing LoadingDots behavior and label
content for text without a trailing ellipsis.
In `@greenfield/src/browser/chat/chatToolPresentation.ts`:
- Around line 22-31: Update toolDisplayName to remove Unicode control and format
characters, including \p{Cc} and \p{Cf}, from the provider-supplied name, then
enforce the same maximum length used by the collapsed summary before returning
the display label. Preserve the existing prefix stripping, tool-name
normalization, separator cleanup, and “Tool” fallback behavior.
In `@greenfield/src/browser/chat/chatViewProjection.ts`:
- Around line 325-338: The comparator in the chat message sorting flow must use
a transitive ordering for provider-run messages. Update the surrounding sort
logic to derive a single order key for each provider run, using the run’s
earliest relevant timestamp, then compare messages by that key, sequence, and
id; retain consistent handling for ungrouped messages and avoid switching
between timestamp and sequence based on the pair being compared.
In `@greenfield/src/browser/ui/Virtualizer.tsx`:
- Around line 183-197: Update remeasureMountedItems to avoid reading
offsetWidth/offsetHeight from non-HTMLElement elements: either guard each cached
element with an HTMLElement check before accessing sizeProperty, or constrain
TItemElement to HTMLElement. Ensure resizeItem is called only with valid
HTMLElement dimensions while preserving the existing connected-element behavior.
---
Nitpick comments:
In `@greenfield/src/browser/chat/chatContractAdapter.ts`:
- Around line 731-749: Update the Branch A logic around lastAssistantIndex and
previousPart so it replaces only the matched text part within the existing
segment parts, preserving all sibling parts and their ordering. Build the
replacement from previous.message.parts with the targeted text part updated,
then pass the complete parts array to replaceSegmentParts instead of a
single-element array.
In `@greenfield/src/browser/chat/chatRuntimeStore.ts`:
- Around line 1372-1400: Update the projection reconciliation block to call
mergeExternalSegments and mergeTruncatedExternalParts only when
projection.projectionTruncated is true. For complete projections, use
externalSegments(projection) directly without computing segments or
authoritativeParts, while preserving the existing truncated reconciliation
behavior.
In `@greenfield/src/browser/chat/ChatTranscript.tsx`:
- Around line 218-248: Update the expiry calculation in the messages effect by
assigning message.timestampMs to a local variable and guarding that value before
using it, removing the non-null assertion. Also reduce the unconditional nowMs
refresh effect tied to messages changes—remove it or gate it on messages
containing an activity-bearing control part—while preserving the expiry effect’s
boundary refresh behavior.
In `@greenfield/src/browser/chat/chatViewProjection.ts`:
- Around line 348-352: Remove the unused groupedExternalIds Set construction and
simplify the unanchored condition near canonicalAssistantProviderRunIds so it no
longer includes the unreachable guard. Preserve the remaining filtering behavior
and update any references to groupedExternalIds accordingly.
🪄 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: 2b4d4555-cbc3-4948-b397-8e018d86ef4a
⛔ Files ignored due to path filters (1)
greenfield/docs/generated/schemas/chat.runtime.output.schema.jsonis excluded by!**/generated/**and included by**/*
📒 Files selected for processing (67)
greenfield/.bun-browser-test-timings.jsongreenfield/.bun-test-timings.jsongreenfield/docs/architecture/greenfield-rewrite/data-and-security.mdgreenfield/migrations/20260804022252_dashboard-foundation/migration.sqlgreenfield/migrations/20260804022252_dashboard-foundation/snapshot.jsongreenfield/scripts/delivery/installProductionSystemdUnits.test.tsgreenfield/src/app/worker.test.tsgreenfield/src/browser/application.test.tsxgreenfield/src/browser/chat/ChatBrowser.tsxgreenfield/src/browser/chat/ChatLiveProjection.test.tsxgreenfield/src/browser/chat/ChatMessageBubble.test.tsxgreenfield/src/browser/chat/ChatMessageBubble.tsxgreenfield/src/browser/chat/ChatTranscript.test.tsxgreenfield/src/browser/chat/ChatTranscript.tsxgreenfield/src/browser/chat/chatContractAdapter.tsgreenfield/src/browser/chat/chatRuntimeStore.test.tsgreenfield/src/browser/chat/chatRuntimeStore.tsgreenfield/src/browser/chat/chatToolPresentation.tsgreenfield/src/browser/chat/chatTranscriptProjection.tsgreenfield/src/browser/chat/chatTypes.tsgreenfield/src/browser/chat/chatViewProjection.test.tsgreenfield/src/browser/chat/chatViewProjection.tsgreenfield/src/browser/chat/useChatRuntimeProjection.test.tsxgreenfield/src/browser/logs/LogsBrowser.test.tsxgreenfield/src/browser/logs/LogsView.test.tsxgreenfield/src/browser/logs/LogsView.tsxgreenfield/src/browser/logs/logQueries.test.tsgreenfield/src/browser/notifications/AuthenticatedNotificationCenter.tsxgreenfield/src/browser/notifications/NotificationCenter.test.tsxgreenfield/src/browser/ui/Virtualizer.test.tsxgreenfield/src/browser/ui/Virtualizer.tsxgreenfield/src/contracts/chatModel.tsgreenfield/src/contracts/logs.test.tsgreenfield/src/contracts/logs.tsgreenfield/src/server/database/migrations/jobsSchema.test.tsgreenfield/src/server/database/schema/jobRuns.tsgreenfield/src/server/domains/chat/provider.tsgreenfield/src/server/domains/chat/service.test.tsgreenfield/src/server/domains/chat/service.tsgreenfield/src/server/domains/jobs/actionExecutors.test.tsgreenfield/src/server/domains/jobs/actionExecutors.tsgreenfield/src/server/domains/jobs/actionRegistry.tsgreenfield/src/server/domains/jobs/coordinator.test.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.tsgreenfield/src/server/domains/jobs/repository.test.tsgreenfield/src/server/domains/jobs/repository.tsgreenfield/src/server/domains/jobs/workerRuntime.test.tsgreenfield/src/server/domains/jobs/workerSystem.test.tsgreenfield/src/server/domains/logs/operationAudit.test.tsgreenfield/src/server/domains/logs/operationAudit.tsgreenfield/src/server/domains/logs/service.test.tsgreenfield/src/server/domains/logs/service.tsgreenfield/src/server/domains/security/securityAuditLifecycle.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.tsgreenfield/src/server/platform/gateway/persistentGatewayProtocol.test.tsgreenfield/src/server/platform/gateway/persistentGatewayProtocol.tsgreenfield/src/server/platform/logs/logRotationEpochProbe.test.tsgreenfield/src/server/platform/logs/redaction.tsgreenfield/src/server/platform/logs/safeLogReader.test.tsgreenfield/src/server/platform/logs/safeLogReader.tsgreenfield/src/shared/databaseMigrationManifest.tsgreenfield/src/shared/logMaintenanceUnits.tsgreenfield/src/worker/logs/logMaintenanceExecutor.tsgreenfield/src/worker/logs/managedLogRotation.test.tsgreenfield/src/worker/logs/managedLogRotation.ts
💤 Files with no reviewable changes (1)
- greenfield/src/browser/logs/LogsView.tsx
🚧 Files skipped from review as they are similar to previous changes (24)
- greenfield/src/shared/databaseMigrationManifest.ts
- greenfield/src/server/domains/logs/operationAudit.test.ts
- greenfield/src/server/domains/security/securityAuditLifecycle.test.ts
- greenfield/migrations/20260804022252_dashboard-foundation/migration.sql
- greenfield/docs/architecture/greenfield-rewrite/data-and-security.md
- greenfield/src/contracts/logs.test.ts
- greenfield/src/server/domains/jobs/repository.test.ts
- greenfield/src/server/platform/logs/logRotationEpochProbe.test.ts
- greenfield/src/server/domains/jobs/coordinator.test.ts
- greenfield/src/browser/logs/logQueries.test.ts
- greenfield/src/worker/logs/logMaintenanceExecutor.ts
- greenfield/src/server/domains/jobs/actionExecutors.ts
- greenfield/src/worker/logs/managedLogRotation.test.ts
- greenfield/src/server/platform/logs/redaction.ts
- greenfield/src/server/domains/logs/service.test.ts
- greenfield/src/server/domains/jobs/logMaintenanceQueue.ts
- greenfield/src/contracts/logs.ts
- greenfield/src/server/domains/jobs/actionExecutors.test.ts
- greenfield/src/server/platform/logs/safeLogReader.ts
- greenfield/src/server/platform/logs/safeLogReader.test.ts
- greenfield/src/server/domains/logs/service.ts
- greenfield/src/browser/logs/LogsBrowser.test.tsx
- greenfield/src/server/domains/jobs/repository.ts
- greenfield/src/worker/logs/managedLogRotation.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: dashboard-checks
- GitHub Check: Analyze JavaScript and TypeScript
🧰 Additional context used
🧠 Learnings (2)
📚 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/server/domains/jobs/workerRuntime.test.tsgreenfield/src/server/platform/gateway/persistentGatewayProtocol.test.tsgreenfield/src/app/worker.test.tsgreenfield/src/browser/chat/chatViewProjection.test.tsgreenfield/scripts/delivery/installProductionSystemdUnits.test.tsgreenfield/src/server/domains/jobs/workerSystem.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.tsgreenfield/src/server/domains/chat/service.test.tsgreenfield/src/browser/chat/chatRuntimeStore.test.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/server/database/migrations/jobsSchema.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/server/domains/jobs/workerRuntime.test.tsgreenfield/src/server/platform/gateway/persistentGatewayProtocol.test.tsgreenfield/src/app/worker.test.tsgreenfield/src/shared/logMaintenanceUnits.tsgreenfield/src/browser/chat/chatViewProjection.test.tsgreenfield/scripts/delivery/installProductionSystemdUnits.test.tsgreenfield/src/server/domains/jobs/workerSystem.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.tsgreenfield/src/server/domains/chat/service.test.tsgreenfield/src/server/platform/gateway/persistentGatewayChatProvider.tsgreenfield/src/server/domains/chat/provider.tsgreenfield/src/contracts/chatModel.tsgreenfield/src/server/domains/jobs/actionRegistry.tsgreenfield/src/browser/chat/chatTypes.tsgreenfield/src/server/database/schema/jobRuns.tsgreenfield/src/browser/chat/chatRuntimeStore.test.tsgreenfield/src/server/domains/logs/operationAudit.tsgreenfield/src/browser/chat/chatToolPresentation.tsgreenfield/src/server/platform/gateway/persistentGatewayProtocol.tsgreenfield/src/server/domains/jobs/logMaintenanceQueue.test.tsgreenfield/src/server/domains/chat/service.tsgreenfield/src/browser/chat/chatTranscriptProjection.tsgreenfield/src/browser/chat/chatContractAdapter.tsgreenfield/src/server/database/migrations/jobsSchema.test.tsgreenfield/src/browser/chat/chatRuntimeStore.tsgreenfield/src/browser/chat/chatViewProjection.ts
🔇 Additional comments (65)
greenfield/src/shared/logMaintenanceUnits.ts (1)
10-14: LGTM!greenfield/src/server/database/migrations/jobsSchema.test.ts (1)
1-1: LGTM!Also applies to: 286-305, 2076-2076, 2087-2087, 2098-2118, 2127-2163, 2180-2180
greenfield/src/server/database/schema/jobRuns.ts (1)
16-19: LGTM!Also applies to: 213-220
greenfield/src/server/domains/chat/provider.ts (1)
108-115: LGTM!greenfield/src/server/domains/jobs/actionRegistry.ts (1)
25-33: LGTM!greenfield/src/server/domains/logs/operationAudit.ts (1)
17-32: LGTM!Also applies to: 54-54, 66-79
greenfield/src/server/domains/jobs/logMaintenanceQueue.test.ts (1)
437-442: LGTM!greenfield/src/server/domains/jobs/workerRuntime.test.ts (1)
32-32: LGTM!greenfield/src/server/domains/jobs/workerSystem.test.ts (1)
33-33: LGTM!greenfield/src/server/platform/gateway/persistentGatewayProtocol.ts (1)
385-385: LGTM!Also applies to: 409-419, 757-757, 1272-1298
greenfield/src/server/platform/gateway/persistentGatewayProtocol.test.ts (1)
789-818: LGTM!greenfield/src/server/platform/gateway/persistentGatewayChatProvider.test.ts (1)
1826-1893: LGTM!greenfield/src/browser/notifications/NotificationCenter.test.tsx (1)
282-288: LGTM!Also applies to: 315-321, 337-389
greenfield/src/server/domains/chat/service.test.ts (1)
2450-2517: LGTM!greenfield/src/server/platform/gateway/persistentGatewayChatProvider.ts (1)
889-923: LGTM!Also applies to: 973-975
greenfield/src/browser/notifications/AuthenticatedNotificationCenter.tsx (1)
1-1: LGTM!Also applies to: 20-20, 31-41
greenfield/src/browser/ui/Virtualizer.test.tsx (1)
71-71: LGTM!Also applies to: 81-81, 109-114, 135-140, 832-919
greenfield/migrations/20260804022252_dashboard-foundation/snapshot.json (1)
6033-6058: LGTM!Also applies to: 6059-6084
greenfield/src/server/domains/chat/service.ts (1)
249-257: LGTM!Also applies to: 740-781, 1328-1330
greenfield/src/browser/logs/LogsView.test.tsx (12)
3-5: LGTM!
17-26: LGTM!
159-208: LGTM!
284-412: LGTM!
569-580: LGTM!
600-683: LGTM!
685-793: LGTM!
795-836: LGTM!
838-973: LGTM!
975-1009: LGTM!
1011-1095: LGTM!
1097-1169: LGTM!greenfield/src/browser/application.test.tsx (1)
5-12: LGTM!Also applies to: 257-521
greenfield/src/browser/chat/ChatBrowser.tsx (1)
508-516: LGTM!greenfield/src/browser/chat/ChatLiveProjection.test.tsx (2)
18-78: LGTM!
189-234: LGTM!Also applies to: 256-256
greenfield/src/browser/chat/chatContractAdapter.ts (4)
536-581: LGTM!
582-672: LGTM!
673-717: LGTM!
9-9: LGTM!Also applies to: 781-814, 852-852
greenfield/src/browser/chat/chatRuntimeStore.test.ts (1)
651-654: LGTM!Also applies to: 1442-1554, 1556-1591, 1593-1631
greenfield/src/browser/chat/chatRuntimeStore.ts (7)
167-175: LGTM!Also applies to: 184-191
617-694: LGTM!
696-773: LGTM!
1180-1226: LGTM!
1404-1417: LGTM!
1459-1459: LGTM!Also applies to: 1477-1477, 1487-1532, 1552-1559
316-320: 🎯 Functional CorrectnessKeep one compaction activity identity per provider run.
updateExternalCompactionPartupdatescompaction:${event.providerRunId}across lifecycle phases. Separate OpenClaw operations use separate compaction run identities.> Likely an incorrect or invalid review comment.greenfield/src/browser/chat/useChatRuntimeProjection.test.tsx (1)
280-280: LGTM!Also applies to: 325-343
greenfield/src/contracts/chatModel.ts (1)
596-598: LGTM!greenfield/src/app/worker.test.ts (1)
122-122: LGTM!greenfield/.bun-browser-test-timings.json (1)
2-161: LGTM!greenfield/.bun-test-timings.json (1)
2-370: LGTM!greenfield/src/browser/chat/ChatMessageBubble.test.tsx (1)
18-62: LGTM!Also applies to: 219-227, 244-280, 282-320
greenfield/src/browser/chat/ChatMessageBubble.tsx (1)
115-122: LGTM!greenfield/src/browser/chat/ChatTranscript.test.tsx (1)
1-8: LGTM!Also applies to: 58-79, 81-128, 130-165, 167-212, 214-274
greenfield/src/browser/chat/ChatTranscript.tsx (1)
12-16: LGTM!Also applies to: 64-67, 158-171
greenfield/src/browser/chat/chatToolPresentation.ts (1)
5-15: LGTM!Also applies to: 38-66, 73-81
greenfield/src/browser/chat/chatTranscriptProjection.ts (1)
1-16: LGTM!Also applies to: 18-36, 38-111, 113-132
greenfield/src/browser/chat/chatTypes.ts (2)
94-94: LGTM!
121-122: 🗄️ Data Integrity & IntegrationNo change required.
chatRuntimeStore.tsmapsprecedingUserTexttoprecedingUserTextAnchor.> Likely an incorrect or invalid review comment.greenfield/src/browser/chat/chatViewProjection.test.ts (1)
940-982: LGTM!Also applies to: 984-1047
greenfield/src/browser/chat/chatViewProjection.ts (2)
277-281: LGTM!Also applies to: 297-324
339-347: LGTM!Also applies to: 353-405
greenfield/src/browser/ui/Virtualizer.tsx (1)
322-322: LGTM!Also applies to: 470-470
greenfield/scripts/delivery/installProductionSystemdUnits.test.ts (1)
190-258: LGTM!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
greenfield/src/browser/ui/Virtualizer.test.tsx (1)
993-1004: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the SVG skip assertion observable.
Lines 993-1004 only verify unchanged render count and total size. An implementation can still read the unsupported SVG height and discard
undefined. This test would pass although it did not skip the SVG row.Replace the deleted property with a getter that returns
undefinedand records reads. After remeasurement, assert that the getter was not called.Proposed test adjustment
- Reflect.deleteProperty(SVGElement.prototype, "offsetHeight"); + const unavailableOffsetHeight = jest.fn(() => undefined); + Object.defineProperty(SVGElement.prototype, "offsetHeight", { + configurable: true, + get: unavailableOffsetHeight, + }); fireEvent.click( screen.getByRole("button", { name: "Remeasure SVG content" }) ); await flushAnimationFrames(); + expect(unavailableOffsetHeight).not.toHaveBeenCalled(); expect(onRender).toHaveBeenCalledTimes(renderCount);🤖 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/ui/Virtualizer.test.tsx` around lines 993 - 1004, Update the SVG remeasurement test around the “Remeasure SVG content” button to replace the deleted offsetHeight property with a getter that returns undefined and records accesses. After flushAnimationFrames(), assert the getter was not called, while preserving the existing render-count and data-total-size assertions.
🤖 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/browser/ui/Virtualizer.test.tsx`:
- Around line 993-1004: Update the SVG remeasurement test around the “Remeasure
SVG content” button to replace the deleted offsetHeight property with a getter
that returns undefined and records accesses. After flushAnimationFrames(),
assert the getter was not called, while preserving the existing render-count and
data-total-size assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ccfd69be-2d46-472a-89ae-ec9996f89173
📒 Files selected for processing (13)
greenfield/src/browser/chat/ChatLiveProjection.test.tsxgreenfield/src/browser/chat/ChatMessageBubble.test.tsxgreenfield/src/browser/chat/ChatMessageBubble.tsxgreenfield/src/browser/chat/ChatTranscript.test.tsxgreenfield/src/browser/chat/ChatTranscript.tsxgreenfield/src/browser/chat/chatContractAdapter.tsgreenfield/src/browser/chat/chatMessageOrdering.tsgreenfield/src/browser/chat/chatRuntimeStore.tsgreenfield/src/browser/chat/chatToolPresentation.tsgreenfield/src/browser/chat/chatViewProjection.test.tsgreenfield/src/browser/chat/chatViewProjection.tsgreenfield/src/browser/ui/Virtualizer.test.tsxgreenfield/src/browser/ui/Virtualizer.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- greenfield/src/browser/chat/chatToolPresentation.ts
- greenfield/src/browser/chat/ChatMessageBubble.test.tsx
- greenfield/src/browser/ui/Virtualizer.tsx
- greenfield/src/browser/chat/ChatMessageBubble.tsx
- greenfield/src/browser/chat/chatContractAdapter.ts
- greenfield/src/browser/chat/chatRuntimeStore.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: dashboard-checks
- GitHub Check: storybook
- GitHub Check: Analyze JavaScript and TypeScript
🧰 Additional context used
🧠 Learnings (2)
📚 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/browser/chat/chatMessageOrdering.tsgreenfield/src/browser/chat/chatViewProjection.tsgreenfield/src/browser/chat/chatViewProjection.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/browser/chat/chatViewProjection.test.ts
🔇 Additional comments (7)
greenfield/src/browser/chat/chatMessageOrdering.ts (1)
1-59: LGTM!greenfield/src/browser/chat/chatViewProjection.test.ts (1)
9-9: LGTM!Also applies to: 942-1021
greenfield/src/browser/chat/chatViewProjection.ts (1)
18-18: LGTM!Also applies to: 310-326, 327-392
greenfield/src/browser/chat/ChatLiveProjection.test.tsx (1)
296-369: LGTM!greenfield/src/browser/chat/ChatTranscript.test.tsx (1)
7-7: LGTM!Also applies to: 247-258, 289-346
greenfield/src/browser/chat/ChatTranscript.tsx (1)
2-2: LGTM!Also applies to: 133-155, 181-259
greenfield/src/browser/ui/Virtualizer.test.tsx (1)
3-3: LGTM!Also applies to: 175-219
mira-2026
added
status: needs-review
1 participant
Summary
Depends on #411.
/logsparity with durable active/latest-terminal maintenance status;Security and operations
Verification
106735/115292)Preview activation uses a rehearsed, data-preserving database transition with paired rollback before the live preview is changed.