mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix(chat): hide ignored provider information (#14929)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Task and agent chats show agent progress and problems that need attention. > - Codex also sends account, skill, and unrelated thread notifications. > - The runner correctly ignores that information but reports it as a warning. > - Chat then shows an internal diagnostic as an actionable provider notice. > - This pull request keeps the diagnostic in run logs and removes it from chat. > - Real provider warnings, errors, and agent replies remain visible. ## Linked Issues or Issue Description **What happened?** Chat showed “Received a provider update” and a warning with the text “ignored unrelated provider information”. Its details said “User Actionable: Yes” even though no user action was needed. Saved conversations retained the same noise. **Expected behavior** Keep ignored provider information in the run log. Do not show it as chat activity or a user warning. Preserve real warnings and errors. **Steps to reproduce** 1. Start a conversation with the native Codex runner. 2. Have the provider send an account update, skill change, or unrelated thread notification during the turn. 3. Inspect live chat and reload its saved history. The regression tests also reproduce the old stored notice without a live account. **Paperclip version or commit** Source implementation on master at `e00d10d5d`. The duplicate search found no open PR for this fix. Related prior work: #13109 improved provider-notice presentation. #12367 added Codex thread normalization. This change addresses the internal information that those paths still projected as chat warnings. **Deployment mode** Native Paperclip Runner with the Codex app-server provider. The issue was seen in hosted chat and can be reproduced with local provider fixtures. ## What Changed - Map ignored unrelated Codex information to `harness.diagnostic` in the Rust and TypeScript normalizers. - Retain a bounded allowlist of redacted provider method and thread/turn identifiers. - Use the same Unicode character limit and truncation marker in both normalizers. - Share the text redactor through a pure helper. Keep provider connection code out of the standalone demo's source closure. - Omit that diagnostic and the matching legacy notice from live chat. - Omit the matching legacy notice from saved chat history. - Test diagnostic retention, account-notification integration, live and saved chat, and continued visibility of real warnings, errors, and replies. - Document the local run-log event and historical display behavior. ## Verification - Passed: 68 tests in the two affected UI transcript suites. - Passed: 60 TypeScript tests across provider events, transport behavior, and the standalone demo boundary. - Passed: 13 Rust provider-event tests and the Codex account-notification integration test. - Passed: `pnpm check:token-gates` and Cargo formatting checks. - Passed: full `pnpm build` and `pnpm -r typecheck`. After the review fix, the provider package build, typecheck, and both provider-event suites passed again. - Full local `pnpm test:run` failed: 608 files / 10,904 tests passed, 30 server suites failed, and 104 files / 4,012 tests were skipped. Most failures were embedded PostgreSQL startup errors. Two tests timed out in `heartbeat-comment-wake-batching` and `workspace-git-snapshot-streaming`. PostgreSQL startup also failed in `heartbeat-run-event-sequencing` and `native-finalization-migration`. These server files are unchanged by this PR. Isolated heartbeat reruns were skipped locally. The stable test script stopped after this general-server group, so later groups did not run locally. - The original review thread is resolved. Greptile is 5/5 on current head `683dab7cce57187c57e84c83f5e9da4ad75c9c04`. - All current-head CI gates passed, including the full server/chat/workspace test matrix, Rust and TypeScript runner suites, browser E2E, build, typecheck, and release canary. [CI run](https://github.com/paperclipai/paperclip/actions/runs/37021330663). - Replay the exact old warning in either transcript adapter. It must produce no chat row. A genuine provider warning or error must still produce a row. ## Risks - Low risk. The display filter matches one diagnostic code or the complete legacy warning shape. Other provider notices remain visible. - New ignored-information events use the existing harness-diagnostic event type. They retain diagnostic evidence without original account payloads. - No database migration, API permission, provider execution, or recovery behavior changes. This affects the local run log, not Telemetry or OpenTelemetry exports. ## Model Used OpenAI Codex, GPT-6. The exact backend model ID and context-window size are not exposed in this session. Used reasoning, repository inspection, code editing, tool use, and test execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run the affected tests locally and they pass (the broad local run has PostgreSQL startup errors and timeouts documented above; the full CI matrix passed) - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
479debe9e3
commit
ec3bacc9bd
13 files changed
+278
-34
No files matched your search
@@ -812,6 +812,25 @@ pub fn normalize_codex_notification(method: &str, params: &Value) -> Vec<Normali
|
||||
}),
|
||||
);
|
||||
}
|
||||
"warning"
|
||||
if params.get("classification").and_then(Value::as_str)
|
||||
== Some("unrelated_information") =>
|
||||
{
|
||||
push(
|
||||
&mut events,
|
||||
"harness.diagnostic",
|
||||
EventPriority::P1,
|
||||
json!({
|
||||
"code": "codex_unrelated_information",
|
||||
"classification": "unrelated_information",
|
||||
"providerMethod": params.get("providerMethod").and_then(Value::as_str).map(|value| bounded_text(value, 160)),
|
||||
"expectedThreadId": params.get("expectedThreadId").and_then(Value::as_str).map(|value| bounded_text(value, 256)),
|
||||
"receivedThreadId": params.get("receivedThreadId").and_then(Value::as_str).map(|value| bounded_text(value, 256)),
|
||||
"expectedTurnId": params.get("expectedTurnId").and_then(Value::as_str).map(|value| bounded_text(value, 256)),
|
||||
"receivedTurnId": params.get("receivedTurnId").and_then(Value::as_str).map(|value| bounded_text(value, 256)),
|
||||
}),
|
||||
)
|
||||
}
|
||||
"error" | "warning" | "deprecationNotice" | "configWarning" => push(
|
||||
&mut events,
|
||||
"provider.notice.recorded",
|
||||
@@ -1421,6 +1440,68 @@ fn has_rfc_uri_scheme_prefix(value: &str) -> bool {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn unrelated_information_retains_only_bounded_run_log_diagnostics() {
|
||||
let events = normalize_codex_notification(
|
||||
"warning",
|
||||
&json!({
|
||||
"classification": "unrelated_information",
|
||||
"message": "ignored unrelated provider information",
|
||||
"providerMethod": "account/updated",
|
||||
"expectedThreadId": "root",
|
||||
"receivedThreadId": "x".repeat(300),
|
||||
"expectedTurnId": "turn-1",
|
||||
"receivedTurnId": null,
|
||||
"accessToken": "not-for-the-log",
|
||||
"planType": "private-account-data",
|
||||
}),
|
||||
);
|
||||
assert_eq!(events.len(), 1);
|
||||
assert_eq!(events[0].event_type, "harness.diagnostic");
|
||||
assert_eq!(events[0].priority, EventPriority::P1);
|
||||
assert_eq!(
|
||||
events[0].payload,
|
||||
json!({
|
||||
"code": "codex_unrelated_information",
|
||||
"classification": "unrelated_information",
|
||||
"providerMethod": "account/updated",
|
||||
"expectedThreadId": "root",
|
||||
"receivedThreadId": format!("{}…[truncated]", "x".repeat(244)),
|
||||
"expectedTurnId": "turn-1",
|
||||
"receivedTurnId": null,
|
||||
})
|
||||
);
|
||||
let unicode_events = normalize_codex_notification(
|
||||
"warning",
|
||||
&json!({
|
||||
"classification": "unrelated_information",
|
||||
"expectedThreadId": "token=not-for-the-log",
|
||||
"receivedThreadId": "😀".repeat(300),
|
||||
}),
|
||||
);
|
||||
assert_eq!(
|
||||
unicode_events[0].payload["expectedThreadId"],
|
||||
"token=[REDACTED]"
|
||||
);
|
||||
assert_eq!(
|
||||
unicode_events[0].payload["receivedThreadId"],
|
||||
format!("{}…[truncated]", "😀".repeat(244))
|
||||
);
|
||||
assert_eq!(unicode_events[0].payload["receivedTurnId"], Value::Null);
|
||||
// Authoritative errors must retain their failure meaning.
|
||||
assert_eq!(
|
||||
normalize_codex_notification(
|
||||
"error",
|
||||
&json!({
|
||||
"classification": "unrelated_information",
|
||||
"message": "Provider connection failed",
|
||||
})
|
||||
)[0]
|
||||
.event_type,
|
||||
"provider.notice.recorded"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_codex_notice_text_from_current_and_legacy_payloads() {
|
||||
for method in ["configWarning", "deprecationNotice", "warning"] {
|
||||
|
||||
@@ -617,6 +617,14 @@ fn codex_account_updates_do_not_interrupt_turns_or_publish_account_details() {
|
||||
assert!(!params.to_string().contains("fixture-login"));
|
||||
assert!(params.get("authMode").is_none());
|
||||
assert!(params.get("planType").is_none());
|
||||
let normalized = normalize_codex_notification(&method, ¶ms);
|
||||
assert_eq!(normalized.len(), 1);
|
||||
assert_eq!(normalized[0].event_type, "harness.diagnostic");
|
||||
assert_eq!(normalized[0].payload["code"], "codex_unrelated_information");
|
||||
assert_eq!(
|
||||
normalized[0].payload["providerMethod"],
|
||||
params["providerMethod"]
|
||||
);
|
||||
}
|
||||
if method == "turn/completed" {
|
||||
completed = true;
|
||||
|
||||
@@ -59,6 +59,14 @@ describe("Codex app-server transport limits", () => {
|
||||
.toBe("Basic API foundation");
|
||||
});
|
||||
|
||||
it.each(["PAPERCLIP_API_KEY", "OPENAI_API_KEY", "OPENROUTER_API_KEY", "CUSTOM_API_KEY"])(
|
||||
"redacts the complete %s environment value through the shared text helper",
|
||||
(key) => {
|
||||
expect(redactCodexDiagnostic(`${key}=private;still-private`))
|
||||
.toBe(`${key}=[REDACTED]`);
|
||||
},
|
||||
);
|
||||
|
||||
it("reports restart-safe process-group ownership", async () => {
|
||||
const transport = nodeTransport("process.stdin.resume()", { processGroup: true });
|
||||
const info = transport.processInfo();
|
||||
|
||||
@@ -2,6 +2,9 @@ import type { NativeTurnControlCapabilities } from "../../contracts/types.js";
|
||||
import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
|
||||
import type { HarnessRuntimeRequestResolution } from "../../contracts/harness-driver.js";
|
||||
import { githubCredentialEnvironment } from "../../github-credential-environment.js";
|
||||
import { redactCodexDiagnostic } from "./diagnostic-redaction.js";
|
||||
|
||||
export { redactCodexDiagnostic } from "./diagnostic-redaction.js";
|
||||
|
||||
export interface CodexRpcNotification {
|
||||
method: string;
|
||||
@@ -243,40 +246,6 @@ export function sanitizedEnvironmentKeys(
|
||||
return Object.keys(createSanitizedCodexEnvironment(source)).sort();
|
||||
}
|
||||
|
||||
export function redactCodexDiagnostic(message: string): string {
|
||||
return message
|
||||
.replaceAll(/\u001b\[[0-?]*[ -/]*[@-~]/g, "")
|
||||
.replace(/Bearer\s+[A-Za-z0-9._~+\/-]+/gi, "Bearer [REDACTED]")
|
||||
.replace(/Basic\s+([A-Za-z0-9+/=]+)/gi, (match, encoded: string) => {
|
||||
try {
|
||||
// Only redact an actual RFC 7617 credential. Treating every word after
|
||||
// “Basic” as base64 corrupted ordinary question copy such as
|
||||
// “Basic API” before it entered the Paperclip protocol.
|
||||
const decoded = Buffer.from(encoded, "base64").toString("utf8");
|
||||
return decoded.includes(":") ? "Basic [REDACTED]" : match;
|
||||
} catch {
|
||||
return match;
|
||||
}
|
||||
})
|
||||
.replace(/([a-z][a-z0-9+.-]*:\/\/)[^\s/@:]+:[^\s/@]+@/gi, "$1[REDACTED]@")
|
||||
.replace(
|
||||
/([?&](?:api[_-]?key|token|secret|password)=)[^&#\s]+/gi,
|
||||
"$1[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/(["'](?:api[_-]?key|token|secret|password|authorization)["']\s*:\s*["'])[^"']+/gi,
|
||||
"$1[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/(api[_-]?key|token|secret|password)\s*[=:]\s*[^\s,;]+/gi,
|
||||
"$1=[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/(PAPERCLIP_API_KEY|OPENAI_API_KEY|OPENROUTER_API_KEY)=[^\s]+/g,
|
||||
"$1=[REDACTED]",
|
||||
);
|
||||
}
|
||||
|
||||
function proxyContainsCredentials(value: string): boolean {
|
||||
try {
|
||||
const url = new URL(value);
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
/** Redact diagnostic text without importing the provider process or its credentials. */
|
||||
export function redactCodexDiagnostic(message: string): string {
|
||||
return message
|
||||
.replaceAll(/\u001b\[[0-?]*[ -/]*[@-~]/g, "")
|
||||
.replace(/Bearer\s+[A-Za-z0-9._~+\/-]+/gi, "Bearer [REDACTED]")
|
||||
.replace(/Basic\s+([A-Za-z0-9+/=]+)/gi, (match, encoded: string) => {
|
||||
try {
|
||||
// Only redact an actual RFC 7617 credential. Ordinary prose such as
|
||||
// “Basic API” must remain readable.
|
||||
const decoded = Buffer.from(encoded, "base64").toString("utf8");
|
||||
return decoded.includes(":") ? "Basic [REDACTED]" : match;
|
||||
} catch {
|
||||
return match;
|
||||
}
|
||||
})
|
||||
.replace(/([a-z][a-z0-9+.-]*:\/\/)[^\s/@:]+:[^\s/@]+@/gi, "$1[REDACTED]@")
|
||||
.replace(
|
||||
/([?&](?:api[_-]?key|token|secret|password)=)[^&#\s]+/gi,
|
||||
"$1[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/(["'](?:api[_-]?key|token|secret|password|authorization)["']\s*:\s*["'])[^"']+/gi,
|
||||
"$1[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/(api[_-]?key|token|secret|password)\s*[=:]\s*[^\s,;]+/gi,
|
||||
"$1=[REDACTED]",
|
||||
)
|
||||
.replace(
|
||||
/((?:[A-Z][A-Z0-9]*_)+API_KEY)=[^\s]+/g,
|
||||
"$1=[REDACTED]",
|
||||
);
|
||||
}
|
||||
@@ -34,6 +34,48 @@ function envelope(
|
||||
}
|
||||
|
||||
describe("provider-neutral events", () => {
|
||||
it("keeps unrelated information as bounded run-log evidence without a provider notice", () => {
|
||||
const [event] = canonicalProviderEventsFromCodex("warning", {
|
||||
classification: "unrelated_information",
|
||||
message: "ignored unrelated provider information",
|
||||
providerMethod: "account/updated",
|
||||
expectedThreadId: "root",
|
||||
receivedThreadId: "x".repeat(300),
|
||||
expectedTurnId: "turn-1",
|
||||
receivedTurnId: null,
|
||||
accessToken: "not-for-the-log",
|
||||
planType: "private-account-data",
|
||||
});
|
||||
expect(event).toEqual({
|
||||
eventType: "harness.diagnostic",
|
||||
itemId: "provider-item",
|
||||
payload: {
|
||||
code: "codex_unrelated_information",
|
||||
classification: "unrelated_information",
|
||||
providerMethod: "account/updated",
|
||||
expectedThreadId: "root",
|
||||
receivedThreadId: "x".repeat(244) + "…[truncated]",
|
||||
expectedTurnId: "turn-1",
|
||||
receivedTurnId: null,
|
||||
},
|
||||
});
|
||||
expect(validatePrpEvent(envelope(event)).ok).toBe(true);
|
||||
const [unicodeEvent] = canonicalProviderEventsFromCodex("warning", {
|
||||
classification: "unrelated_information",
|
||||
expectedThreadId: "token=not-for-the-log",
|
||||
receivedThreadId: "😀".repeat(300),
|
||||
});
|
||||
expect(unicodeEvent.payload).toMatchObject({
|
||||
expectedThreadId: "token=[REDACTED]",
|
||||
receivedThreadId: "😀".repeat(244) + "…[truncated]",
|
||||
receivedTurnId: null,
|
||||
});
|
||||
expect(canonicalProviderEventsFromCodex("error", {
|
||||
classification: "unrelated_information",
|
||||
message: "Provider connection failed",
|
||||
})[0].eventType).toBe("provider.notice.recorded");
|
||||
});
|
||||
|
||||
it("preserves Codex notice summaries with legacy and empty-message fallbacks", () => {
|
||||
for (const method of ["configWarning", "deprecationNotice", "warning"]) {
|
||||
for (const [params, expected] of [
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import { redactCodexDiagnostic } from "./drivers/codex/diagnostic-redaction.js";
|
||||
|
||||
/**
|
||||
* Structural subset of an ACP runtime event consumed by the canonical event
|
||||
@@ -593,6 +594,29 @@ export function canonicalProviderEventsFromCodex(
|
||||
const type = text(item.type);
|
||||
const itemId = safeId(text(item.id, text(params.itemId)), "provider-item");
|
||||
const completed = method === "item/completed";
|
||||
if (method === "warning" && params.classification === "unrelated_information") {
|
||||
const boundedField = (key: string, limit: number) => {
|
||||
if (typeof params[key] !== "string") return null;
|
||||
const characters = [...redactCodexDiagnostic(params[key])];
|
||||
const marker = "…[truncated]";
|
||||
return characters.length <= limit
|
||||
? characters.join("")
|
||||
: characters.slice(0, limit - marker.length).join("") + marker;
|
||||
};
|
||||
return [{
|
||||
eventType: "harness.diagnostic",
|
||||
itemId,
|
||||
payload: {
|
||||
code: "codex_unrelated_information",
|
||||
classification: "unrelated_information",
|
||||
providerMethod: boundedField("providerMethod", 160),
|
||||
expectedThreadId: boundedField("expectedThreadId", 256),
|
||||
receivedThreadId: boundedField("receivedThreadId", 256),
|
||||
expectedTurnId: boundedField("expectedTurnId", 256),
|
||||
receivedTurnId: boundedField("receivedTurnId", 256),
|
||||
},
|
||||
}];
|
||||
}
|
||||
if (method === "turn/plan/updated") {
|
||||
const turnPlanId = safeId(text(params.turnId), "turn-plan");
|
||||
return [
|
||||
|
||||
Reference in new issue
Block a user