Commit Graph
7 Commits
Author SHA1 Message Date
DottaandClaude Opus 5.5 46f32ed33f fix(sentry): stop treating short host settings as credentials in run-failure redaction (#20013)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - When an agent run fails, Paperclip sends a redacted failure report
to Sentry so the team can fix the cause
> - Redaction treats the server's own environment values as possible
secrets and removes them wherever they appear in the report
> - Since #19461, every value under an environment name the allowlist
does not know counts as a secret, even very short ones such as `vi`,
`RUN`, `off` or `1`
> - On a host with such settings, the report text is damaged: `service`
becomes `ser***REDACTED***ce`, and a `1` anywhere can vanish. One server
test failed on every such host, while CI (which has no such values)
stayed green
> - This pull request keeps the fail-closed rule for an agent's
configured environment, and requires a plausible credential in the
server's own environment
> - The benefit is that Sentry reports stay readable on every host, and
real credentials stay private

## Linked Issues or Issue Description

Refs #19461 (introduced the catch-all), #14585 (added the failing test).

**What happened?**
`heartbeat-process-recovery.test.ts` › "redacts opaque environment-bound
credentials from Sentry diagnostics: result" failed locally on `master`:
`expected 'ser***REDACTED***ce' to be 'service'`. Instrumentation showed
that the server's own `process.env` contributed these redaction values:
`["false","67591","true","0x0","RUN","off","vi","64","1","0","8"]`. They
came from a desktop agent session's environment.

**Expected behavior**
Short host settings are not credentials. Declared, secret-named and
credential-shaped values stay private. An agent's configured environment
keeps its fail-closed rule.

**Steps to reproduce**
Run that test on a machine whose environment contains
`SOME_UNKNOWN_NAME=vi`.

## What Changed

- `packages/adapter-utils/src/command-redaction.ts`:
`collectDiagnosticEnvironmentSecrets` gets an option, `{ hostProcess:
true }`. With it, an undeclared value under an unrecognized name is
collected only in these cases:
- it has at least 8 characters and is not a bare number, hex number or
boolean-like word; or
- its name has a credential segment: `TOKEN`, `SECRET`, `PASSWORD`,
`BEARER` or `PRIVATE`, or a final `KEY` or `KEY_ID`.

  Without the option, behavior is unchanged.
- `server/src/services/run-failure-report.ts`: the report collects the
server's own `process.env` with `hostProcess: true`. The agent's
configured environment (`heartbeat.ts`) and the provider environment
(`terminal-session-failure.ts`) keep the default.

## Verification

- `command-redaction.test.ts` (42 pass). The new test fails on `master`
and passes here. It checks three things: the host's short values are not
collected; a short value under `AWS_ACCESS_KEY_ID` and a long opaque
value stay private; and an agent binding `CUSTOM_BINDING: "1"` stays
private.
- `run-failure-diagnostics.test.ts` (135 pass, unchanged). These tests
pin the agent-environment fail-closed rule.
- `heartbeat-process-recovery.test.ts` › "redacts opaque
environment-bound credentials…": both modes pass here. The `result` mode
failed on unchanged `master` on this host.
- The adapter-utils and server typechecks pass.

## Risks

- The server's own environment now leaves out a non-credential-named
value that is shorter than 8 characters or is a bare number or boolean,
for example `MY_PIN=1234`. Values under credential names, declared
secrets and credential-shaped values are still redacted.
- No Telemetry path changes; this is the Sentry run-failure report.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.

## 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 tests locally and they pass
- [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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-08 05:50:30 -06:00
DottaandPaperclip 6ec4180ea1 fix: preserve ordinary diagnostic text during environment secret redaction (#19461)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Run diagnostics help operators find the cause of a failed agent run.
> - The failure reporter treated unknown environment settings as
credentials.
> - It replaced every use of those values in diagnostic text.
> - A flag set to `1`, `2`, or `safe` removed parts of dates, model
names, versions, and normal words.
> - This pull request preserves explicit ordinary settings and keeps
unknown values private.
> - Operators can read the failure details while real credentials stay
masked.

## Linked Issues or Issue Description

**What happened?**

Run-failure contexts contained redaction markers inside timestamps,
model names, driver versions, and the word "safely". Provider failure
diagnostics used the same broad environment-value rule.

**Expected behavior**

Ordinary flags, counts, paths, regions, and mode labels leave diagnostic
text readable. API keys, URL passwords, and declared secret bindings
remain masked.

**Steps to reproduce**

1. Set ordinary environment settings to `1`, `2`, and `safe`.
2. Report a failed native run with a timestamp, `gpt-6.1-sol`,
`phase6-v2`, and "safely" in its details.
3. Inspect the run failure report. Before this fix, unrelated fragments
are replaced.

Related work: #13668 addresses a separate model-ID false positive.
#11311 and #15524 address credential capture at other boundaries. This
change keeps the existing command scanner and registered-secret
replacement.

## What Changed

- Add a shared environment selector in adapter-utils. Keep unknown
values private. Exempt explicit ordinary process settings, typed flags
and counts, mode labels, and key selectors.
- Use the same selector for run-failure reports and inherited provider
failure environments. Keep one policy for both consumers.
- Keep configured provider bindings private because that API does not
carry separate secret-reference metadata.
- Preserve short declared secrets, unknown opaque values, encoded
credentials, and inherited Bedrock tokens. Add regressions for ordinary
`SORT_KEY` values and both Bedrock consumers.
- Add screenshot-shaped regressions, real database reporter coverage,
and credential tests.
- Document the policy in `doc/observability.md`. The optional monitoring
gate stays in place.

## Verification

Linux, Node v25.9.0, pnpm 9.15.4. The published tree matches the tested
local tree `cb236f072cca195e4c2cfb4034c7cc06ea7adbc5`.

- Before the fix, four diagnostic regressions failed. The output
reproduced the damaged timestamp, model, version, and word.
- Review regressions: six tests failed on the first revision. They
proved the missed Bedrock token, opaque inherited-value exposure, and
key-selector false positive. All pass after the correction.
- 555 focused tests pass on the final source tree.
- `cd server && pnpm exec vitest run
src/services/__tests__/run-failure-diagnostics.test.ts
src/__tests__/redaction.test.ts src/__tests__/redact-sensitive.test.ts
src/__tests__/run-secret-redaction.test.ts`: 206 passed.
- `cd server && pnpm exec vitest run
src/services/__tests__/run-failure-report.test.ts`: 289 passed, with
real embedded PostgreSQL.
- `pnpm exec vitest run
packages/adapter-utils/src/command-redaction.test.ts
packages/adapter-utils/src/acpx-engine/terminal-session-failure.test.ts`:
60 passed.
- `pnpm --filter @paperclipai/adapter-utils typecheck`: passed.
- `git diff --check`: passed.
- The first reporter attempt could not load the unbuilt plugin SDK. It
passed after `pnpm --filter @paperclipai/plugin-sdk ensure-build-deps`.
- The filtered install omitted the CLI tsx loader, so `pnpm verify
--explain` could not start. The equivalent verifier entry points ran
with the installed server loader and returned exit 2: broader CI
coverage is required for the observability document.
- Full local build/typecheck and browser suites were not duplicated.
[Final-revision
CI](https://github.com/paperclipai/paperclip/actions/runs/42231412547)
passed on `013960a4c6fa6279e1f28b0f6af5f036d7cad5a7`, attempt 1. This
includes typecheck, build, general/server/chat tests, serialized suites,
Runner checks, eight E2E shards, and the canary dry run.
- [Greptile
review](https://github.com/paperclipai/paperclip/pull/19461#issuecomment-6686592783)
scored this head 5/5 with zero new findings. All four inline threads are
resolved. The security rescan and Snyk status passed. The PR has no
merge conflicts.

## Risks

- Ordinary-setting exemptions need a known name or a name/value type
rule. Unknown opaque values remain private. Declared secret bindings and
recognizable credentials override these exemptions.
- A real short secret still masks matching fragments. Removing that
protection would expose the secret.
- Existing stored redaction markers cannot restore their original text.
This fix applies when new diagnostics are produced.
- No migration, dependency, workflow, authorization, or default
monitoring changes.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. This change is a focused bug
fix.

## Model Used

OpenAI GPT-6, through Codex, with shell execution and file tools. The
host does not expose the exact runtime variant or context window size.

## 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 tests locally and they pass
- [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>
2026-11-06 09:01:55 -06:00
AlexFrontegg 8424a51018 fix(redaction): scan long letter/digit runs in linear time (#16999)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Paperclip removes secrets from run logs, tool results and
diagnostics with `redactSensitiveText` before it stores or shows them.
> - The JSON field-name patterns in that function start with an
unanchored `[A-Za-z0-9_-]*`. Each start position inside a long run of
letters and digits scans to the end of the run, so the cost grows with
the square of the run length.
> - Base64 of text is one long run of letters and digits. A 40 KB base64
blob of source code in a shell command takes about 5 s to redact, and a
large Gmail RAW tool result does not finish.
> - The redaction runs on the main event loop, so one such chunk blocks
every request and every run on the instance.
> - This pull request makes these scans start only at the start of a
run. The scans become linear, and the redacted output does not change.
> - The benefit is that large base64 payloads no longer freeze the
server, and every secret that was redacted before is still redacted.

## Linked Issues or Issue Description

Fixes #15393. The tool content guard runs `redactSensitiveText` on tool
results. A Gmail RAW body is base64url of a MIME message, which is one
long letter/digit run. `validateToolContent` on a Gmail-shaped result
with a 68 KB `raw` field takes 59 s on master and 19 ms with this
change; a 5.4 MB `raw` field takes 0.65 s.

Thanks @ts-beat for the clear report in #15393.

Refs #12309 (false positives in the same patterns; this pull request
does not change which text matches).

I searched issues and pull requests for "redaction",
"redactSensitiveText", "quadratic", "backtracking", "hangs the server"
and "100% CPU". #15393 reports the hang. No open pull request fixes it.

**Subsystem affected:** secret redaction (`server/src/redaction.ts`,
`packages/adapter-utils/src/command-redaction.ts`), used by run logs,
tool content guards and run diagnostics.

**Problem:** `compactRunLogChunk` redacts each run log chunk before it
truncates the chunk. A shell command such as `printf '%s' '<base64>' |
base64 -d > file.ts` with base64 of source text makes one long
letter/digit run. Base64 of ASCII text contains `+` or `/` only where
every third byte is `>`, `?`, `~` or DEL, so random base64 does not show
the problem. One `redactSensitiveText` call on such a tool-call line
takes (local measurement):

| Blob | Before | After |
| --- | --- | --- |
| 4 KB | 66 ms | 1.1 ms |
| 8 KB | 272 ms | 0.5 ms |
| 16 KB | 1.1 s | 1.0 ms |
| 40 KB | 5.4 s | 2.5 ms |
| 1 MB | not measured | 36 ms |

The `\b`-anchored command patterns (`KEY=value`, JWT, `Authorization`)
and the CLI option pattern have the same quadratic cost on
dash-separated runs, for example base64url data.

**Proposed solution:** start the unanchored field-name scans only where
no name character precedes them (`(?<![A-Za-z0-9_-])`). A prefixed name
such as `myApiKey`, `x_auth_token` or `GITHUB_TOKEN` still matches,
because the scan from the start of the run absorbs the prefix.

**Alternatives considered:**

- Truncate or cap the input before redaction. Rejected: a cut can split
a secret and leave part of it unredacted, and the scan stays quadratic
below the cap.
- Run the scan in a worker thread with a deadline, as #15393 suggests.
That limits the damage, but the CPU cost stays, and a timeout must then
drop or block the content. It is still useful as an additional defense.

## What Changed

- `server/src/redaction.ts`: `JSON_SECRET_FIELD_TEXT_RE` and
`ESCAPED_JSON_SECRET_FIELD_TEXT_RE` start a field name only where no
name character precedes it. The lookbehind comes after the optional
opening quote, so quoted keys behave as before. The `Authorization` word
scan uses the same start instead of `\b`.
- `packages/adapter-utils/src/command-redaction.ts`: the `KEY=value`
pattern uses the same start. The JWT pattern starts only at the first
word character of a run. A CLI option name can no longer span `--`, so
each `--` starts its own short scan.
- Redaction still runs before truncation. `compactRunLogChunk` does not
change.

Why the output does not change: when an old pattern could match from the
middle of a run, it could also match from the start of that run, because
the `[A-Za-z0-9_-]*` prefix absorbs the extra characters. The engine
tries the start of the run first. So the old patterns never matched from
the middle of a run, and the new start condition only skips positions
that cannot produce a match. The old JWT pattern also always matched
from the first word character of a run. A CLI option name always ends at
the end of its run, so the match from the last `--` in the run ends at
the same place.

## Screenshots

No UI change.

## Verification

- New tests:
- prefixed secret field names in plain, single-quoted and escaped JSON
are still redacted, also when the name is glued to the end of a long
run;
- a 48 KB base64-of-source tool call through `compactRunLogChunk` stays
intact, the secret in the same command is redacted, and the call
finishes within 1 s;
- `redactSensitiveText` and `redactCommandText` handle 48-120 KB
letter/digit, dash-separated, `--`-separated and base64url runs within 1
s, and redact the secret that follows each run;
- `x--token`, `--x--token`, `a-b--api-key=`, `prefix-MY_API_KEY=` and
`--<jwt>` are still redacted, and `my-token is here` is not changed.
- Without the change, the timing tests fail: 9.3 s, 9.8 s, 32 s and 60 s
against the 1 s budget. The correctness tests pass with and without the
change.
- Mutation check: I reverted each of the six pattern changes one at a
time. Each revert makes at least one test fail.
- Differential fuzzing: 1.1 million random inputs built from secret
names, quotes, escaped quotes, dashes, dots, colons and JWT parts gave
the same output from the old and new `redactSensitiveText`,
`redactCommandText` and `redactDiagnosticText`.
- `cd server && npx vitest run <files>`: all 33 server test files that
use the redaction helpers, with embedded Postgres, 1730 passed.
- `npx vitest run --project @paperclipai/adapter-utils
packages/adapter-utils/src/command-redaction.test.ts`: 25 passed.
- `npx vitest run --project @paperclipai/adapter-utils`, complete suite:
80 of 82 files passed in the parallel run. The 2 other files
(`github-launcher-environment`, `sandbox-managed-runtime`) had 5 s
timeouts in git-based tests and passed on rerun.

## Risks

- Lookbehind assertions need ES2018. `redactCommandText` also runs in
the UI bundle. Node and current browsers support lookbehind, and the
server (`server/src/log-redaction.ts`) and the UI
(`ui/src/components/task-chat/task-chat-attachments.ts`) already use it.
- A change to a redaction pattern could leak a secret. The reasoning
above, the unchanged existing tests and the differential fuzzing show
the same output as before.
- Not changed here, found while checking: `redactDiagnosticText`
backtracks exponentially on an escaped JSON secret value made of many
escaped backslashes with no closing quote, and `redactSensitiveText`
redacts a value only up to its first quote of either kind
(`{"apiKey":"abc\"tail"}` keeps `tail"`). Both behave the same before
and after this pull request.

## Model Used

- Claude Opus 5.5 (`claude-opus-5-5`) by Anthropic, in Claude Code, with
tool use (shell, code search, and test runs) and extended thinking. A
human directed the design choices.

## 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 tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] 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
2026-11-02 10:49:08 -06:00
DottaandPaperclip 3f7b00a8d0 fix: preserve public identifiers in feedback redaction (#16168)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - People rate agent comments and documents to capture feedback.
> - Feedback bundles remove credentials and personal information before
export.
> - The feedback scanner treats ordinary dotted text as JWTs and
authentication terms as bearer tokens.
> - This pull request checks encoded JWT headers and preserves known
authentication terms.
> - The benefit is readable feedback context with credential masking
kept in place.

## Linked Issues or Issue Description

Refs #12309 for the related dotted-identifier false-positive report.
This PR fixes the separate feedback scanner. It does not close every
case in that report.

Related PR: #14318 adds fine-grained GitHub token coverage. It addresses
a separate credential form and is not duplicated here.

**What happened?**

Feedback exports replace `api.openai.com`, dotted filenames, and public
schema IDs with a JWT placeholder. They also replace the words after
`bearer` in phrases such as “bearer tokens.” Five-part encrypted JWTs
are only partly masked.

**Expected behavior**

Keep public identifiers and authentication terms readable. Mask complete
credential tokens. Count only text replacements in the redaction
summary.

**Steps to reproduce**

1. Pass `api.openai.com` or `Use bearer tokens.` to
`sanitizeFeedbackText`.
2. Check that the original text is returned and the redaction counts
stay empty.
3. Pass a signed JWT or an encrypted JWT to the same function.
4. Check that the full token is replaced and one JWT replacement is
counted.

**Paperclip version or commit**

Reproduced in the feedback scanner on master at
`5e8e992df2b3d71f82091b7bbf29fd482854d9f5`.

**Deployment mode**

Built from source. The defect is in the shared feedback export service
and does not depend on deployment mode.

## What Changed

- Use the existing JWT header predicate in the feedback scanner.
- Match complete three-part and five-part tokens, including
direct-encryption tokens with an empty key segment. Accept short token
segments in the predicate.
- Preserve the closed set of authentication terms already used by the
general scanner, including terms inside closing prose delimiters.
- Count only replacements that change the text.
- Add tests for readable context, credential masking, nested values, and
redaction counts.
- Document feedback text redaction.

## Verification

- Focused redaction and feedback service tests: 89 tests pass across
five suites.
- `pnpm --filter @paperclipai/adapter-utils typecheck` passes. Full
local typecheck, tests, and build were attempted. The kernel killed
compiler and test processes at the sandbox memory limit of 4 GB. Serial
retries and a capped Node heap still could not complete the server
compiler. CI runs the full repository checks.
- `git diff --check` passes.
- Latest commit `0eb8d3fb4257bb79e67fab615a8299ef69230ad8`: **55 checks
passed**, including full typecheck, tests, build, and E2E; two optional
Storybook checks were skipped. [CI
evidence](https://github.com/paperclipai/paperclip/actions/runs/38407932875).
Both Greptile findings have regression tests and fixes. [Greptile gives
this commit
5/5](https://github.com/paperclipai/paperclip/pull/16168#issuecomment-6146917423),
with both review threads resolved.

## Risks

- This changes the feedback export matching rules. Public dotted text no
longer triggers credential masking by shape alone.
- Explicit credential assignments, provider keys, structured secret
fields, email addresses, and phone numbers remain masked. Tests cover
these cases.
- Existing feedback snapshots are not rewritten. No schema change or
migration is needed.
- The encoded-header check remains a heuristic. It is not a full JWT
validator.

## Model Used

OpenAI Codex, GPT-6, with reasoning, tool use, and code execution. The
runtime identifies the model as GPT-6. It does not expose the exact
provider deployment ID or context window size. The earlier patch and
this PR preparation were AI-assisted.

## 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 tests locally and they pass
- [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>
2026-10-13 09:10:37 -05:00
DottaandPaperclip 9786f6df56 fix(runner): preserve credential content in document saves (#14937)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The Runner sends authorized tool calls to the control plane.
> - Agents use these calls to save plans and instruction files.
> - The Runner used diagnostic secret detection to reject execution
arguments.
> - Ordinary credential-related prose could reject a document save
before persistence.
> - This pull request forwards the original arguments and leaves
credential policy to the provider harness.
> - The benefit is reliable saves with useful diagnostic records.

## Linked Issues or Issue Description

Related foundation: Refs #12415 and #14430. No duplicate save-policy fix
was found.

**What happened?**

A `write_document` call failed before the server saved its plan. The
Runner reported `semantic tool input contains credential material;
refusing to execute altered arguments`. The detector also masked
ordinary phrases such as `secret manager` and `credential handling` in
diagnostics. Both TypeScript dispatchers had equivalent execution gates.
One dispatcher also rewrote structured approval and question payloads
before execution.

**Expected behavior**

Paperclip forwards authorized arguments unchanged. The provider harness
decides credential-content policy. Log and audit redaction does not
reject or rewrite save input.

**Steps to reproduce**

1. Send an authorized `write_document` call with a plan that discusses
credential handling.
2. Include an intentional credential value in the body to exercise
harness-owned policy.
3. The old Runner rejects the call. With this change, the document
service stores the exact body.
4. Diagnostic records still mask explicit credential values. Qualified
credential fields, short bearer values, opaque diagnostic pairs, and
valid encoded JSON token headers have regression coverage.

**Paperclip version or commit**

Reproduced at `c46e41e81c03cd3c8b64cf993615b604d7fe8c62`. The branch is
based on current `master`.

**Deployment mode**

Server deployment with the native Paperclip Runner. Local regression
tests use the real document service and an embedded test database.

## What Changed

- Remove credential-content vetoes from Rust admission and both
TypeScript semantic dispatchers.
- Preserve original structured approval and question arguments during
execution.
- Keep transport bounds, schema checks, authorization, idempotency, and
audit masking.
- Require explicit credential syntax or recognized formats for
diagnostic masking. Preserve ordinary prose, metadata, and dotted
identifiers.
- Test exact document persistence, replay, nested argument identities,
and masked audit copies.
- Remove obsolete retry guidance and document harness-owned credential
policy.

## Verification

- `cargo test --manifest-path
packages/paperclip-runner/runner/Cargo.toml --locked -p
paperclip-runner-core --lib --test acpx_event_payload --test
acpx_provider_state --test acpx_provider_turns`: 355 tests passed.
- Focused server and adapter tests: 189 tests passed after rebase. These
include the real document save and the complete tool-gateway suite.
- Semantic dispatcher and conformance tests: 34 tests passed.
- Diagnostic redaction and MCP tests: 46 tests passed, including all six
review examples.
- `pnpm -r typecheck` and `pnpm build` passed on the repaired branch.
- The broad local root suite was interrupted after database fixture
setup failures. The focused database suites passed. CI runs the complete
configured test lanes.

## Risks

- Authorized tool arguments can intentionally contain credentials. The
harness must enforce its content policy.
- Diagnostic detection is narrower. Explicit assignments, credential
fields, and recognized credential formats remain masked.
- The change does not add a database migration or change company
authorization.

## Model Used

- OpenAI GPT-6 through Codex. The session exposes the GPT-6 model
family; its exact runtime model identifier and context window size are
not exposed. Used reasoning, tool use, and code 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 tests locally and they pass
- [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>
2026-10-02 14:19:41 -05:00
Dotta 0f94521017 fix(runner): restore local session and task integrity (#12721)
## Thinking Path

> - Paperclip is the control plane for agents that perform work.
> - Paperclip Runner connects durable provider sessions to individual
task runs through PRP.
> - Provider continuity and per-run authority are different lifetimes.
> - The existing implementation mixed those lifetimes and lost event
metadata between provider frames, runnerd, persistence, API
sanitization, and the task thread.
> - That caused failed continuation, missing progress and Plans,
duplicate replies, hidden failures, and unsafe recovery.
> - This repair gives every heartbeat fresh authority, preserves
qualified provider-session continuity, and restores one lossless
presentation path without changing direct adapters.

## Linked Issues or Issue Description

**What happened?**

A second native heartbeat could reuse tickets, leases, command receipts,
sequence state, and run identity from the first heartbeat. Provider
phase and item identity could be lost before the UI read them. Redaction
could corrupt protocol discriminators while still missing malformed
credential tails. The task thread could fold progress into the final
response, hide failures, or show more than one final answer. Native
Codex also exposed approval modes that do not yet have a durable
approval bridge.

**Expected behavior**

Each heartbeat uses a new PRP authority epoch. Codex and OpenCode
preserve exact qualified provider sessions; ACPX emits an explicit
continuity event when its qualified process-replacement policy is used.
Every accepted provider event is presented, classified as internal, or
surfaced as unsupported. The task page shows chronological progress,
reasoning summaries, activity, Plans, interactions, terminal failures,
and exactly one final reply. Direct adapters retain their existing path.

**Steps to reproduce**

1. Enable the unified experimental Paperclip Runner setting.
2. Create a local native Codex, OpenCode, ACPX Claude, or ACPX Codex
agent.
3. Run response, Plan, structured-question/resume, restart,
cancellation, and failure scenarios.
4. Reload the task while active, waiting, failed, and settled.
5. On the old implementation, observe stale run authority, missing
classifications, incomplete output, or duplicated/folded replies.

**Paperclip version or commit**

The repair is based directly on `master` at
`87d05e194b643810d16d20612115acd01d735d43`.

**Deployment mode**

Local development with the embedded database.

Related work: Refs #12616, #12646, #12666, #12685, and #12700.

## What Changed

- Rotates PRP control-plane, outbox, ticket, lease, command, receipt,
and sequence authority for each heartbeat while carrying forward only a
validated provider-session identity.
- Reads `control-plane-state.json`, validates both durable schemas and
lifecycle values, resumes coherent current runs, archives qualified
settled authority, and quarantines malformed or mismatched scoped state
without moving ambiguous live legacy state.
- Preserves Codex provider phase and stable item identities so
commentary remains progress and only `final_answer` becomes final.
- Adds raw OpenCode HTTP/SSE boundary coverage and canonical reasoning
lifecycle mapping.
- Makes ACPX normalization lossless for visible reasoning, tool
lifecycle metadata, stable bounded identities, Plan revisions,
structured requests, failures, and qualified process replacement. Only
the compatible terminal assistant message is promoted as final.
- Applies schema-aware redaction before generic JWT-shaped detection and
scans every diagnostic string leaf. Malformed raw/escaped quoted
credential tails are redacted in both server and durable Rust state.
- Restores snapshot-style chronological task presentation, expandable
tool activity, inline Plan cards, visible waiting/resume/cancel/failure
states, and exactly one final answer.
- Makes `never` the only qualified native Codex permission mode and
rejects unsupported persisted native modes with remediation. OpenCode
and ACPX policies remain intact.
- Keeps the unified experimental Runner setting as the only enablement
flag. Onboarding and direct Codex, Claude, and OpenCode stay on their
legacy execution/finalization paths.
- Adds cross-language goldens, authority/recovery/fault coverage, exact
response/count assertions, and native plus legacy acceptance scenarios.

## Verification

- Pull-request GitHub Actions run Rust formatting/tests, TypeScript
checks, server/UI tests, builds, protocol drift checks, browser E2E, and
security scans.
- A separate workflow-only validation ref is pinned directly on this PR
head and runs the 35-cell paid local matrix: three core scenarios plus
structured-question resume and restart/resume for native Codex, native
OpenCode, ACPX Claude, ACPX Codex, and direct Codex/Claude/OpenCode.
Run: https://github.com/paperclipai/paperclip/actions/runs/33682434315
- Acceptance requires exact single visible replies, monotonic sequences,
matching envelope discriminators, one semantic terminal, one run
terminal, no unresolved interaction, no duplicate mutation, no secret
leakage, provider continuity, and zero native rows for direct adapters.
- Per maintainer direction, tests are running in GitHub Actions rather
than on the slower local host. Only formatters and static diff checks
were run locally.

## Risks

- Recovery from old or partial filesystem state is sensitive. The repair
fails closed, preserves active or unverifiable authority, and
quarantines only state whose scoped ownership is safe to move.
- Provider event formats can change. Closed validators and boundary
goldens turn new or malformed events into visible diagnostics instead of
silent drops.
- Shared task presentation could affect direct adapters. Runtime-fact
gating plus the direct-adapter matrix protect the existing path.
- Managed and remote providers are not qualified here. Shared code
continues to compile and fail safely, but live qualification is
deferred.

> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.

## Model Used

OpenAI Codex based on GPT-5. The exact deployed snapshot and
context-window size are not exposed to this task. It used agentic
reasoning, repository inspection, code editing, Git, parallel subagents,
and GitHub Actions.

## 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 and contains no internal
Paperclip ticket id
- [ ] I have run tests locally and they pass (intentionally deferred to
GitHub Actions)
- [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 risks above
- [ ] All Paperclip CI gates are green
- [ ] The paid local-provider matrix is green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
2026-09-02 16:11:26 -05:00
Nicky LeachandPaperclip c1c46f1e4e feat: Claude login on the new-agent page before agent creation (#11347)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The Claude local adapter supports subscription login through a
sandbox
> - The new-agent page must show login before the user creates an agent
> - Test results must not expose raw sandbox diagnostics or secret
values
> - This pull request adds the login UI to both Test lanes and closes
the diagnostic boundary
> - The branch also adds durable cleanup recovery for failed sandbox
teardown
> - Reusable sandboxes must retain both their recorded teardown
configuration and a valid lifecycle path until destruction succeeds
> - The benefit is a usable login flow with fixed public checks,
redacted server logs, and recoverable sandbox cleanup

## Linked Issues or Issue Description

Related public work:
[#9488](https://github.com/paperclipai/paperclip/pull/9488) adds
first-class recognition for `CLAUDE_CODE_OAUTH_TOKEN` in headless and
remote runs. Related public issue:
[#2681](https://github.com/paperclipai/paperclip/issues/2681) requests
Claude Code subscription support. This pull request adds the login
transport and new-agent UI flow that those changes do not provide.

**Subsystem affected:** Claude local adapter, server login probes,
sandbox provider setup, cleanup recovery, and the new-agent UI.

**Problem or motivation:** The Test lanes did not show the sandbox login
panel in all supported cases. Test results also exposed raw probe
diagnostics, and JSON escapes could end secret redaction early.

**Proposed solution:** Surface the login capability through the bundled
provider manifest. Prepare the same probe runtime in the ACP lane. Send
diagnostics only to redacted server logs. Keep Test checks on fixed
public messages. Normalize login URL hints to allowlisted HTTPS Claude
and Anthropic hosts. Consume JSON escapes during redaction. Preserve
failed sandbox cleanup state across retries and restarts, and prevent
deletion from severing the lifecycle context of a live reusable sandbox.

**Alternatives considered:** Keep raw diagnostics in Test checks or
trust login URL text from the sandbox. Both choices increase information
exposure. Keep separate probe behavior in the ACP lane. That choice
would leave the two Test lanes inconsistent.

## What Changed

- Surface the sandbox login panel on both Test lanes.
- Reconcile the bundled Daytona plugin manifest so
`supportsSetupTokenLogin` reaches the UI capability gate.
- Prepare the ACP Test lane with the same probe runtime as the CLI Test
lane.
- Add the `claude_acp_login_probe_unavailable` warning when the ACP
probe cannot run.
- Send raw sandbox diagnostics only to redacted server logs.
- Keep Test checks on fixed public messages in the ACP, managed-config,
and CLI paths.
- Normalize login URL hints to allowlisted HTTPS Claude and Anthropic
hosts.
- Redact JSON and escaped-JSON secret values, including escaped quotes
and backslashes.
- Preserve orphan cleanup records across provider failures, restarts,
and unavailable plugins.
- Atomically block environment deletion while a live reusable sandbox
lease still depends on it.
- Verify pending cleanup destroys plugin sandboxes with the provider
configuration recorded on the lease, even after the current environment
configuration changes.

## Verification

- Head under review: `506b7fa2d83c36bfa5fd722ee9d95b0c7431c241`.
- Focused environment route/service/runtime coverage passes: 196 tests
across 3 files.
- `pnpm -r typecheck` passes.
- `pnpm build` passes.
- The full Vitest run completed with 4,754 passing and 28 failing tests.
All 23 source-test failures reproduce unchanged on parent head
`58cfe61a33191ce03d965d65085d26064b4888ba`; the other 5 are duplicate
executions from stale `server/dist` output. The failures are unrelated
macOS path/listener and scheduler-fixture failures, so there is no new
bad commit for bisect to localize.
- All required CI checks pass for the current head, including build,
typecheck/release registry, all server and workspace shards, serialized
server suites, canary, and e2e.
- A fresh Greptile review for `506b7fa2d83c36bfa5fd722ee9d95b0c7431c241`
reports 5/5, “safe to merge,” with no blocking failure remaining.

## Risks

- A probe or redaction change could hide useful server diagnostics.
- An allowlist change could reject a valid Claude login URL.
- Cleanup recovery changes could affect provider teardown ordering.
- An environment with a live reusable sandbox can no longer be deleted
until the owning issue or execution workspace completes teardown.
- The implementation keeps public Test messages fixed and sends detail
to redacted server logs.

## Model Used

OpenAI GPT-5 via Codex — exact model ID: GPT-5; tool use and code
execution enabled; extended reasoning enabled. The implementation author
used AI-assisted development.

## 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 tests locally and documented the result
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation or confirmed no separate
documentation change is needed
- [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>
2026-08-17 13:42:51 -07:00