## What Problem This Solves
Skill Workshop experience review ran as an isolated agent with a re-rendered 60k-character transcript, its own bootstrap, and a trimmed tool surface. Every review was a cold request: no prompt-cache reuse, a large re-serialized trajectory, and a reasoning/tool profile that differed from the foreground turn. Autonomous updates could target any workspace skill, size limits allowed 40 KB skills to grow unchecked (one deployed workspace hit a 21 KB `SKILL.md`), the collection reviewer had to re-list every skill in its plan and read them under a fixed 24k-character budget (a 16-skill workspace failed every pass with "Read every current skill before reconciling"), collection review only recorded successes so a failing pass retried on every Gateway restart, and `openclaw skills curator status` showed nothing about what the last reviews did.
## Why This Change Was Made
- **Warm fork.** Experience review now continues the finished turn from the same in-memory session prefix (same session identity, bootstrap, skills prompt, tool schemas, `promptCacheKey`), appends one short review message, and runs with `sessionPersistence: "detached"` so nothing it writes reaches the foreground transcript or session record. Only `skill_workshop` executes; every other tool keeps its schema for cache parity and fails with a next-step message. The gate wraps the core tool list before Code Mode / Tool Search compaction, so catalog-hidden tools stay gated while `tool_call`/`exec` remain callable. `src/agents/embedded-agent-runner/run/attempt.skills-policy.test.ts` asserts identical system-prompt and tool digests between a foreground turn and its review, and that `tool_call` reaches `skill_workshop` but not `read` during review. Code Mode swarm globals (`phase`, `log`, `agents.run`) are the `sessions_spawn` capability and gate on the same allowlist, so a review cannot emit foreground lifecycle events or launch collectors. The review shares the foreground session, so it no longer retires that session's MCP runtime on run end; it reuses the warm runtime like any turn. Accepted tradeoff: the review inherits foreground tool construction (it creates the session MCP runtime only when the session has none, and spawns per-attempt LSP servers only when configured), because splitting tool construction from the foreground turn would break the cache-parity invariant this PR exists for.
- **One apply owner, rechecked at the write boundary.** `src/skills/workshop/autonomous-apply.ts` decides apply-vs-pending for both the post-review loop and foreground repair: creates and updates to Workshop-authored skills apply; updates to user-authored skills stay pending with a recorded reason. `applySkillProposalTransition` rechecks path-based ownership under the commit lock for non-operator actors (agent, or no actor), so a claim released after the pre-check cannot let an agent write a user-authored skill; gateway and CLI operators still approve any pending proposal. The user-authored pending write runs under the same commit lock and rereads the row, so an operator apply that lands first is kept. The old `auto-apply.ts` path and name-based `listWorkshopAuthoredSkillNames` are deleted.
- **Sparse collection plan and model-relative reads.** The collection reviewer returns only `write` and `drop` entries; unlisted skills stay untouched, so a 16-skill workspace no longer has to restate 16 keeps. The skill read budget is derived from the model's context window (35%) instead of a fixed 24k characters, with the 10,000-character skill cap still enforced per skill. No windowed read parameters: a skill is read whole or not at all.
- **Weekly cadence.** Collection review runs once every 7 days instead of daily (`REVIEW_INTERVAL_MS`), matching how slowly a skill library actually changes; the attempt is recorded before the model starts, so a failed pass does not retry on every restart.
- **Lean cap.** `AUTONOMOUS_SKILL_MAX_CHARS = 10,000` in `src/skills/workshop/collection-contracts.ts` is enforced by the tool and by collection reconcile; an oversized skill may only shrink. Tool description, experience prompt, and collection prompt were rewritten around procedures-not-records, one trigger per branch, and "NOTHING_TO_LEARN is the correct answer for most turns".
- **Detached runs end quietly.** A detached run writes no transcript or session record and runs under the foreground session key, so `attempt-finalize` now skips agent_end side effects for it: plugins do not observe the review as a foreground turn, and a deep review cannot schedule a successor review of itself.
- **Recorded outcomes.** Experience review records `applied | proposed | nothing | failed` with usage; one attempt per turn, drop on error. `openclaw skills curator status` prints the last collection and experience review outcome.
## User Impact
- Default `auto` mode: experience review reuses the foreground prompt cache and skips message/other tool execution, so review cost is one bounded continuation instead of a cold re-render.
- Autonomous edits touch only Workshop-authored skills; handwritten skills surface as pending proposals for operator approval.
- Autonomous `SKILL.md` results stay ≤ 10,000 characters.
- Collection review runs weekly and succeeds on larger skill libraries. A workspace with no recorded attempt reviews at the first daily check after Gateway start; an upgraded install keeps its recorded last attempt and reviews once it is older than 7 days. The 10,000-character cap applies to the next autonomous write; existing oversized skills are only ever shrunk.
- `openclaw skills curator status` shows the last collection and experience review outcome, time, and tokens.
- Docs: https://docs.openclaw.ai/tools/self-learning, https://docs.openclaw.ai/tools/skill-workshop
Related: #123866, #128871 both patch skills above the reviewer read budget; this PR caps autonomous skill size and restricts oversized skills to shrink-only rewrites.
## Evidence
- Live gateway (Linux, Telegram, `xai` provider) at `133ffe3`: manual experience review on a real foreground turn → `nothing` (usage: 0 uncached input, 13,902 cached, 36 output — the forked review hit the foreground prompt cache); manual collection review over the workspace → `succeeded`; the three oversized skills were rewritten under the cap (21,002 → 9,449; 11,882 → 7,735; 10,300 → 7,413 bytes, originals in `skill-workshop/collection-backups/`), the rest untouched, and the pre-PR "Read every current skill before reconciling" failure is gone. The next foreground turn's automatic experience review recorded `nothing` and `openclaw skills curator status` shows both outcomes. The apply recheck, MCP-runtime change (`26e821f`), and detached agent_end skip (`7e4a506`) landed after that run and are covered by the regression tests below.
- `pnpm test src/skills/workshop src/agents/tools/skill-workshop-tool src/gateway/server-methods/skills` plus `src/agents/embedded-agent-runner/run/attempt.skills-policy.test.ts`, `src/agents/embedded-agent-runner/run/attempt.tool-search-catalog-abort.test.ts`, `src/cli/skills-cli.curator.test.ts`, `src/agents/harness/tool-surface-bridge.test.ts` — green. New regression tests (`service.test.ts` agent-vs-operator apply on a user-authored skill and operator apply kept over a stale pending snapshot; `experience-review.apply.test.ts` no `cleanupBundleMcpOnRunEnd`; `attempt-phase-lifecycle.test.ts` no agent_end for a detached run; `code-mode-swarm.test.ts` swarm globals refused under the review allowlist) fail on the pre-fix code.
- Dependent sweep: 46 test files importing the touched modules — green.
- `oxfmt`, `scripts/run-oxlint.mjs` on changed files, `git diff --check` — clean.
- `pnpm tsgo && pnpm check:test-types` on Blacksmith Testbox — clean.
- Local ClawSweeper review (`gpt-5.6-terra`, high): `133ffe3` raised two findings (ownership recheck removed from the apply boundary; review retiring the shared session's MCP runtime), fixed in `26e821f`; `26e821f` raised one (detached review re-entering agent-end scheduling), fixed in `7e4a506`; `7e4a506` raised three: pending write racing an operator apply and Code Mode swarm globals bypassing the execution gate, both fixed in `e4b4322`; "prove detached review does not start configured MCP or LSP runtimes" is skipped as the cache-parity tradeoff stated above (LSP tool schemas come from the LSP runtime; the MCP runtime is session-owned and already warm). Maintainer decision on `e4b4322`: accepted — a detached review reuses the session MCP runtime and re-spawns configured per-attempt LSP servers exactly like a foreground turn; it still cannot execute them.
- Unrelated CI on `26e821f`/`7e4a506`: `check-lint-core-2` (`max-lines` in `src/gateway/server-methods/models-list-result.ts`, unused param in `models.test.ts`, both from #129332) and `checks-node-compact-small-8` (`doctor-auth.profile-health.test.ts`, Claude CLI auth from #129052) fail identically on `main` run 32857954734; `checks-node-compact-small-31` pins plugin SDK export counts (`4340` vs `4342`) that #129052 moved — this PR touches no `src/plugin-sdk` file. On `e4b4322` the failing set is the same twelve `checks-node-compact` shards that fail on `main` run 32857954734 (`large-5/12/13/14/15/18/22`, `small-8/14/20/21/26`) plus `small-31`; `check-lint-core-2` passes here. None are fixed here; #129357 carries the `main` fix.
- LOC (raw numstat): production +712 / −664 (net +48; the four review-fix commits after the live run add +76 / −26); tests + docs +1269 / −1238 (net +31).
* refactor(agents): reduce tool failure warnings to two rules
* test(agents): remove obsolete tool recovery receipt proof
* refactor(agents): drop unused meta param from buildToolMutationState
* test(agents): remove stale tool warning assertions
* test: export runtime source snapshot from closed runtime-snapshot mocks
Heal the main breakage introduced by 9441e3fe6e / #126531, which added a runtime source-snapshot read to provider model route resolution. Closed Vitest factories now return null for that source snapshot, preserving their pre-projection behavior.
* test(gateway): make compaction read-error faults order-immune
Generation-2 CI failure in run 32342180898, job 96343444772 showed that the mock factory initialized while shared gateway-server importers remained bound to the real transcript reader.
The dedicated isolated project fixes normal shards. Complete its ownership by adding it to the root project matrix and excluding the test from the non-isolated OPENCLAW_GATEWAY_PROJECT_SHARDS=0 fallback.
* fix(copilot): add OpenClaw prompt guidance
Copilot append-mode system messages included credential safety, workspace bootstrap, and extra context but omitted OpenClaw delegation and reply-delivery policy.
Build guidance from the final policy-filtered tool surface so visible delegated work, Skill Workshop, and source replies follow the same behavior as Codex.
* fix(copilot): break prompt guidance import cycle
The CI architecture gate detected a cycle through attempt-config and prompt-guidance. Isolate raw-run mode detection in a leaf module.
Show an explicit waiting acknowledgment when sessions_yield ends an otherwise-silent interactive turn, while keeping private resume context out of channel delivery and preserving existing visible replies.
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
Use the selected harness for isolated title generation, including native Codex auth.
Retry failed dashboard titles from the first user message without overriding manual names.
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
* refactor: consolidate coercion ownership
Centralize four canonical coercion helpers, migrate exact core and plugin duplicates through narrow Plugin SDK facades, and enforce declaration and plugin-normalization ownership boundaries.
The sweep adds eight focused SDK exports while deleting more production and tooling code than it adds. User-visible behavior is unchanged except for safer equivalent object and UI parsing at existing boundaries.
* fix: guard integer option ownership
Register resolveIntegerOption with the canonical function owner and extend the declaration-guard fixture so future local duplicates fail validation.
* fix: keep integer helpers on numeric facade
Remove the unshipped duplicate string-coerce exports and route every affected plugin consumer through the existing number-runtime contract.
* fix: point numeric coercion to number runtime
Make boundary and declaration diagnostics recommend the canonical numeric facade, with failing-before coverage for both guidance paths.
* perf(doctor): keep bundled doctor contract closures dependency-light
Doctor contract enumeration cold-loads each plugin's doctor-contract-api
closure via jiti, so a static value import of openclaw/plugin-sdk/runtime-doctor
pulled the state-db/kysely graph (~4.3s per closure) into
listPluginDoctorLegacyConfigRules / listPluginDoctorStateMigrationEntries.
- migrate all light doctor-contract closures (66 files) to the
dependency-light openclaw/plugin-sdk/runtime-doctor-migrations subpath
- voice-call: load detect/repairOpenClawStateDatabaseSchema* lazily inside
the migration bodies; keep only a type-only static runtime-doctor import
- matrix: split pure credential record shapes/normalizers into
credentials-state.ts so the doctor closure no longer imports the sync
plugin-state store through credentials-read
- guard: doctor-contract-closure-guard.test.ts now forbids static value
imports of runtime-doctor in closures alongside agent-runtime
* fix(matrix): keep credential revocation record type module-local
Knip production scan flags the export as consumer-less; the type is only
referenced by the exported union and revocation guard signature.
* perf(plugins): declare doctor contract surfaces
* perf(doctor): slim migration import closures
* perf(plugins): narrow doctor declaration record surface and wire owner-test lane
Registry records carry only the doctorContract declaration instead of the whole
parsed manifest, and check:changed now selects the src/plugins-owned declaration
honesty and closure-guard tests for extension module/manifest changes so
cross-lane drift cannot pass PR classification.
* fix(doctor): keep control-plane dist imports require-safe
Keep doctor and channel control-plane chunks off exec-class dependencies, and enforce native require(esm) loading during postbuild.
* chore(plugin-sdk): regenerate API baseline
* chore(plugin-sdk): sync export ordering
* fix(plugins): satisfy doctor contract CI boundaries
* perf(doctor): make qqbot doctor closure dependency-light
qqbot was the last plugin above 5s in doctor state-migration enumeration
(~8s under tsx/jiti). The cost was not the state-key builder (already a
leaf): its doctor closure value-imported the runtime-doctor SDK barrel,
whose plugin-state-store/state-db re-exports pull kysely (~330 modules),
plus security-runtime for one fileExists (~200 modules), all resolved
per-module by jiti during enumeration.
Split the migration-define helpers and light re-exports into a new
private-local plugin-sdk/runtime-doctor-migrations subpath; runtime-doctor
re-exports it so its public surface is byte-identical (API baseline hash
unchanged). qqbot's doctor-contract and state-migrations now import only
the light subpath, swapping fileExists for the equivalent async
legacyStateFileExists already in the closure.
qqbot enumeration: ~8.0s/531 modules -> ~0.25s/18 modules.
* chore(plugin-sdk): drop private-local subpath from API baseline
runtime-doctor-migrations is private-local-only; the baseline tracks public
modules, and the earlier line was generated before the classification.
* fix(plugins): register runtime-doctor-migrations boundary paths
The private-local subpath list feeds the extension package boundary map;
the shared paths config and xai's derived overrides must carry the same
entry or the boundary contract test fails.
Ayaan Zaidi