fix: read the settings values, not the references they arrive as
Every field is `volatile()`, which is what puts the settings card on the Plugins page — and what makes the resolved config carry a reference per field rather than a value. Every read was a plain-value test (`typeof x === 'string'`, `Number(x)`, `x !== true`), so all of them were false and the plugin behaved as if nothing had been configured. The card displayed and persisted correctly the whole time, because the settings wire unwraps references for display; only the plugin's own reads were broken. Reads now go through `configValue()`, and the tool takes a fresh snapshot per call, which is what lets an edit reach a running instance.
This commit is contained in:
1 parent
9ebd484e60
commit
8f68ef4e37
4 files changed
+231
-14
No files matched your search
+54
-2
@@ -101,9 +101,18 @@ writeFileSync(stubSlopo, '#!/bin/sh\nexit 0\n', { mode: 0o755 })
|
||||
*
|
||||
* `jobs` is deliberately absent, which forces the tool down its foreground path
|
||||
* so the assertions can see a result rather than a job handle.
|
||||
*
|
||||
* `configFromSchema` hands `apply` the config the way Cordis actually builds it
|
||||
* — through `Config['~standard'].validate`, so every volatile field arrives as a
|
||||
* reference object rather than a value. Without it a test can only prove the
|
||||
* plugin works on a shape no real profile ever produces.
|
||||
*/
|
||||
function harness(overrides = {}, { withSettings = true, withJobs = false, withShell = true, sandboxMode, sandboxPolicyResolver, shellRun } = {}) {
|
||||
const config = { ...FULL_CONFIG, binary: stubSlopo, ...overrides }
|
||||
function harness(
|
||||
overrides = {},
|
||||
{ withSettings = true, withJobs = false, withShell = true, sandboxMode, sandboxPolicyResolver, shellRun, configFromSchema = false } = {},
|
||||
) {
|
||||
const plain = { ...FULL_CONFIG, binary: stubSlopo, ...overrides }
|
||||
const config = configFromSchema ? resolveConfig(plain) : plain
|
||||
// A per-harness shell, so a test can vary sandboxMode and every resolved
|
||||
// request is recorded for assertion.
|
||||
const scopedShell = {
|
||||
@@ -187,6 +196,13 @@ function harness(overrides = {}, { withSettings = true, withJobs = false, withSh
|
||||
|
||||
const execContext = (cwd) => ({ signal: new AbortController().signal, agent: { id: 'session-1', session: { header: { cwd } } } })
|
||||
|
||||
/** Resolve a fixture the way Cordis does, volatile references and all. */
|
||||
function resolveConfig(patch) {
|
||||
const result = Config['~standard'].validate(patch)
|
||||
assert.equal(result.issues, undefined, `the schema refused the fixture: ${JSON.stringify(result.issues)}`)
|
||||
return result.value
|
||||
}
|
||||
|
||||
test('Config is a schemastery schema, which is what cordis requires', () => {
|
||||
assert.equal(typeof Config, 'function')
|
||||
assert.ok(Config['~standard'] !== undefined, 'expected a Standard Schema')
|
||||
@@ -281,6 +297,42 @@ test('the embedding API key is a declared secret, so the settings wire never car
|
||||
}
|
||||
})
|
||||
|
||||
test('a schema-resolved config reaches the tool as values, not as volatile references', async () => {
|
||||
// The regression: Cordis hands `apply` a reference object per volatile field,
|
||||
// so every plain-value read (`typeof settings.embeddingModel === 'string'`)
|
||||
// was false and the configured settings were silently discarded. It looked
|
||||
// healthy because `status` fell back to autodiscovery and to whatever stale
|
||||
// slopo.conf.yaml was already on disk.
|
||||
const { state } = harness(
|
||||
{ embeddingModel: 'ollama/example:latest', embeddingDimensions: 768 },
|
||||
{ configFromSchema: true },
|
||||
)
|
||||
const value = await state.tools.get('slopo').execute({ action: 'status' }, execContext('/tmp'))
|
||||
assert.deepEqual(value.details.missingSettings, [], 'the settings must not read as unset')
|
||||
assert.equal(value.ok, true)
|
||||
assert.equal(value.details.embeddingModel, 'ollama/example:latest')
|
||||
assert.equal(value.details.embeddingDimensions, 768)
|
||||
})
|
||||
|
||||
test('a settings edit lands in the running tool, because the snapshot is taken per call', async () => {
|
||||
// A volatile-only write does not re-apply the plugin — the Loader writes the
|
||||
// new value into the reference the instance already holds — so a snapshot
|
||||
// cached in `apply` would freeze this tool at its composition values forever.
|
||||
const { config, state } = harness(
|
||||
{ embeddingModel: 'ollama/first:latest', embeddingDimensions: 768 },
|
||||
{ configFromSchema: true },
|
||||
)
|
||||
const tool = state.tools.get('slopo')
|
||||
const before = await tool.execute({ action: 'status' }, execContext('/tmp'))
|
||||
assert.match(before.message, /ollama\/first:latest/)
|
||||
|
||||
const [write] = Object.getOwnPropertySymbols(config.embeddingModel)
|
||||
config.embeddingModel[write]('ollama/second:latest')
|
||||
|
||||
const after = await tool.execute({ action: 'status' }, execContext('/tmp'))
|
||||
assert.match(after.message, /ollama\/second:latest/)
|
||||
})
|
||||
|
||||
test('status reports the missing settings instead of pretending to be ready', async () => {
|
||||
const { state } = harness()
|
||||
const tool = state.tools.get('slopo')
|
||||
|
||||
Reference in new issue
Block a user