mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 09:41:04 +01:00
936215693c
## Thinking Path > - Paperclip runs Codex as a sandboxed agent with its own CODEX_HOME; at the start of each run Paperclip merges the sandbox's auth.json with the host's so the agent can authenticate to the Codex API > - The merge uses `codex-auth-merge-decision.cjs`, a small CJS script that reads two `auth.json` snapshots and exits `10` (use source copy) or `20` (keep destination copy) > - Until now the script only handled the **inbound** direction (host → sandbox) with hard-coded sandbox/host semantics > - Sandbox-to-host copy-back (**outbound**) requires the same freshness guard but with swapped roles — and exposing both directions via a flag requires hard-coding knowledge about which role is "sandbox" and which is "host" into the predicate itself > - A cleaner design: the predicate is **direction-agnostic** — it accepts a `source` path and a `destination` path; the caller decides direction by which argument it passes first. The script answers one question: "should destination be replaced by source?" Exit `10` = yes (use source); exit `20` = no (keep destination) > - This PR refactors `codex-auth-merge-decision.cjs` to this source/destination model, removes the `--direction` flag, and extends coverage with a 15-row source/destination predicate matrix plus updated extract integration tests ## Linked Issues or Issue Description No public GitHub issue exists. The underlying problem: **Context** — Paperclip merges `auth.json` files between sandbox and host to keep Codex credentials in sync across agent runs. The existing decision predicate (`codex-auth-merge-decision.cjs`) handled the inbound direction (host→sandbox) with hard-coded sandbox/host naming. A copy-back (outbound) direction was needed, but rather than add a `--direction` flag that hard-codes role knowledge into the predicate, the design was simplified: the predicate is now **direction-agnostic**, taking `source` and `destination` paths and answering "replace destination with source?" The freshness guard is unified: exit `10` only when source and destination share the same `account_id`, same subscription-kind, both carry a parseable `last_refresh`, and `source.last_refresh > destination.last_refresh` (strict). Ties and null freshness keep the destination copy so a spent single-use refresh token (openai/codex#15502) is never written back over a valid one. Related PRs: - Refs #9785 — Phase 2: relocated the decision predicate from `adapter-utils` into `codex-local`; this PR builds on that merged change - Refs #9621 — community PR targeting Codex sandbox auth sync-back via a different approach; operates on different files (runtime layer vs. decision script), non-conflicting ## What Changed - `packages/adapters/codex-local/src/server/codex-auth-merge-decision.cjs` — refactored from direction-parameterized (sandbox/host with `--direction`) to **direction-agnostic source/destination** semantics. Args: first = source path, second = destination path. Exit `10` (use source) only when: same `account_id`, same subscription-kind, both carry a parseable `last_refresh`, and `source.last_refresh > destination.last_refresh` (strict). Every other case exits `20` (keep destination). Unknown/extra flags are still rejected with exit `1`. No token bytes are emitted to stdout/stderr. - `packages/adapters/codex-local/src/server/codex-auth-merge.test.ts` — rewritten to source/destination API: 15-row predicate matrix (strictly-newer→`10`; tie/older/identity-mismatch/null-refresh/apikey/kind-mismatch/unusable→`20`) plus token-bytes non-emission guard. The inbound extract integration suite updated to match. - Runtime callers (`codex-auth-merge-extract.sh`) already invoke `node decision <sandbox> <host>` = `(source, destination)` and branch on exit `10` → keep sandbox, so **no caller changes are needed**. **Behavioral note:** unifying the two directions under a single "use source only when strictly fresher" rule means a tie (exact equal `last_refresh`) now keeps the **destination** for both the inbound and outbound frames. For inbound restore, this means an exact timestamp tie keeps the host copy. On a tie, both credentials are the same generation — preferring the canonical destination (host) copy is more conservative. This is an intentional consequence of the unified guard. ## Verification ```sh # From the codex-local package root: pnpm vitest run src/server/codex-auth-merge.test.ts # Expected: all predicate rows green (15-row source/destination matrix + token-leak guard) # Plus: codex-auth-merge-extract integration rows green ``` The test suite drives the `.cjs` as a child process over temporary `auth.json` fixtures: - Source/destination predicate matrix (15 rows): strictly-newer→`10`; tie→`20`; older→`20`; identity-mismatch→`20`; source last_refresh unparseable→`20`; destination last_refresh unparseable→`20`; apikey either side→`20`; kind-mismatch→`20`; unusable either side→`20` - Token-bytes non-emission guard: asserts no auth token bytes appear in `stdout`/`stderr` - Inbound extract integration: `codex-auth-merge-extract.sh` end-to-end rows including tie/identity-mismatch/sandbox-newer cases ## Risks - **Tie behavior change**: exact `last_refresh` ties now always keep the destination copy (was: keep-sandbox for inbound). On a tie both credentials are the same generation; keeping the canonical destination (host) copy is the more conservative choice. Covered by test assertions. - **Token non-emission**: `stdout`/`stderr` of the `.cjs` must not echo credential bytes; covered by the token-bytes non-emission guard test. - **Unknown/extra flags**: handled with `process.exit(1)` rather than a silent default, so misconfigured callers surface errors immediately. - **Caller compatibility**: the only runtime caller, `codex-auth-merge-extract.sh`, already calls `node decision <sandbox> <host>` with no flags — compatible with the new source/destination API with no changes required. - **Low risk overall**: the script is exercised end-to-end via the extract integration tests; all paths covered. ## Model Used Claude claude-sonnet-4-6 (Anthropic, `claude-sonnet-4-6`), 200 k context window, tool use and agentic code execution enabled. Automated Paperclip multi-agent system. ## 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: Harold Kim <harold@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing>