-
Notifications
You must be signed in to change notification settings - Fork 35
feat(recovery): record passive trajectory ledger (#1017) #1073
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
af7962c
e4cdcb6
4851b7c
70f6a03
a6f78ed
2ab2f89
0a68816
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,11 +61,13 @@ import { extractRunId, getRunStore } from './run-harness/store'; | |
| import { | ||
| substituteSecrets, | ||
| redactSecrets, | ||
| redactSecretString, | ||
| MissingSecretError, | ||
| getSecretStore, | ||
| } from './core/secrets'; | ||
| import { currentRequestContext } from './observability/request-id'; | ||
| import type { TransportMessageContext } from './transports'; | ||
| import { RecoveryTrajectoryLedger, type RecoveryResultStatus } from './recovery'; | ||
|
|
||
|
|
||
| function redactActVariablesForTelemetry(toolName: string, args: Record<string, unknown>): Record<string, unknown> { | ||
|
|
@@ -335,6 +337,7 @@ export class MCPServer { | |
| private activityTracker: ActivityTracker | null = null; | ||
| private operationController: OperationController | null = null; | ||
| private hintEngine: HintEngine | null = null; | ||
| private recoveryLedger: RecoveryTrajectoryLedger | null = null; | ||
| private options: MCPServerOptions; | ||
| private profileWarningShown = false; | ||
| private exposedTier: ToolTier = 1; | ||
|
|
@@ -438,6 +441,14 @@ export class MCPServer { | |
| this.hintEngine.enableLogging(hintsDir); | ||
| this.hintEngine.enableLearning(hintsDir); | ||
|
|
||
| // Initialize passive recovery trajectory ledger (#1017). Default-on with the | ||
| // existing .openchrome harness logs; set OPENCHROME_RECOVERY_LEDGER=0 to disable. | ||
| if (process.env.OPENCHROME_RECOVERY_LEDGER !== '0') { | ||
| this.recoveryLedger = new RecoveryTrajectoryLedger({ | ||
| dirPath: path.join(process.cwd(), '.openchrome', 'recovery'), | ||
| }); | ||
| } | ||
|
|
||
| // Initialize task journal | ||
| getTaskJournal().init().catch((err: unknown) => { | ||
| console.error('[MCPServer] Task journal init failed:', err); | ||
|
|
@@ -1812,6 +1823,8 @@ export class MCPServer { | |
|
|
||
| // End activity tracking (success) | ||
| this.activityTracker!.endCall(callId, 'success'); | ||
| result = redactSecrets(result); | ||
| this.recordRecoveryTrajectory(callId, toolName, sessionId, toolArgs, result.isError ? 'no_progress' : 'success', result); | ||
| getDashboardState().recordToolEnd(callId, 'success'); | ||
|
|
||
| // Record Prometheus metrics | ||
|
|
@@ -1999,11 +2012,13 @@ export class MCPServer { | |
| return finalResult; | ||
| } catch (error) { | ||
| const message = formatError(error); | ||
| const redactedMessage = redactSecretString(message); | ||
| const abortReason = isClientDisconnect(error) ? 'client_disconnect' : null; | ||
| const aborted = abortReason !== null; | ||
|
|
||
| // End activity tracking (error) | ||
| this.activityTracker!.endCall(callId, aborted ? 'aborted' : 'error', message); | ||
| this.recordRecoveryTrajectory(callId, toolName, sessionId, toolArgs, aborted ? 'aborted' : 'error', undefined, redactedMessage); | ||
| getDashboardState().recordToolEnd(callId, aborted ? 'aborted' : 'error', message); | ||
|
|
||
| // Audit log failed invocation — same correlation fields as success path. | ||
|
|
@@ -2430,6 +2445,53 @@ export class MCPServer { | |
| * Get a tool handler by name (for internal server-side plan execution). | ||
| * Returns null if the tool is not registered. | ||
| */ | ||
|
|
||
| private recordRecoveryTrajectory( | ||
| callId: string, | ||
| toolName: string, | ||
| sessionId: string, | ||
| toolArgs: Record<string, unknown>, | ||
| resultStatus: RecoveryResultStatus, | ||
| result?: MCPResult, | ||
| error?: string, | ||
| ): void { | ||
| if (!this.recoveryLedger || !this.activityTracker) return; | ||
|
|
||
| try { | ||
| const recent = this.activityTracker.getRecentCalls(3, sessionId); | ||
| const current = recent.find((call) => call.id === callId); | ||
| const tabId = typeof toolArgs.tabId === 'string' ? toolArgs.tabId : undefined; | ||
| const previousTrajectory = this.recoveryLedger.getLastNode(sessionId, tabId); | ||
| const previousFailed = | ||
| previousTrajectory?.resultStatus === 'error' || | ||
| previousTrajectory?.resultStatus === 'no_progress' || | ||
| previousTrajectory?.resultStatus === 'aborted'; | ||
|
Comment on lines
+2465
to
+2468
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Recovery detection treats Useful? React with 👍 / 👎. |
||
| const recovered = | ||
| resultStatus === 'success' && | ||
| previousTrajectory !== undefined && | ||
| previousFailed && | ||
| previousTrajectory.toolName !== toolName; | ||
| const progressStatus = | ||
| resultStatus === 'error' || resultStatus === 'no_progress' || current?.result === 'error' | ||
| ? 'stuck' | ||
| : 'unknown'; | ||
|
|
||
| this.recoveryLedger.record({ | ||
| sessionId, | ||
| tabId, | ||
| toolName, | ||
| args: toolArgs, | ||
| resultStatus: recovered ? 'recovered' : resultStatus, | ||
| progressStatus, | ||
| error, | ||
| result, | ||
| recoveryTool: recovered ? toolName : undefined, | ||
| }); | ||
| } catch { | ||
| // Recovery telemetry is best-effort and must not affect tool behavior. | ||
| } | ||
| } | ||
|
|
||
| getToolHandler(toolName: string): ToolHandler | null { | ||
| const registry = this.tools.get(toolName); | ||
| return registry ? registry.handler : null; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| export { | ||
| RecoveryTrajectoryLedger, | ||
| summarizeArgs, | ||
| summarizeResult, | ||
| } from './trajectory-ledger'; | ||
| export type { | ||
| RecoveryProgressStatus, | ||
| RecoveryResultStatus, | ||
| RecoveryTrajectoryLedgerOptions, | ||
| RecoveryTrajectoryNode, | ||
| RecoveryTrajectoryNodeInput, | ||
| } from './trajectory-ledger'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recordRecoveryTrajectoryis called with the rawresultbefore the existingredactSecrets(result)pass runs, so any tool response that echoes a substituted secret can be persisted to.openchrome/recovery/trajectory.jsonlviaobservationSummary. This only occurs when a tool result includes literal secret text (for example after${SECRET:...}substitution), but in that case it creates a disk-level secret leak even though normal MCP responses are redacted.Useful? React with 👍 / 👎.