Commit Graph
7 Commits
Author SHA1 Message Date
Devin FoleyandPaperclip ac66fd65cb Fix Cody default model adapter test config (#9365)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agent configuration includes adapter-specific model settings and a
built-in adapter test action so operators can verify runtime
configuration before saving changes.
> - Cody/Codex-style local adapters can use an adapter default model
when the user clears the explicit model field.
> - The adapter test path still passed an object containing `model:
undefined` in some create/edit flows, which is different from omitting
the model and can break default-model behavior.
> - The previous fix was reverted because it also included an unrelated
skill documentation edit.
> - This pull request reapplies only the UI default-model test-config
fix, with no doc or skill changes.
> - The benefit is that testing Cody/Codex adapter settings with the
default model follows the same contract as saving default model
settings: no explicit model key is sent.

## Linked Issues or Issue Description

Bug report:

- Summary: Testing a Cody/Codex local agent after selecting the default
model could send an adapter config with an undefined model value instead
of omitting the model key.
- Expected behavior: Clearing the model to use the adapter default
should test with `adapterConfig: {}` unless another model is explicitly
selected.
- Actual behavior: The UI test-config path could preserve `model:
undefined`, causing the adapter test to fail instead of exercising the
default model.
- Related PRs: Reapplies the UI-only portion of #9361 after #9363
reverted the original PR.

## What Changed

- Exported and reused `omitUndefinedEntries` so adapter test config
payloads drop undefined adapter config entries before calling the test
endpoint.
- Hardened the current model display value so create-mode values that
are nullish or non-string do not crash the model selector/test flow.
- Added render coverage for editing a Codex agent back to the default
model and for testing a create form with the default model.

## Verification

- `pnpm exec vitest run
ui/src/components/AgentConfigForm.render.test.tsx`
- `pnpm check:token-gates`
- Confirmed `git diff origin/master --name-only` contains only:
  - `ui/src/components/AgentConfigForm.render.test.tsx`
  - `ui/src/components/AgentConfigForm.tsx`
  - `ui/src/lib/agent-config-patch.ts`

## Risks

Low risk. The change only removes `undefined` adapter config entries
from the UI adapter-test payload and adds focused render coverage.
Explicit model values and other adapter config fields are preserved.

> 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, GPT-5 coding agent, tool-use enabled. Context window size
not exposed in this runtime.

## 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

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-07-10 14:49:28 -07:00
Devin Foley e84731af70 Revert "Fix default model adapter test config" (#9363)
Reverts paperclipai/paperclip#9361
2026-07-10 08:55:23 -07:00
Devin FoleyandCody ebd62ca5ae Fix default model adapter test config (#9361)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The board UI lets operators create and edit agent adapter
configuration, including a primary model field and an adapter test
action.
> - For Cody and similar adapter forms, selecting the default model
means the model value is intentionally unset so the adapter can use its
default.
> - The adapter test path still allowed `model: undefined` to survive in
the generated adapter config, which could send an invalid test payload
instead of omitting the field.
> - This pull request normalizes create/edit adapter test config so
default-model selections omit `model` entirely.
> - The benefit is that testing an agent configured to use the adapter
default model exercises the same clean config shape that should be saved
and run.

## Linked Issues or Issue Description

No public GitHub issue was found for this local UI bug, so the problem
is described inline.

Bug description:
- What happened: using the adapter test action after choosing the
default model could include `model: undefined` in adapter config and
surface a UI/runtime error instead of testing with the adapter default.
- Expected behavior: choosing the default model should omit the `model`
field from adapter config so the adapter default is used.
- Steps to reproduce: edit a Codex/Cody-style agent with a concrete
model, switch the model selector to Default, then run the adapter Test
action.
- Paperclip version/commit: current `master` before this PR.
- Deployment mode: board UI, deployment-mode independent.

## What Changed

- Sanitized adapter test config assembly so undefined adapter config
entries are omitted before the test request is sent.
- Made create-mode current model display resilient when the model is
unset for adapter defaults.
- Added regression coverage for editing an existing agent from a
concrete model back to Default and testing it.
- Added regression coverage for create-mode testing with an
unset/default model.
- Hardened the developer skill wording used by the existing server skill
utility contract test.

## Verification

- `pnpm --filter @paperclipai/ui exec vitest run
src/components/AgentConfigForm.render.test.tsx`
- `pnpm exec vitest run
server/src/__tests__/paperclip-skill-utils.test.ts`
- GitHub PR checks on this branch are green, including Typecheck +
Release Registry, Build, General tests, e2e, verify, security scans, and
Greptile Review.

## Risks

- Low risk: this only removes undefined values from adapter test config
payloads, which aligns with the existing persisted patch behavior.
- Low risk: default-model display now treats unset create-mode model
values as an empty string.
- No database, API schema, migration, auth, or adapter runtime contract
changes.

> 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, GPT-5 coding agent, tool-enabled software-engineering
session. Exact context window size was not exposed by the runtime.

## 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: Cody <cody@paperclip.ing>
2026-07-10 08:54:20 -07:00
Nicky LeachandPaperclip 8a058f9d79 fix: deduplicate adapter-agnostic config keys (#9058)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - When you swap an agent's adapter (e.g. from one LLM provider to
another), the server merges the incoming PATCH body with stored config —
keys listed in \`ADAPTER_AGNOSTIC_KEYS\` are preserved regardless of
which adapter is active
> - That constant was defined independently in two places:
\`server/src/agents.ts\` (used by the adapter-swap route) and
\`ui/src/lib/agent-config-patch.ts\` (used by the UI patch builder)
> - PR #8975 fixed the bug where \`paperclipSkillSync.desiredSkills\`
was dropped on adapter swap by adding it to the server-side constant,
but the UI-side copy was not updated in the same PR — creating ongoing
drift risk
> - This pull request hoists \`ADAPTER_AGNOSTIC_KEYS\` into
\`packages/shared\` so both consumers import the same constant
> - The benefit is a single source of truth: any future key addition is
made in one place and both the server route and the UI patch builder
pick it up automatically, with a drift guard to catch any accidental
re-duplication

## Linked Issues or Issue Description

Refs #8975 — follow-up deduplication: #8975 fixed the runtime bug but
left the constant duplicated across server and UI. This PR closes that
gap.

## What Changed

- Added \`ADAPTER_AGNOSTIC_KEYS\` constant and \`AdapterAgnosticKey\`
type to \`packages/shared/src/adapter-agnostic-keys.ts\`
- Updated \`server/src/agents.ts\` to import the shared constant,
removing the local copy
- Updated \`ui/src/lib/agent-config-patch.ts\` to import the shared
constant, removing the local copy
- Added \`packages/shared/src/adapter-agnostic-keys.test.ts\`: drift
guard asserting the expected key set and both consumer import sites

## Verification

\`\`\`bash
pnpm exec vitest run packages/shared/src/adapter-agnostic-keys.test.ts
ui/src/lib/agent-config-patch.test.ts
server/src/__tests__/agent-instructions-routes.test.ts
pnpm --filter @paperclipai/shared typecheck
pnpm --filter @paperclipai/server typecheck
pnpm --filter @paperclipai/ui typecheck
\`\`\`

All 15 tests pass across the three files; all three packages typecheck
clean.

## Risks

Low risk — behavior-preserving refactor. The key set is unchanged; only
the import source changes. The drift guard will fail loudly if someone
accidentally re-introduces a local copy or modifies one without updating
the other.

> 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

- Provider: Anthropic
- Model: Claude Sonnet 4.6 (\`claude-sonnet-4-6\`)
- Context: standard context window, tool use enabled
- Reasoning: standard mode (no extended thinking)

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

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-07-05 21:47:38 -07:00
fengqing eb2cb916be fix(agents): preserve skill selection when switching adapter type (#8975)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Each agent runs on an adapter (`claude_local`, `codex_local`, …) and
can be assigned company skills that are synced into its runtime
> - An agent's desired-skill selection is persisted inside its single
`adapterConfig` JSON blob under `paperclipSkillSync`, even though the
selection is a company-level, adapter-agnostic choice
> - When a user changes an agent's adapter type, both the server PATCH
handler and the UI patch builder rebuild `adapterConfig` and carry over
only a hardcoded allow-list of adapter-agnostic keys (`env`, `cwd`,
instructions bundle, …)
> - `paperclipSkillSync` was missing from both allow-lists, so switching
adapters (e.g. claude_local → codex_local) silently wiped every assigned
skill
> - This pull request adds `paperclipSkillSync` to the adapter-agnostic
preservation list on both layers and covers it with regression tests
> - The benefit is that switching an agent's adapter no longer destroys
its skill configuration — skills are preserved exactly like
env/cwd/instructions already are

## Linked Issues or Issue Description

Fixes #8974

## What Changed

- **Server (authoritative fix)** — `server/src/routes/agents.ts`: added
`"paperclipSkillSync"` to the `ADAPTER_AGNOSTIC_KEYS` list in the
`changingAdapterType` branch of `PATCH /agents/:id`. On an adapter-type
change the handler now restores the skill-sync selection from the
existing persisted config when the incoming config omits it — the same
mechanism already used for `env`, `cwd`, and the instructions bundle.
This protects every API/CLI client, not just the UI.
- **UI (defense in depth)** — `ui/src/lib/agent-config-patch.ts`: added
`"paperclipSkillSync"` to the client-side `ADAPTER_AGNOSTIC_KEYS` in
`buildAgentUpdatePatch`, so the optimistic patch the client builds on an
adapter switch stops stripping the key before it reaches the server.
- **Tests** — added regression tests on both layers:
- `server/src/__tests__/agent-instructions-routes.test.ts`: `PATCH`ing
`adapterType` (claude_local → codex_local) with `replaceAdapterConfig:
true` keeps `adapterConfig.paperclipSkillSync`.
- `ui/src/lib/agent-config-patch.test.ts`: `buildAgentUpdatePatch`
preserves `paperclipSkillSync` when the overlay changes the adapter
type.

## Verification

```
# server (run from repo root)
cd server && ../node_modules/.bin/vitest run \
  src/__tests__/agent-instructions-routes.test.ts \
  src/__tests__/agent-skills-routes.test.ts \
  src/__tests__/agent-adapter-validation-routes.test.ts \
  src/__tests__/agent-permissions-routes.test.ts
# 83 passed
../node_modules/.bin/tsc --noEmit -p tsconfig.json   # clean

# ui
cd ui && ./node_modules/.bin/vitest run src/lib/agent-config-patch.test.ts   # 7 passed
pnpm --filter @paperclipai/ui typecheck   # clean
```

Both new tests fail without the corresponding source change (verified
red → green).

Manual: create an agent on `claude_local`, assign skills, switch it to
`codex_local`, and confirm `GET /api/agents/:id/skills` still returns
the desired skills.

## Risks

Low risk.

- The change only *adds* one key to an existing preservation allow-list;
it does not alter how any other key is handled. Behavior for agents
without a `paperclipSkillSync` block is unchanged (the key is simply
absent and nothing is copied).
- `paperclipSkillSync` is adapter-agnostic (company skill keys, not
adapter-specific), so carrying it across an adapter switch is always
safe — a target adapter that does not support skill sync just ignores
it, and switching back restores the selection.
- Same-adapter config edits already merged and preserved the key; this
only closes the adapter-type-change gap, matching the existing
env/cwd/instructions behavior.
- Follow-up (not in this PR to keep it minimal): the server and client
`ADAPTER_AGNOSTIC_KEYS` lists are maintained separately and already
diverge (`instructionsFilePath` is client-only); a shared constant could
prevent future drift.

## Model Used

Claude Opus 4.8 (`claude-opus-4-8`, 1M-token context), extended thinking
enabled, with tool use (file edit, shell, GitHub CLI) via Claude Code. A
read-only sub-agent was used to trace the root cause across the server
and UI layers.

## 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 (bug fix, not a feature)
- [x] I have searched GitHub for duplicate or related PRs and linked
them above (none found)
- [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
(`fix/preserve-skills-on-adapter-type-switch`) 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 (N/A —
internal config-preservation fix, no user-facing docs or API contract
change)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending CI on this PR)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending review)
- [x] I will address all Greptile and reviewer comments before
requesting merge
2026-07-04 15:25:01 -07:00
a3de1d764d Add cheap model profiles for local adapters (#4881)
## Thinking Path

> - Paperclip is a control plane for autonomous AI companies, where
adapters are the boundary between the board, agents, and execution
runtimes.
> - Local adapters currently expose a primary runtime configuration, but
operators often need a cheaper model lane for routine or low-risk work.
> - That cheap lane has to stay adapter-owned: runtime profile settings
should not mutate the primary adapter config or bypass existing
auth/secret mediation.
> - Issue creation also needs an ergonomic way to request primary,
cheap, or custom model behavior for a selected assignee.
> - This pull request adds a first-class `cheap` model profile contract
across adapter capabilities, heartbeat config resolution, agent
configuration, and issue creation.
> - The benefit is cheaper task execution can be configured and
requested explicitly while preserving adapter boundaries, secret
handling, and audit visibility.

## What Changed

- Added adapter model-profile capability metadata and a `cheap` profile
contract for supported local adapters.
- Applied `runtimeConfig.modelProfiles.cheap.adapterConfig` during
heartbeat config resolution, including requested/applied/fallback run
metadata.
- Added agent configuration UI for cheap model profile settings without
writing those settings into primary `adapterConfig`.
- Added New Issue assignee model lane controls for Primary / Cheap /
Custom and request payload handling.
- Added run ledger profile badges and Storybook stories for the new
cheap-lane UI states.
- Added tests for validators, heartbeat model profile application,
permission/secret mediation, UI payload helpers, and run ledger
rendering.
- Added committed UI verification screenshots under
`docs/pr-screenshots/pap-2837/`.
- Addressed Greptile review feedback around cheap-profile defaults,
shared profile types, and fallback test data.

## Verification

Local:

- `pnpm exec vitest run packages/shared/src/validators/issue.test.ts
server/src/__tests__/adapter-registry.test.ts
server/src/__tests__/agent-permissions-routes.test.ts
server/src/__tests__/heartbeat-model-profile.test.ts
ui/src/components/IssueRunLedger.test.tsx
ui/src/lib/agent-config-patch.test.ts
ui/src/lib/issue-assignee-overrides.test.ts
ui/src/lib/new-agent-runtime-config.test.ts` — passed, 8 files / 103
tests.
- `pnpm exec vitest run ui/src/lib/new-agent-runtime-config.test.ts
ui/src/components/IssueRunLedger.test.tsx` — passed after
Greptile/rebase follow-up, 2 files / 17 tests.
- `pnpm --filter @paperclipai/ui typecheck` — passed after
Greptile/rebase follow-up.
- `pnpm -r typecheck` — passed.
- `pnpm build` — passed.
- `pnpm test:run` — did not complete successfully in this local
worktree: it stopped in pre-existing `@paperclipai/adapter-utils`
sandbox/SSH fixture suites outside this PR diff. Failures were 5s local
timeouts plus `git init -b` unsupported by this machine's Git 2.21.0.
The branch-specific targeted suites above passed.
- Branch was fetched/rebased onto `public-gh/master`; `git rev-list
--left-right --count public-gh/master...HEAD` reports `0 9`.

Remote PR checks on latest head
`e30bf399146451c86cee98ed528d51d33fa5af5a`:

- `policy` — passed.
- `verify` — passed.
- `e2e` — passed.
- `Greptile Review` — passed, confidence score 5/5; Greptile review
threads resolved.
- `security/snyk (cryppadotta)` — passed.

Screenshots:

- [New issue cheap lane
desktop](https://github.com/paperclipai/paperclip/blob/PAP-2837-plan-cheap-model-for-adapters-that-can-support-it/docs/pr-screenshots/pap-2837/newissue-cheap-desktop.png)
- [New issue custom lane
desktop](https://github.com/paperclipai/paperclip/blob/PAP-2837-plan-cheap-model-for-adapters-that-can-support-it/docs/pr-screenshots/pap-2837/newissue-custom-desktop.png)
- [New issue unsupported adapter
desktop](https://github.com/paperclipai/paperclip/blob/PAP-2837-plan-cheap-model-for-adapters-that-can-support-it/docs/pr-screenshots/pap-2837/newissue-unsupported-desktop.png)
- [Run ledger model profile badges
desktop](https://github.com/paperclipai/paperclip/blob/PAP-2837-plan-cheap-model-for-adapters-that-can-support-it/docs/pr-screenshots/pap-2837/runledger-profile-badges-desktop.png)
- Mobile variants are also in `docs/pr-screenshots/pap-2837/`.

## Risks

- Medium: heartbeat config mediation now merges runtime model profiles
into adapter configs, so adapter secret normalization and host-command
restrictions must keep covering nested config paths.
- Medium: the UI adds another issue creation choice; unsupported
adapters must keep hiding the cheap lane and preserve primary behavior.
- Low migration risk: no database migration is included.

> 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 coding agent using GPT-5-class reasoning with repo tool use
and command execution. Exact served model/context window was not exposed
by the runtime.

## 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
- [ ] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] If this change affects the UI, I have included before/after
screenshots
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-04-30 15:32:04 -05:00
Asish Kumar 44d94d0add fix(ui): persist cleared agent env bindings on save
Agent configuration edits already had an API path for replacing the full adapterConfig, but the edit form was still sending merge-style patches. That meant clearing the last environment variable serialized as undefined, the key disappeared from JSON, and the server merged the old env bindings back into the saved config.

Build adapter config save payloads as full replacement patches, strip undefined keys before send, and reuse the existing replaceAdapterConfig contract so explicit clears persist correctly. Add regression coverage for the cleared-env case and for adapter-type changes that still need to preserve adapter-agnostic fields.

Fixes #3179
2026-04-09 17:50:14 +00:00