v1.2.0: advisor is per session and off by default (new and existing sessions); old global setting ignored

test/advisor.test.mjs

@@ -6,14 +6,23 @@ import path from 'node:path'
 import { Readable } from 'node:stream'
 import * as plugin from '../index.js'
 
-// Settings persist to <DSH_PROFILE_DIR>/omp-advisor.json; keep tests away from the real profile.
+// Settings persist to <DSH_PROFILE_DIR>/omp-advisor/sessions.json; keep tests away from the real profile.
 const PROFILE = fs.mkdtempSync(path.join(os.tmpdir(), 'omp-advisor-test-'))
 process.env.DSH_PROFILE_DIR = PROFILE
-const SETTINGS = path.join(PROFILE, 'omp-advisor.json')
+const SETTINGS = path.join(PROFILE, 'omp-advisor', 'sessions.json')
 test.after(() => fs.rmSync(PROFILE, { recursive: true, force: true }))
 
-function harness(replies, config = {}) {
+/**
+ * @param config  plugin config overrides
+ * @param opts    { id: session id (default 'session-a'), on: pre-enable this session (default true) }
+ */
+function harness(replies, config = {}, opts = {}) {
+  const sessionId = opts.id ?? 'session-a'
   fs.rmSync(SETTINGS, { force: true }) // live settings load in apply(): never leak between tests
+  if (opts.on !== false) {
+    fs.mkdirSync(path.dirname(SETTINGS), { recursive: true })
+    fs.writeFileSync(SETTINGS, JSON.stringify({ [sessionId]: { enabled: true, at: 1 } }))
+  }
   const listeners = new Map()
   const on = (n, f) => { listeners.set(n, [...(listeners.get(n) ?? []), f]); return () => listeners.set(n, listeners.get(n).filter((x) => x !== f)) }
   const prompts = []
@@ -34,7 +43,7 @@ function harness(replies, config = {}) {
     deriveEventMessage: (e) => e.msg,
   }
   const steered = []
-  const agent = { session, ctx: { on }, steer: (m) => steered.push(m) }
+  const agent = { id: sessionId, session: { ...session, header: { ...session.header, id: sessionId } }, ctx: { on }, steer: (m) => steered.push(m) }
   const routes = new Map()
   const webServer = { register: (r) => { routes.set(r.path, r.handler); return () => routes.delete(r.path) } }
   const connection = { requestRejection: (req) => (req.headers?.['x-reject'] ? 401 : undefined) }
@@ -52,11 +61,12 @@ function harness(replies, config = {}) {
   const stopTurn = async () => { for (const f of listeners.get('agent/turn-stopping') ?? []) await f({ agent, turn: 1 }) }
   const add = (role, content) => events.push({ msg: { role, content } })
   /** Call the settings route like the web client does. */
-  const http = async (method, body, headers = {}) => {
+  const http = async (method, body, headers = {}, sid = sessionId) => {
     const handler = routes.get('/omp-advisor/settings')
     const req = Readable.from(body === undefined ? [] : [JSON.stringify(body)])
     req.method = method
     req.headers = headers
+    req.url = sid === null ? '/omp-advisor/settings' : `/omp-advisor/settings?session=${encodeURIComponent(sid)}`
     let status = 200; let out = ''
     const res = { statusCode: 200, setHeader() {}, end: (s) => { status = res.statusCode; out = s ?? '' } }
     await handler(req, res)
@@ -111,20 +121,56 @@ test('minLevel filters notes and duplicates are suppressed', async () => {
   assert.equal((await h.preStep(4)).messages.length, 0) // identical concern suppressed
 })
 
-test('settings route: GET/POST, rejection, persistence', async () => {
-  fs.rmSync(SETTINGS, { force: true })
-  const h = harness([])
+test('settings route: per-session GET/POST, rejection, missing session, persistence', async () => {
+  const h = harness([], {}, { on: false })
   assert.equal((await h.http('GET', undefined, { 'x-reject': '1' })).status, 401)
+  assert.equal((await h.http('GET', undefined, {}, null)).status, 400) // no ?session=
+  assert.equal((await h.http('GET', undefined, {}, '../etc')).status, 400) // invalid id
   const g = await h.http('GET')
   assert.equal(g.status, 200)
-  assert.equal(g.json.enabled, true)
+  assert.equal(g.json.enabled, false) // off by default
+  assert.equal(g.json.sessionId, 'session-a')
   const p = await h.http('POST', { enabled: true, provider: 'omni', model: 'cheap-model', reasoningEffort: 'low' })
   assert.equal(p.json.model, 'cheap-model')
   assert.equal(p.json.reasoningEffort, 'low')
-  assert.deepEqual(JSON.parse(fs.readFileSync(SETTINGS, 'utf8')), { enabled: true, provider: 'omni', model: 'cheap-model', reasoningEffort: 'low' })
+  const stored = JSON.parse(fs.readFileSync(SETTINGS, 'utf8'))
+  assert.deepEqual(Object.keys(stored), ['session-a'])
+  assert.equal(stored['session-a'].model, 'cheap-model')
+  assert.equal(typeof stored['session-a'].at, 'number')
   assert.equal((await h.http('PUT', {})).status, 405)
 })
 
+test('default is off: no review for a session that never turned the advisor on', async () => {
+  const h = harness(['{"verdict":"concern","message":"should never be asked"}'], {}, { on: false })
+  h.add('user', [{ type: 'text', text: 'task' }])
+  assert.equal((await h.preStep(2)).messages.length, 0)
+  assert.equal(h.prompts.length, 0)
+})
+
+test('the old global omp-advisor.json is ignored (existing sessions stay off)', async () => {
+  fs.writeFileSync(path.join(PROFILE, 'omp-advisor.json'), JSON.stringify({ enabled: true, model: 'x' }))
+  const h = harness(['{"verdict":"ok","message":""}'], {}, { on: false })
+  h.add('user', [{ type: 'text', text: 'task' }])
+  await h.preStep(2)
+  assert.equal(h.prompts.length, 0)
+  assert.equal((await h.http('GET')).json.enabled, false)
+  fs.rmSync(path.join(PROFILE, 'omp-advisor.json'), { force: true })
+})
+
+test('sessions are independent: turning one on does not affect another', async () => {
+  const h = harness(['{"verdict":"ok","message":""}', '{"verdict":"ok","message":""}'], {}, { on: false })
+  h.add('user', [{ type: 'text', text: 'task' }])
+  await h.http('POST', { enabled: true, model: 'other-model', provider: 'omni' }, {}, 'session-b')
+  await h.preStep(2) // this agent is session-a
+  assert.equal(h.prompts.length, 0)
+  assert.equal((await h.http('GET', undefined, {}, 'session-b')).json.model, 'other-model')
+  assert.equal((await h.http('GET')).json.enabled, false)
+  await h.http('POST', { enabled: true })
+  await h.preStep(3)
+  assert.equal(h.prompts.length, 1)
+  assert.equal(h.prompts[0].model, 'main-model') // session-a uses the agent model, not session-b's
+})
+
 test('UI choice applies on the next review without restart; off disables reviews', async () => {
   fs.rmSync(SETTINGS, { force: true })
   const h = harness(['{"verdict":"ok","message":""}', '{"verdict":"ok","message":""}'])