Repository navigation
fix(runtime-logs): preserve PEM context across polling pages - #759
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs changes before merge. Reviewed October 1, 2026, 5:32 AM ET / 09:32 UTC (Revision 6). ClawSweeper reviewWhat this changesThe PR carries authenticated PEM key-block masking state between container-log polls and adds regression coverage and documentation for timestamp uncertainty and view resets. Merge readiness⛔ Needs changes before merge - 1 item remains The fix remains useful because current main loses PEM masking context between polls. No actionable patch defects remain, and the earlier documentation finding is addressed. Enterprise policy retains this PR for maintainer triage. Priority: P2 Review scores
Verification
How this fits togetherOpenClaw Control Plane reads container output through the selected Compute Driver after authorizing the exact Agent. Its runtime-log reader removes overlap, masks sensitive text, and returns bounded records plus a signed cursor to console, CLI, and API callers. flowchart TD
A[Console or CLI log poll] --> B[Exact Agent authorization]
B --> C[Compute Driver container logs]
C --> D[Validate cursor and delivered order]
D --> E[Carry PEM masking context]
E --> F[Redacted records and signed cursor]
F --> A
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Retain one cursor-backed masking implementation that conservatively handles uncertain chronology while keeping ordinary operational text visible. Do we have a high-confidence way to reproduce the issue? Yes, source establishes the current-main failure: poll an observed PEM opening, then a middle page containing only a short body fragment. Main resets block state between those pages; this review did not execute the reproduction. Is this the best way to solve the issue? Yes, carrying authenticated masking context through the existing cursor is a focused repair, and the documented chronology rules match the implementation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against cc521e90a6f9. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
Distinguish equality with the incoming cursor frontier from ordered same-page BEGIN and END boundaries at a newer timestamp. Preserve masking limits.
Preserve PEM masking across runtime-log polling pages by carrying authenticated
pemOpenandpemAfterTimecontext in the cursor. The reader validates the consumed prefix and compares delivered lines with the prior cursor frontier. An ordered line strictly newer than a reliable prior frontier can close a carried block. Ordered same-page BEGIN and END at that newer timestamp can close it; equality with the prior frontier does not establish forward progress. Missing, invalid or reordered times retain conservative open context, and undelivered lookahead cannot advance or close it. Cursor identity, expiry, stream and view resets and per-read authorization remain enforced. Ordinary operational text stays visible; existing JSON and bracket rules are unchanged.The cursor authorization negative test also changes the first MAC character with an explicit decoded-byte precondition. The previous final-
AAconstruction could leave a valid cursor unchanged; this was a test defect, not a production authentication bypass. The type-only follow-up permits the already-usedundefinedvalue in two optional declarations; its JavaScript output was independently verified unchanged. The guide and owning flow distinguish the prior frontier from ordered same-page boundaries and document conservative persistence and initial-tail limits.This update merges pinned main
33a05c987de2d5c91e0243ceea011d7489abadf9into the reviewed48c3a9d9branch. All eight feature-path blobs remain identical to48c3a9d9, every other leaf of that pinned main is preserved, and published ancestry is retained. Main changed 116 leaves since the previously tested base13cd2e81, with no overlap in these eight paths. Receiving those changes is not a claim that they fix the historical CI failure.Validation and remaining limits:
d4aaadadexecuted all four named controller routes successfully, with 52 top-level tests filtered. That is predecessor evidence, not a fourth current-composition route pass or a historical hosted result.git diff --checkpassed. Full workspace lint, typecheck, documentation build and CI have not been rerun locally. Previous source reviews and scoped checks remain evidence only for their stated tuples and scope.36795515059, attempt 1, on published48c3a9d9remains failed. Its tested merge wasd4aaadadwith parents13cd2e81and48c3a9d9; the individual baseline failure cause is unresolved. The two previously denied result artifacts remain unreceived and were not retried, refreshed or accessed by an alternate route. Passing local routes does not erase that failure. This new merge needs its own hosted required checks and named-case receiving.reviewer_unavailable/engine_failed, reviewer exit 124, no report; the wrapper was terminal after 120.86 seconds. External descendant settlement is unproven. This is unavailable review evidence, not a helper PASS. Earlier d25 and 48c helper timeouts remain separate; the type-only correction had no new invocation. No historical helper request was retried.This draft still requires final-head required CI, the named route evidence, review-conversation and Claw disposition, and applicable repository protection checks before landing. Unknown initial log tails without an observed boundary remain best-effort; this change is not a general secret-detection guarantee.