Skip to content

(feat) hot-reload engine settings via settings.yaml + /acp config (phase 1; supersedes #76) - #130

Merged
Tyan66666 merged 9 commits into
Tyan66666:mainfrom
ranxianglei:feat/runtime-settings-75
Sep 11, 2026
Merged

(feat) hot-reload engine settings via settings.yaml + /acp config (phase 1; supersedes #76)#130
Tyan66666 merged 9 commits into
Tyan66666:mainfrom
ranxianglei:feat/runtime-settings-75

Conversation

@ranxianglei

Copy link
Copy Markdown
Contributor

What this PR does

Makes the engine's runtime configuration hot-editable, no restart required. The six scalar knobs — modelContextLimit, autoModelContextLimit, nudgeMinContextLimitPct, nudgeMaxContextLimitPct, nudgeEmergencyThresholdPct, autoNudge — can now be changed through the host's ~/.dsh/settings.yaml (namespace compaction-acp) and take effect on a running session. Adds a /acp config command (list / set / reset) and a settingsEnabled kill switch that reverts to the current composition-layer-only behavior. Full design: docs/settings-integration-design.md.

Why a new PR (supersedes #76)

PR #76 carried this exact feature work, but it was branched from an older commit. Since then, main shipped three changes that touch the very files this feature rewrites — #106 (output reservation), #103 (shadow-price meter heuristic), #124 (tool-pairing mirror) — which left #76 with conflicts that can no longer be auto-resolved. This branch is the same feature work with main merged in and every conflict resolved, so it is cleanly mergeable.

Merge notes (what was kept from each side)

Verification (all run on the merged branch)

  • npm run typecheck: clean (tsc --noEmit, zero errors).
  • npm test: 228/228 pass, 0 fail.
  • dist/ rebuilt from merged source.

Supersedes #76.

Tyan66666 and others added 6 commits September 6, 2026 18:54
…yaml / /acp config

Phase 1 of issue Tyan66666#75: the six scalar knobs (modelContextLimit,
autoModelContextLimit, nudgeMin/Max/EmergencyPct, autoNudge) now resolve
through the host settings layer (installSettingsSection seam) — editing
~/.dsh/settings.yaml or the new /acp config subcommand applies to running
sessions without a restart. Window-key changes clear the probe cache so a
fixed gateway re-probes; settingsEnabled:false is a composition-only kill
switch. New deps: peer @deepseek-ai/dsh-settings (dual-tuple range) +
schemastery peer; dev deps pinned exact (token-meter caret fixed). 188 tests
pass incl. E2E via an in-memory provider + the HMR remount gate.
…TS.md module map + design doc

Config table rows carry runtime-adjustable markers, a new README section
covers settings.yaml / /acp config usage and the kill switch, the window
probe-failure copy now points at /acp config, and the docs index lists the
new design doc.
Sync Tyan66666#106 output-reservation, Tyan66666#103 shadow-price (heuristicTokens ?? tokens), Tyan66666#124 tool-pairing local mirror, the dist-bot CI workflow, and related docs into the settings-integration feature branch. Nine conflicts resolved as orthogonal unions (the settings seam and the output-reservation logic are independent and both kept); dist rebuilt from merged source; full regression green (228/228).
Resolve conflicts against the 0.1.5 seam and port the settings wiring. The 0.1.0 seam exposed a free function installSettingsSection(ctx, ns, schema, entry, hooks) plus a settingsNamespace(id) helper; 0.1.5 replaced both with the provider method settingsProvider.installSection(...) reached through ctx.inject(['settings'], ...) and a plain namespace literal branded at the call site.

Also: peer range for @deepseek-ai/dsh-settings now spans the 0.1.5 line like the other four seam peers, @deepseek-ai/schemastery is declared as a peer (it is a value import), devDep pinned to 0.1.5-rc.2, and the docs that said four runtime seam packages now say five. dist/ is not carried on the branch: repo policy is that dist-bot rebuilds it after the merge lands.
@Tyan66666

Copy link
Copy Markdown
Owner

Merged main (through #93 + the dist-bot refresh) into this branch and adapted the settings seam to dsh-settings 0.1.5 — pushed as 9af0cab4 (a merge commit; its first parent is 95fca89, so this is a plain fast-forward).

1. Why the merge was needed, and what the seam adaptation touched

The branch was written against dsh-settings 0.1.0-rc.6, where the seam was two free functions: installSettingsSection(...) and settingsNamespace(). On the host line this PR targets, both are gone — installSection is now a SettingsProvider method that must be called inside a ctx.inject(['settings'], …) callback, and the namespace is a plain string literal ('compaction-acp'), not a branded runtime helper. Without the adaptation the plugin does not compile against the installed seam at all (error TS2305: Module '"@deepseek-ai/dsh-settings"' has no exported member 'installSettingsSection').

Adapted in this push: src/settings.ts (type-only import, plain-literal namespace, the String(descriptor.ns) === ACP_SETTINGS_NAMESPACE comparison), src/index.ts (one inject callback that captures the service and registers the section), package.json (peer >=0.1.5-alpha.1 <0.1.6-0 + devDep 0.1.5-rc.2), and tests/peer-range.test.ts (dsh-settings folded into the shared five-seam peer assertion). dist/ carries no changes — it is reset to origin/main, since the repo now rebuilds it after merge via .github/workflows/dist-bot.yml.

2. Four findings from a merge review, all fixed

  • (blocker) The window projection gate read the composition snapshot, not the live value. windowFor tested this.config.autoModelContextLimit while the probe branch below it already used the live settings snapshot. Consequence: /acp config set autoModelContextLimit false cleared the window cache and printed success, but the very next windowFor still returned { source: 'projection' } — the knob's documented semantics were violated in both directions. Both gates now read the live source.
  • The registered base layer was the resolved snapshot instead of the filtered composition subset. The seam registers entry verbatim (verified against the real 0.1.5 provider: descriptor.base === entry, no validation on register), so passing resolveAcpSettings(filterSettingsEntry(config)) made every untouched key look composed: /acp config list printed source: base for keys the operator never wrote, and reset named a composition value that does not exist. The filtered subset is now what gets registered; the resolved snapshot is used for reads only.
  • Nothing restored state when the settings provider detached. The inject callback left this.settingsService and the source thunk pointing at the disposed provider, so /acp config kept reporting available: true and calling describe/update on a dead service while the engine silently froze at the last value. The callback now returns a disposer that clears both.
  • Both reset paths called replaceSection unguarded and rewrote the whole section. A SettingsConflictError (or a schema rejection) escaped as a raw rejection instead of the friendly "run /acp config again" copy that set uses; and because the settings layer does not whitelist keys (publish of an unknown key reaches descriptor.user and update({ bogus: 1 }) is accepted — both verified), rebuilding the section from SETTING_KEYS would silently delete a hand-written entry. Both paths now share the set error mapping, and a single-key reset deletes only the target key while carrying the rest through verbatim. The write-back data always comes from the YAML file, never from this.config, so no object/function value can reach the layer.

3. Verification

  • npm run typecheck — clean.
  • npm test275/275 (was 270; +5 regression locks in tests/settings.test.ts, each asserting both the pre-fix and post-fix value, so every one of them goes red against the old code): the filtered base entry plus /acp config list sources, the seam→window gate, the kernelConfigFor output, provider-detach fallback through the inject disposer, and reset preserving hand-written keys.
  • Docs in the same push (per the repo's "docs stay in sync" rule): AGENTS.md gains hard-won rule 17 (the three traps above), docs/settings-integration-design.md §4.6 documents the single-key-reset semantics and §6 lists the added locks.
  • git diff origin/main -- dist is empty, so no dist conflict with the dist-bot.

This PR supersedes #76 (it carries the same feature plus /acp config); #76 can be closed once this lands.

@Tyan66666

Copy link
Copy Markdown
Owner

Merged origin/main into the branch so the PR is mergeable again (7237eac), then pushed.

Why it had gone stalemain moved on while this PR was open: c527104 (#118) added paging to /acp decompress, followed by the dist-bot commit baa8834.

Conflict resolution — one file, one hunk: src/commands.ts, inside the /acp usage string. Both sides had appended a clause to it, so both are kept:

/acp status | /acp compress <startSeq> <endSeq> <summary> | /acp decompress <blockId> [offset] [limit] | /acp config [list|set <key> <value>|reset <key>|all]

Everything else auto-merged: AGENTS.md, README.md, README.en.md, docs/INSTALL.md, src/tools.ts, tests/commands.test.ts. Both features survive the merge — the paged path still calls sliceDecompressPage(expanded, offset, limit, DEFAULT_DECOMPRESS_PAGE_CHARS, …) and rejects a bad offset with offset must be a non-negative integer, while this PR's /acp config path still runs configText(env, …) and maps SettingsConflictError through settingsWriteFailure in all three write paths.

dist — reset to origin/main's state. Under the current policy (AGENTS.md) a PR branch does not carry dist/; dist-bot rebuilds it after the merge.

Verificationnpm run typecheck clean; npm test 280/280, i.e. main's pagination tests and this PR's settings tests passing together. Net diff vs main: 17 files, +1596/−57 (src/settings.ts 253 lines, tests/settings.test.ts 396, docs/settings-integration-design.md 539).

This PR supersedes #76, so #76 can be closed once it lands.

@ranxianglei

ranxianglei commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Powered by ework · qwen3.8-27b

Independently re-verified against the current head 7237eac (isolated worktree checked out at that sha, deps installed fresh from its lockfile). Everything checks out — no blockers found.

Build & tests (independent run)

  • npm run typecheck → clean (exit 0)
  • npm test280/280 pass, 0 fail (main's pagination tests + this PR's settings tests together, as you noted)
  • npm ci → exit 0, so the newly added @deepseek-ai/dsh-settings + @deepseek-ai/schemastery peer/devDep closure resolves with no ERESOLVE cascade

Diff & dist (vs true current origin/main = baa8834)

  • Net diff: 17 files, +1596/−57 — matches your number exactly (src/settings.ts 253, tests/settings.test.ts 396, docs/settings-integration-design.md 539)
  • git diff origin/main -- distempty ✓ (dist correctly reset to main; dist-bot rebuilds post-merge)

The conflict you flaggedsrc/commands.ts:208 usage string keeps both clauses: /acp decompress <blockId> [offset] [limit] (main #118) and /acp config [list|set <key> <value>|reset <key>|all] (this PR). Both features survive the merge.

Seam adaptation (spot-checked in code)

  • src/settings.ts: type-only import from @deepseek-ai/dsh-settings, plain-literal ACP_SETTINGS_NAMESPACE = 'compaction-acp', String(descriptor.ns) === ACP_SETTINGS_NAMESPACE — matches the 0.1.5 SettingsProvider.installSection shape.
  • All three traps from rule 17 are handled: base layer registers the filtered compositionEntry (src/index.ts:289, src/index.ts:318); both window gates read the live readSettingsSource() (src/index.ts:509, :523, :538); the inject callback returns a disposer clearing settingsService + thunk (src/index.ts:337-338). Single-key reset deletes only the target key and carries the rest verbatim (src/commands.ts:295-309), with SettingsConflictError mapped through the shared handler in all three write paths (src/commands.ts:205).

Docs — synced: AGENTS.md rule 17 present; README zh/en gained the runtime-settings section + config-table rows (+ settingsEnabled); docs/settings-integration-design.md covers §4.6 (/acp config) and §6 (test plan).

One minor nit (non-blocking): docs/e2e-harness-design.md §「依赖钉」still describes the pinned closure as 0.1.0-rc.6 ("18 新增") and does not list the new dsh-settings seam. AGENTS.md §4 asks to re-derive/document the closure whenever a host package is added, so that section should be refreshed (add dsh-settings, correct the version line to the 0.1.5 line). Doesn't gate the merge — happy to sweep it in a small follow-up commit if you want.

Verdict: ready to merge. (Merges stay human-only, so I won't merge it myself.) Once it lands, #76 can be closed as superseded.

一句话中文:独立复验通过——typecheck 干净、280/280、git diff origin/main -- dist 为空、净 diff 17 文件与你所述完全吻合,seam 适配与四处修复都在代码里确认到位;唯一小瑕疵是 docs/e2e-harness-design.md 的依赖钉一节需补 dsh-settings/更到 0.1.5 线(非阻塞),可以合并。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants