Add Governance Ledger foundation#128
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR introduces a complete governance ledger feature that persistently records approval and oversight decisions across the harness platform. It spans core data models, file-based persistence, service orchestration, admin APIs, integration into existing tool approval workflows, export functionality, admin UI, and comprehensive test coverage. ChangesGovernance Ledger Feature
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🤖 Augment PR SummarySummary: Adds a passive “Governance Ledger” foundation to persist approval/oversight decisions as durable harness state without changing approval enforcement semantics. Changes:
Technical Notes: Ledger entries are passive/inspectable only; writes are designed to be non-blocking and not gate the underlying approval decision path. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| approved, | ||
| GovernanceLedgerSources.ToolApproval, | ||
| EndpointHelpers.GetOperatorActorId(ctx, auth), | ||
| auth.AuthMode == "browser-session" ? "browser" : "http", |
There was a problem hiding this comment.
In the admin-override path, TryRecordApprovalDecisionAsync(..., actorChannelId, actorSenderId, ...) is being passed values derived from auth.AuthMode (e.g., "browser" / "browser-session") rather than something like the audit’s "http_admin" channel and the operator identity. This makes ActorId/metadata misleading for governance ledger entries and can break filtering/auditing expectations.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| Source = NormalizeSource(entry.Source), | ||
| ActionType = CleanOptional(entry.ActionType), | ||
| ToolName = CleanOptional(entry.ToolName), | ||
| ActionSummary = entry.ActionSummary, |
There was a problem hiding this comment.
Normalize() redacts/truncates arguments, but other free-text fields like ActionSummary (and later DecisionReason/Tags) are persisted as-is; if these ever include secrets (e.g., derived from tool summaries), they’ll be stored unredacted. Consider consistently applying _redaction to these fields too so the ledger can’t accidentally persist secret material.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
src/OpenClaw.Tests/GatewayAdminEndpointTests.cs (1)
459-460: ⚡ Quick winStrengthen redaction assertions to avoid false positives on missing fields.
These checks pass even if
redactedArgumentsisnull/missing. Assert field presence first, then verify the secret is absent.Suggested test hardening
- Assert.DoesNotContain("sk-testsecret123", createPayload.RootElement.GetProperty("entry").GetProperty("redactedArguments").GetString()); + var createRedacted = createPayload.RootElement.GetProperty("entry").GetProperty("redactedArguments").GetString(); + Assert.False(string.IsNullOrWhiteSpace(createRedacted)); + Assert.DoesNotContain("sk-testsecret123", createRedacted, StringComparison.Ordinal); ... - Assert.DoesNotContain("sk-testsecret123", items[0].GetProperty("redactedArguments").GetString()); + var listedRedacted = items[0].GetProperty("redactedArguments").GetString(); + Assert.False(string.IsNullOrWhiteSpace(listedRedacted)); + Assert.DoesNotContain("sk-testsecret123", listedRedacted, StringComparison.Ordinal);Also applies to: 3661-3664
🤖 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 `@src/OpenClaw.Tests/GatewayAdminEndpointTests.cs` around lines 459 - 460, The test currently uses Assert.DoesNotContain on createPayload.RootElement.GetProperty("entry").GetProperty("redactedArguments").GetString() which will pass if the redactedArguments property is missing or null; first assert the property exists and is non-null (e.g., via entry.TryGetProperty("redactedArguments", out var redactedArgs) or Assert.True on HasProperty/GetProperty) and that redactedArgs.GetString() is not null/empty, then perform Assert.DoesNotContain("sk-testsecret123", redactedArgs.GetString()). Apply the same presence-first pattern to the other assertions referenced (lines ~3661-3664).src/OpenClaw.Gateway/GovernanceLedgerService.cs (1)
277-305: 💤 Low valueConsider excluding
OperationCanceledExceptionfrom the catch filter.The
TryRecord*methods correctly rethrowOperationCanceledException, butAppendEventswallows it. This could mask cancellation signals during shutdown, though the impact is limited since event append is a fire-and-forget side effect.Suggested fix
- catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException) + catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException and not OperationCanceledException)🤖 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 `@src/OpenClaw.Gateway/GovernanceLedgerService.cs` around lines 277 - 305, The catch in AppendEvent currently filters out OutOfMemoryException and StackOverflowException but still swallows OperationCanceledException; update the catch filter in GovernanceLedgerService.AppendEvent so it also excludes OperationCanceledException (i.e., catch (Exception ex) when (ex is not OutOfMemoryException and not StackOverflowException and not OperationCanceledException)) so cancellation is propagated rather than being logged and swallowed; keep the existing _logger.LogWarning call for other exceptions and retain the RuntimeEventEntry construction and _runtimeEvents.Append logic.src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs (1)
143-153: 💤 Low valueConsider extracting duplicated governance ledger recording logic.
The governance ledger recording blocks for
tool_approval_decisionmessages (lines 143-153) and/approvecommands (lines 227-237) are nearly identical. A small helper method could reduce duplication.♻️ Proposed helper extraction
+ private static async Task TryRecordApprovalToLedgerAsync( + GovernanceLedgerService? governanceLedger, + ToolApprovalRequest request, + bool approved, + string senderId, + string channelId, + CancellationToken ct) + { + if (governanceLedger is not null) + { + await governanceLedger.TryRecordApprovalDecisionAsync( + request, + approved, + GovernanceLedgerSources.ToolApproval, + senderId, + channelId, + senderId, + ct); + } + }Then replace both inline blocks with:
await TryRecordApprovalToLedgerAsync(governanceLedger, decisionOutcome.Request, approved, msg.SenderId, msg.ChannelId, lifetime.ApplicationStopping);Also applies to: 227-237
🤖 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 `@src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs` around lines 143 - 153, The two blocks in GatewayInboundMessageWorker that call governanceLedger.TryRecordApprovalDecisionAsync for tool_approval_decision and /approve are duplicated; extract them into a private helper (e.g., TryRecordApprovalToLedgerAsync) that accepts (IGovernanceLedger governanceLedger, RequestType request, bool approved, string senderId, string channelId, CancellationToken token) and calls governanceLedger.TryRecordApprovalDecisionAsync with GovernanceLedgerSources.ToolApproval, senderId for both user fields, and the provided token; then replace both inline blocks with a single await TryRecordApprovalToLedgerAsync(...) invocation to remove duplication (keep method name TryRecordApprovalToLedgerAsync and the existing governanceLedger.TryRecordApprovalDecisionAsync call to preserve 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 `@src/OpenClaw.Core/Features/FileGovernanceLedgerStore.cs`:
- Around line 87-90: RevokeAsync currently accepts blank revokedBy/reason which
yields poor audit records; at the start of RevokeAsync (after EnsureSafeId)
validate that revokedBy and reason are not null/empty/whitespace and throw
ArgumentException (or OperationCanceled/InvalidOperation per project convention)
if invalid; apply the same non-blank validation to the other code path that
writes revoked entries (the revocation branch referenced around the later
revoked-entry write) and ensure the persisted revoked entry includes the
validated values passed through LoadOneAsync/FileForId so blank values are never
written.
- Around line 245-249: In the finally block in FileGovernanceLedgerStore (where
tempPath and cleanupFile are used), wrap the cleanupFile.Delete() call in a
try/catch that catches Exception, logs the failure (including
cleanupFile.FullName and the exception) and swallows it so cleanup failures do
not propagate as save failures; ensure any existing success path (e.g., after
moving the temp file into place) remains unaffected and no exception from
Delete() is rethrown.
In `@src/OpenClaw.Gateway/AdminObservabilityService.cs`:
- Around line 391-396: The code currently hard-codes GovernanceLedgerListQuery {
Limit = 5000 } for calls to _governanceLedger.ListAsync (seen in the block
returning the query and the similar block at lines 416-423), which silently
truncates exports; change this by removing the fixed Limit and implementing
proper pagination: repeatedly call _governanceLedger.ListAsync with the
GovernanceLedgerListQuery (using whatever continuation token/offset the ledger
API provides) to accumulate all pages into the final result (or accept an
explicit caller-specified max and return a truncation indicator), and ensure the
method returns the full assembled list (or a list plus a boolean/metadata flag
indicating truncation) instead of a single 5,000-item page.
In `@src/OpenClaw.Gateway/Endpoints/AdminEndpoints.GovernanceLedger.cs`:
- Around line 93-95: The handlers assume ReadJsonBodyAsync returns a Failure for
bad input, but malformed JSON throws (e.g., System.Text.Json.JsonException) and
escapes the handler; wrap the ReadJsonBodyAsync call in a try/catch that catches
JsonException (and optionally IOException) around the existing calls that assign
requestPayload (the occurrences using
CoreJsonContext.Default.GovernanceLedgerEntry), and convert the caught exception
into the same 400-style response used when requestPayload.Failure is set (return
a BadRequest/failure result with a clear message or the exception.Message);
apply this change to both places where requestPayload is read (the
ReadJsonBodyAsync usages around the current requestPayload.Failure checks).
In `@src/OpenClaw.Gateway/wwwroot/admin.html`:
- Around line 2441-2444: Reset governanceState.selectedId and
governanceState.detail to null in the unauthenticated/logout code paths so
cached governance data is cleared; specifically, in the functions/blocks that
handle the unauthenticated path (the same places referenced around the existing
governanceState declaration and the other occurrence at the later
unauthenticated handler), set governanceState.selectedId = null and
governanceState.detail = null and also clear any UI panes that render governance
list/detail (remove or blank their innerHTML or bound variables) so the previous
session's ledger entries are not visible after logout.
- Around line 1404-1443: Add an "actor" filter input to the governance panel by
inserting an input/select with id "governance-actor-input" (e.g., <input
id="governance-actor-input" type="text" placeholder="Actor">) alongside the
other filters, and update the code that builds the governance ledger query (the
function that reads `#governance-decision-input`, `#governance-status-input`,
`#governance-tool-input`, `#governance-risk-input`, `#governance-scope-input`,
`#governance-session-input`, `#governance-tag-input`) to also read the value of
`#governance-actor-input` and include it as the "actor" query parameter when
fetching/filtering results; make the same HTML and query change at the other
occurrence referenced (lines 4723-4730).
- Around line 1394-1458: The governance UI is always rendered and refreshAll()
always loads it; fix by checking session permissions and gating both rendering
and actions: on init call the existing session/permissions check (or
hasPermission('governance')) and if the user lacks access, disable/hide the
governance controls (governance-refresh-button, governance-revoke-button, revoke
textarea and filter inputs like governance-decision-input) and prevent
refreshAll() from fetching the ledger; instead set governance-list-output and
governance-detail-output to a muted "Access denied" message. If the user has
permission, enable controls and allow refreshAll() to load as before. Ensure
revoke action handler also checks permission before sending any request.
---
Nitpick comments:
In `@src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs`:
- Around line 143-153: The two blocks in GatewayInboundMessageWorker that call
governanceLedger.TryRecordApprovalDecisionAsync for tool_approval_decision and
/approve are duplicated; extract them into a private helper (e.g.,
TryRecordApprovalToLedgerAsync) that accepts (IGovernanceLedger
governanceLedger, RequestType request, bool approved, string senderId, string
channelId, CancellationToken token) and calls
governanceLedger.TryRecordApprovalDecisionAsync with
GovernanceLedgerSources.ToolApproval, senderId for both user fields, and the
provided token; then replace both inline blocks with a single await
TryRecordApprovalToLedgerAsync(...) invocation to remove duplication (keep
method name TryRecordApprovalToLedgerAsync and the existing
governanceLedger.TryRecordApprovalDecisionAsync call to preserve behavior).
In `@src/OpenClaw.Gateway/GovernanceLedgerService.cs`:
- Around line 277-305: The catch in AppendEvent currently filters out
OutOfMemoryException and StackOverflowException but still swallows
OperationCanceledException; update the catch filter in
GovernanceLedgerService.AppendEvent so it also excludes
OperationCanceledException (i.e., catch (Exception ex) when (ex is not
OutOfMemoryException and not StackOverflowException and not
OperationCanceledException)) so cancellation is propagated rather than being
logged and swallowed; keep the existing _logger.LogWarning call for other
exceptions and retain the RuntimeEventEntry construction and
_runtimeEvents.Append logic.
In `@src/OpenClaw.Tests/GatewayAdminEndpointTests.cs`:
- Around line 459-460: The test currently uses Assert.DoesNotContain on
createPayload.RootElement.GetProperty("entry").GetProperty("redactedArguments").GetString()
which will pass if the redactedArguments property is missing or null; first
assert the property exists and is non-null (e.g., via
entry.TryGetProperty("redactedArguments", out var redactedArgs) or Assert.True
on HasProperty/GetProperty) and that redactedArgs.GetString() is not null/empty,
then perform Assert.DoesNotContain("sk-testsecret123",
redactedArgs.GetString()). Apply the same presence-first pattern to the other
assertions referenced (lines ~3661-3664).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 84a6ccd6-a64e-4adf-9548-d89e5540dbca
📒 Files selected for processing (28)
README.mddocs/GOVERNANCE_LEDGER.mddocs/README.mdsrc/OpenClaw.Core/Abstractions/IGovernanceLedgerStore.cssrc/OpenClaw.Core/Features/FileGovernanceLedgerStore.cssrc/OpenClaw.Core/Models/GovernanceLedgerModels.cssrc/OpenClaw.Core/Models/OperatorGovernanceModels.cssrc/OpenClaw.Core/Models/Session.cssrc/OpenClaw.Gateway/AdminObservabilityService.cssrc/OpenClaw.Gateway/Composition/CoreServicesExtensions.cssrc/OpenClaw.Gateway/Composition/FeatureFallbackServices.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.GovernanceLedger.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.Setup.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.cssrc/OpenClaw.Gateway/Endpoints/ControlEndpoints.cssrc/OpenClaw.Gateway/Endpoints/EndpointHelpers.cssrc/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cssrc/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cssrc/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cssrc/OpenClaw.Gateway/Extensions/GatewayWorkers.cssrc/OpenClaw.Gateway/GatewayPaymentApprovalService.cssrc/OpenClaw.Gateway/GovernanceLedgerService.cssrc/OpenClaw.Gateway/Pipeline/PipelineExtensions.cssrc/OpenClaw.Gateway/ToolApprovalCallbackFactory.cssrc/OpenClaw.Gateway/wwwroot/admin.htmlsrc/OpenClaw.Tests/GatewayAdminEndpointTests.cssrc/OpenClaw.Tests/GovernanceLedgerTests.cs
There was a problem hiding this comment.
Pull request overview
Adds a passive Governance Ledger feature to the OpenClaw gateway/runtime: durable storage + inspection of approval/oversight decisions, with optional inclusion in admin audit/trajectory exports and best-effort recording from existing approval flows (without changing enforcement semantics).
Changes:
- Introduces Governance Ledger models, JSON source-gen coverage, file-backed persistence (
IGovernanceLedgerStore+FileGovernanceLedgerStore), and a gatewayGovernanceLedgerService. - Adds authenticated admin API + admin UI panel for creating, listing, inspecting, and revoking ledger entries.
- Hooks governance recording into existing tool-approval paths and adds optional export support (
includeGovernance) for audit bundles and trajectory JSONL.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/OpenClaw.Tests/GovernanceLedgerTests.cs | Adds unit tests for JSON round-trip, file store behavior, and service recording/redaction. |
| src/OpenClaw.Tests/GatewayAdminEndpointTests.cs | Adds admin API contract coverage for governance endpoints and export include/exclude behavior. |
| src/OpenClaw.Gateway/wwwroot/admin.html | Adds admin UI for governance ledger + export toggles. |
| src/OpenClaw.Gateway/ToolApprovalCallbackFactory.cs | Records governance entries for grant-consumed and timeout cases during tool approval callbacks. |
| src/OpenClaw.Gateway/Pipeline/PipelineExtensions.cs | Wires governance ledger into worker startup. |
| src/OpenClaw.Gateway/GovernanceLedgerService.cs | New service for normalizing, redacting (partial), persisting, listing, and revoking ledger entries. |
| src/OpenClaw.Gateway/GatewayPaymentApprovalService.cs | Records governance entries for payment approval outcomes (approved/denied/timeout). |
| src/OpenClaw.Gateway/Extensions/GatewayWorkers.cs | Adds governance ledger dependency to worker bootstrap chain. |
| src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs | Records governance ledger entries for chat-sourced approval decisions. |
| src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs | Passes governance ledger into tool approval callback for OpenAI Responses route. |
| src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs | Passes governance ledger into tool approval callback for OpenAI Chat Completions route. |
| src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs | Ensures governance mutate scopes map to operator-level access. |
| src/OpenClaw.Gateway/Endpoints/ControlEndpoints.cs | Records governance entries when approvals are decided via the admin /tools/approve endpoint. |
| src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs | Extends admin endpoint service bundle to include governance ledger service. |
| src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Setup.cs | Adds includeGovernance query flags to audit/trajectory export endpoints. |
| src/OpenClaw.Gateway/Endpoints/AdminEndpoints.GovernanceLedger.cs | New admin endpoints for governance ledger list/detail/create/revoke. |
| src/OpenClaw.Gateway/Endpoints/AdminEndpoints.cs | Registers governance ledger endpoints + service resolution. |
| src/OpenClaw.Gateway/Composition/FeatureFallbackServices.cs | Adds fallback resolver for governance ledger service/store/redaction. |
| src/OpenClaw.Gateway/Composition/CoreServicesExtensions.cs | Registers governance ledger service and file store in DI. |
| src/OpenClaw.Gateway/AdminObservabilityService.cs | Adds optional inclusion of governance records in audit bundle + trajectory export (with anonymization support). |
| src/OpenClaw.Core/Models/Session.cs | Adds source-generated JSON registrations for governance ledger models. |
| src/OpenClaw.Core/Models/OperatorGovernanceModels.cs | Extends trajectory export record model to include governance ledger entries. |
| src/OpenClaw.Core/Models/GovernanceLedgerModels.cs | Adds governance ledger model types/constants and admin API DTOs. |
| src/OpenClaw.Core/Features/FileGovernanceLedgerStore.cs | Implements file-backed governance ledger persistence and filtering. |
| src/OpenClaw.Core/Abstractions/IGovernanceLedgerStore.cs | Introduces store abstraction for governance ledger persistence. |
| README.md | Mentions Governance Ledger in top-level feature list. |
| docs/README.md | Adds Governance Ledger documentation link. |
| docs/GOVERNANCE_LEDGER.md | New documentation describing scope/relationships and non-goals. |
Comment on lines
+244
to
+248
| Source = NormalizeSource(entry.Source), | ||
| ActionType = CleanOptional(entry.ActionType), | ||
| ToolName = CleanOptional(entry.ToolName), | ||
| ActionSummary = entry.ActionSummary, | ||
| ArgumentSummary = argumentSummary, |
Comment on lines
+68
to
+72
| new ToolApprovalRequest | ||
| { | ||
| ApprovalId = grant.Id, | ||
| SessionId = session.Id, | ||
| ChannelId = approvalChannelId, |
| <option value="rejected">Rejected</option> | ||
| <option value="escalated">Escalated</option> | ||
| <option value="expired">Expired</option> | ||
| <option value="revoked">Revoked</option> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/OpenClaw.Gateway/AdminObservabilityService.cs (1)
391-396: 💤 Low valueGovernance exports now unbounded.
Setting
Limit = 0removes the 5,000-entry cap, addressing the truncation concern from past feedback. However, this means very large ledgers could produce memory pressure during export. Consider whether pagination or streaming would be more appropriate for large-scale deployments, or document this as a known limitation.Note: The
EvidenceBundlequery at line 458 still usesLimit = 5000, which creates inconsistent behavior between evidence and governance exports.Also applies to: 416-421
🤖 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 `@src/OpenClaw.Gateway/AdminObservabilityService.cs` around lines 391 - 396, The Governance ledger export in AdminObservabilityService currently sets GovernanceLedgerListQuery.Limit = 0 (via the call to _governanceLedger.ListAsync), which removes the previous 5,000 cap and can cause memory pressure for very large exports and is inconsistent with the EvidenceBundle query that still uses Limit = 5000; either implement pagination/streaming in the governance export (call ListAsync repeatedly with offset/marker or a paged iterator until no more results) or restore a sensible fixed Limit and loop to fetch pages to avoid unbounded in-memory results, and make the behavior consistent with the EvidenceBundle query (or add a clear comment documenting the limitation) so both exports use the same paging strategy.src/OpenClaw.Gateway/GovernanceLedgerService.cs (1)
409-410: ⚡ Quick win
CleanRedactedOptionalshould handle post-redaction empty results.
CleanRedactedStrings(lines 400-404) filters out empty/whitespace results after redaction, butCleanRedactedOptionaldoes not. If the redaction pipeline completely removes sensitive content (returning an empty string), this method will return""instead ofnull, inconsistent withCleanOptional's behavior and the sister method.Proposed fix
private string? CleanRedactedOptional(string? value) - => string.IsNullOrWhiteSpace(value) ? null : _redaction.Redact(value.Trim()); +{ + if (string.IsNullOrWhiteSpace(value)) + return null; + var redacted = _redaction.Redact(value.Trim()); + return string.IsNullOrWhiteSpace(redacted) ? null : redacted; +}🤖 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 `@src/OpenClaw.Gateway/GovernanceLedgerService.cs` around lines 409 - 410, CleanRedactedOptional currently returns an empty string when redaction strips all content; update it to mirror CleanRedactedStrings/CleanOptional by trimming, redacting, then treating null or whitespace post-redaction as null. In other words, in CleanRedactedOptional (and only that method) accept a nullable input, return null if input is null/whitespace, otherwise call _redaction.Redact on the trimmed value and if the redaction result is null or whitespace return null, else return the redacted string.
🤖 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 `@src/OpenClaw.Gateway/wwwroot/admin.html`:
- Around line 4790-4796: When the filtered list is empty the code resets
governanceState.selectedId and governanceState.detail but does not clear the UI
elements that display the inspected entry and revoke reason; update the
!items.length branch to also clear any rendered detail pane and revoke-reason UI
(e.g. the DOM nodes that show the inspected entry and revoke reason) and then
call applyGovernanceAccessState() so the UI fully reflects the empty result set;
locate the logic around governanceState.selectedId/governanceState.detail and
governanceListOutput.textContent and add steps to clear the detail/revoke-reason
DOM content there.
---
Nitpick comments:
In `@src/OpenClaw.Gateway/AdminObservabilityService.cs`:
- Around line 391-396: The Governance ledger export in AdminObservabilityService
currently sets GovernanceLedgerListQuery.Limit = 0 (via the call to
_governanceLedger.ListAsync), which removes the previous 5,000 cap and can cause
memory pressure for very large exports and is inconsistent with the
EvidenceBundle query that still uses Limit = 5000; either implement
pagination/streaming in the governance export (call ListAsync repeatedly with
offset/marker or a paged iterator until no more results) or restore a sensible
fixed Limit and loop to fetch pages to avoid unbounded in-memory results, and
make the behavior consistent with the EvidenceBundle query (or add a clear
comment documenting the limitation) so both exports use the same paging
strategy.
In `@src/OpenClaw.Gateway/GovernanceLedgerService.cs`:
- Around line 409-410: CleanRedactedOptional currently returns an empty string
when redaction strips all content; update it to mirror
CleanRedactedStrings/CleanOptional by trimming, redacting, then treating null or
whitespace post-redaction as null. In other words, in CleanRedactedOptional (and
only that method) accept a nullable input, return null if input is
null/whitespace, otherwise call _redaction.Redact on the trimmed value and if
the redaction result is null or whitespace return null, else return the redacted
string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40b3cc25-7be8-479d-807a-339b067d1836
📒 Files selected for processing (9)
src/OpenClaw.Core/Features/FileGovernanceLedgerStore.cssrc/OpenClaw.Gateway/AdminObservabilityService.cssrc/OpenClaw.Gateway/Endpoints/AdminEndpoints.GovernanceLedger.cssrc/OpenClaw.Gateway/Endpoints/ControlEndpoints.cssrc/OpenClaw.Gateway/GovernanceLedgerService.cssrc/OpenClaw.Gateway/ToolApprovalCallbackFactory.cssrc/OpenClaw.Gateway/wwwroot/admin.htmlsrc/OpenClaw.Tests/GatewayAdminEndpointTests.cssrc/OpenClaw.Tests/GovernanceLedgerTests.cs
This was referenced May 22, 2026
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
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.
Summary
Behavior Notes
PolicyHintis stored/displayed only and is not read by approval enforcement code.Validation
dotnet test src/OpenClaw.Tests/OpenClaw.Tests.csproj --filter "GovernanceLedger|HarnessContract|EvidenceBundle|GatewayAdminEndpointTests|EndpointConformanceTests"— 156 passed.dotnet test OpenClaw.Net.slnx— 1435 passed.git diff --check— passed.Summary by CodeRabbit
New Features
Integrations
Documentation
Tests