task-139: audit gateway restart attribution#78742
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep this PR open, but it is not merge-ready: the restart-attribution direction is useful, while the branch is conflicted with current main and would need a rebase that preserves the current SQLite restart state, session ownership guard, and privacy boundaries. Canonical path: Close this PR as superseded by #97189. So I’m closing this here and keeping the remaining discussion on #97189. Review detailsBest possible solution: Close this PR as superseded by #97189. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main's slash-command restart path and signal handlers do not emit the proposed three-phase gateway-restart.log attribution chain. I did not run a live gateway restart in this read-only review. Is this the best way to solve the issue? No for the current branch shape. The audit direction is plausible, but the best fix must use current SQLite restart state, preserve session ownership, and settle identifier logging before merge. Security review: Security review needs attention: The diff introduces raw sender/session-style restart audit identifiers into diagnostic logs and needs redaction or explicit maintainer approval.
AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 72f837a4a478. |
|
Hi maintainers — requesting guidance on the Current CI stateAfter the latest fix commit ( Check rollup post-fix (head
Local verification before push:
Why this is a chicken-and-egg under runtime freezeThis PR is the FIX for the observability gap: today, Producing PR-comment-grade after-fix terminal output from a real running gateway requires:
Steps 1–2 require either upstream merge/release first, or local installation of an unmerged branch on a production gateway — neither of which is possible under our current change-control posture. Safety profile of the changeThis PR is observability-only:
Asks for the maintainerCould you advise on one of the following acceptable proof formats? Happy to follow whichever you prefer:
Happy to provide whatever evidence form best fits the project's standards. Thanks for taking a look. |
Real behavior proof — sacrificial dev gateway smoke (Path 2b)Reporting back on the policy gate. Ran a sacrificial dev-gateway smoke against this branch (head Isolation summary
Flow exercised
gateway-restart.log capturedWhat each line proves:
Production drift = zeroPre-flight and post-teardown both confirm:
After the smoke, the dev gateway processes were killed and Cross-checkThis evidence matches the focused integration test at Happy to provide the driver script ( Requesting the |
|
ClawSweeper applied the proposed close for this PR.
|
Problem
When the operator types
/restartto the gateway via TG / Control-UI / any chat-channel slash command, the gateway is killed via supervisor (launchctl kickstart -kon macOS,systemctl --user restarton Linux) and relaunched by KeepAlive. This restart does not write any line togateway-restart.log, leaving operators no audit trail to attribute the restart to a trigger surface, sender, or session.In a recent incident (RCA at Category 2 — external lane manual kickstart, ~95% confidence), this observability gap forced 30+ minutes of cross-log correlation to figure out who/what triggered a gateway PID change in the middle of an active diagnostic.
gateway-restart.logonly recorded openclaw-CLI-managed restarts (source=update,source=launchd-handoff); the chat slash-command path was silent.Root cause
The chat slash
/restarthandler atsrc/auto-reply/reply/commands-session.ts:716callstriggerOpenClawRestart()fromsrc/infra/restart.ts:563, whichspawnSyncslaunchctl kickstart -k(orsystemctl --user restart) without ever appending togateway-restart.log. The signal-handler choke point atsrc/cli/gateway-cli/run-loop.ts:460-509(onSigterm/onSigusr1/onSigint) also did not log on receipt. After supervisor relaunch, the new gateway process had no mechanism to attribute the just-completed restart.Solution summary
Six additive observability hooks. No behavior change to restart logic:
RestartAuditContext(src/infra/restart.ts): coarse channel-name string for sender attribution.writeGatewayRestartIntentSyncaccepts optionalauditfield, written intogateway-restart-intent.json.triggerOpenClawRestartacceptsopts.auditand appends a structuredrestart-dispatchline beforespawnSync(launchctl|systemctl|...)./restarthandler builds atriggerAudit(source: "slash-command",senderId,sessionKey,actionLabel: "/restart", channel name) and threads it through both the SIGUSR1 in-process path (viascheduleGatewaySigusr1Restart({triggerAudit})) and the supervisor path (viatriggerOpenClawRestart({audit})).RestartSentinelPayloadgets an optionalauditsub-field so the new gateway process can read sender attribution from the predecessor's sentinel and emit the post-bootrestart-completedline with stableold_pid → new_pidlinkage.run-loop.tsappendrestart-signal-receivedlines on every SIGTERM / SIGUSR1 / SIGINT receipt; falls back tosource=externalwhen no audit context is present (e.g., directkill -TERM <pid>).New helper
appendGatewayRestartAuditLinewrites structured lines using disjoint vocabulary from the existing shell-wrapper lines (gateway restart-{dispatch,signal-received,completed}vs legacyopenclaw restart {attempt,done,fallback,finished}) so existing parsers keep working.Resulting log shape for
/restartslash command (verified by tests):Files changed
src/infra/restart.tssrc/auto-reply/reply/commands-session.tssrc/cli/gateway-cli/run-loop.tssrc/infra/restart-sentinel.tssrc/infra/restart-audit.test.tsTotal: 5 files, +1029 / −1 lines.
Test evidence
Typecheck core / typecheck core tests / lint core / format check: all clean.
Test coverage highlights (18 cases in
src/infra/restart-audit.test.ts)auditfield still parserestart-dispatchline shape regex assertionsource=externalfallback when no audit context provided (rawkill -TERM)restart-completedline uses sentinel audit when presentisFirstStart=trueon the new process)oldPidwhen sentinel claimsprocess.pid(defensive)source=externalsource=slash-command + sender_id,old_pidstable across all 3 lines,new_pidpins the boot sideSafety boundaries
This PR is observability-only.
spawnSync(launchctl|systemctl|...)calls, signal types (SIGTERM / SIGUSR1 / SIGINT semantics),KeepAliveplist policy, restart timing, in-process restart scheduler — all unchanged.gateway-restart.logand existing sentinel/intent files).try/catchwithrestartLog.warnon failure; restart logic never blocked by an I/O error.sanitizeRestartAuditContexttrims whitespace, drops empty fields, and caps lengths (source64,senderId128,sessionKey256, etc.) so a malformed payload from a stale / hostile writer cannot inflate the intent file past its existing 1024-byte ceiling.old_pid=new_pidrows.auditfield is optional and additive — old sentinels parse fine.Rollout
Each gate has been independently verified locally:
Pending (require explicit upstream/operator action):
/restartpost-deploy and confirms all 3 audit lines appear in~/.openclaw/logs/gateway-restart.logRollback
gateway restart-…vsopenclaw restart …), so existing dashboards / log parsers are unaffected by either landing or reverting.restartLog.warn, restart still completes.auditfield is optional read-side — pre-PR sentinels and post-PR readers both work without modification.Production audit behavior NOT LIVE
This PR does not modify any running gateway. The audit-line behavior described above is not active in production until:
openclawnpm package./restartsmoke verifies the 3-line audit chain ingateway-restart.log.Until all five steps complete, the existing observability gap remains in production.
Evidence pack
G3-RERUN-BM-2026-05-07.mdG3-RERUN-COMMIT-DIFF.patch(1262 lines, 46 KB)~/clawd/mesh/g-2.7-6-native-tg-smoke/task-138-proposal/🤖 Generated with Claude Code