Extract reflection engine to devonian; depend on it instead

__tests__/unit/reflect/engine.test.ts

@@ -2,15 +2,14 @@ import { resolve } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { beforeAll, describe, expect, it } from 'vitest';
 import type { OpenApiDocument } from 'syncables';
+import { ReflectionEngine, InMemoryIdMap, parseMarker } from 'devonian/reflect';
 import { buildDocumentFrom } from '../../../src/sync/document.js';
 import {
   discoverResourceModel,
   type ResourceModel,
 } from '../../../src/sync/resources.js';
 import type { AuthorizedFetcher, PathParams } from '../../../src/oauth/authed-fetch.js';
-import { ReflectionEngine, type ReflectionSide } from '../../../src/reflect/engine.js';
-import { InMemoryIdMap } from '../../../src/reflect/id-map.js';
-import { parseMarker } from '../../../src/reflect/marker.js';
+import { buildReflectionSide, type SideConfig } from '../../../src/reflect/sides.js';
 
 const repoRoot = resolve(fileURLToPath(import.meta.url), '..', '..', '..', '..');
 
@@ -159,9 +158,18 @@ class FakeGitHub {
   }
 }
 
-const RETRY = { baseDelayMs: 1, maxDelayMs: 5, maxAttempts: 3 };
-
-describe('ReflectionEngine (issues)', () => {
+const RETRY: SideConfig['retry'] = { baseDelayMs: 1, maxDelayMs: 5, maxAttempts: 3 };
+
+/**
+ * This test is deliberately an integration test through reflector's real
+ * document/overlay/resource-discovery pipeline (`buildReflectionSide`), not
+ * just the engine in isolation — that coverage now lives in `devonian`'s own
+ * `__tests__/unit/reflect/engine.test.ts`, against a synthetic document.
+ * What this file verifies is that reflector's own GitHub OpenAPI document +
+ * overlay actually produces a `ReflectionSide` devonian's engine can drive
+ * correctly (localthought/atomic-plugins#6).
+ */
+describe('ReflectionEngine via reflector document/overlay (issues)', () => {
   let document: OpenApiDocument;
   let model: ResourceModel;
 
@@ -173,22 +181,24 @@ describe('ReflectionEngine (issues)', () => {
     model = discoverResourceModel(document);
   });
 
-  const sides = (fake: FakeGitHub): [ReflectionSide, ReflectionSide] => [
-    {
+  const sides = (fake: FakeGitHub) => [
+    buildReflectionSide({
       system: 'octo/a',
       document,
       model,
       auth: fake.authFor(),
       context: { owner: 'octo', repo: 'a' },
-    },
-    {
+      retry: RETRY,
+    }),
+    buildReflectionSide({
       system: 'octo/b',
       document,
       model,
       auth: fake.authFor(),
       context: { owner: 'octo', repo: 'b' },
-    },
-  ];
+      retry: RETRY,
+    }),
+  ] as const;
 
   it('reflects an original issue onto the other side with a back-link marker', async () => {
     const fake = new FakeGitHub();
@@ -199,8 +209,8 @@ describe('ReflectionEngine (issues)', () => {
 
     const [a, b] = sides(fake);
     const engine = new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     });
     const summary = await engine.reflect();
 
@@ -209,7 +219,7 @@ describe('ReflectionEngine (issues)', () => {
     expect(copies).toHaveLength(1);
     expect(copies[0]?.title).toBe('Bug');
     expect(copies[0]?.body).toContain('It broke');
-    const marker = parseMarker(copies[0]?.body ?? '');
+    const marker = parseMarker(copies[0]?.body ?? '', 'reflector');
     expect(marker).toEqual({ system: 'octo/a', kind: 'issue', id: '1' });
   });
 
@@ -223,8 +233,8 @@ describe('ReflectionEngine (issues)', () => {
     const [a, b] = sides(fake);
     const idMap = new InMemoryIdMap();
     const engine = new ReflectionEngine(a, b, idMap, {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     });
 
     await engine.reflect();
@@ -245,13 +255,13 @@ describe('ReflectionEngine (issues)', () => {
     const [a, b] = sides(fake);
 
     await new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     }).reflect();
     // Second run with a brand-new id-map (as if persistence was wiped).
     await new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     }).reflect();
 
     expect(fake.issues('octo/b')).toHaveLength(1);
@@ -267,8 +277,8 @@ describe('ReflectionEngine (issues)', () => {
     ]);
     const [a, b] = sides(fake);
     const engine = new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     });
 
     await engine.reflect();
@@ -292,8 +302,8 @@ describe('ReflectionEngine (issues)', () => {
     fake.seed('octo/b', []);
     const [a, b] = sides(fake);
     const engine = new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     });
 
     // Reflect the issue, then close the ORIGINAL on A.
@@ -326,8 +336,8 @@ describe('ReflectionEngine (issues)', () => {
     fake.seed('octo/b', []);
     const [a, b] = sides(fake);
     const engine = new ReflectionEngine(a, b, new InMemoryIdMap(), {
-      retry: RETRY,
       drainTimeoutMs: 2000,
+      namespace: 'reflector',
     });
 
     await engine.reflect(); // creates the copy issue on B
@@ -339,7 +349,7 @@ describe('ReflectionEngine (issues)', () => {
     const bComments = fake.comments('octo/b', copyNumber);
     expect(bComments).toHaveLength(1);
     expect(bComments[0]!.body).toContain('first reply');
-    expect(parseMarker(bComments[0]!.body)).toEqual({
+    expect(parseMarker(bComments[0]!.body, 'reflector')).toEqual({
       system: 'octo/a',
       kind: 'comment',
       id: String(c1.id),

docs/ai-logs/sessions/2026-09-21-atomic-plugins-issue-6-devonian-dependency.md

@@ -0,0 +1,93 @@
+# Session log — 2026-09-21
+
+- **Session:** https://claude.ai/code/session_01B2mbssSNDiu8GpKjaCFRn1
+- **Model:** Claude Sonnet 5 (`claude-sonnet-5`).
+- **Repos touched:** `localthought/reflector` (this repo, where the
+  substantive code below landed); `localthought/devonian` was the target of
+  the extraction this depends on (see that repo's own `docs/ai-logs/` for the
+  other half of this session); `localthought/atomic-plugins` was read for
+  context (the originating issue) and `syncables`/`atomic-server` were
+  surveyed but not changed.
+- **Redactions applied:** none needed — no secrets or personal information
+  appeared in this session.
+
+This log records the substantive human prompts and the assistant's
+substantive outputs, per the scoping explained in
+[`docs/ai-logs/README.md`](../README.md). It omits the coding assistant's
+internal system prompt, tool-call plumbing, and other harness scaffolding.
+
+---
+
+## Turn 1
+
+**User prompt:** "the next step for
+[localthought/atomic-plugins#6](https://github.com/localthought/atomic-plugins/issues/6)
+is to port the plugins to all use reflector / syncables / devonian, please
+work on that."
+
+**Assistant output (summarized):**
+
+After surveying all five repos in scope for the most concrete unblocked next
+step (see `devonian`'s own log for that research), found that #6's decision
+record called for reflector's own `src/reflect/` (the live, tested
+bidirectional GitHub-issue reflection engine behind `POST /api/reflect`) to
+be generalized and moved into `devonian`, with reflector depending on it
+instead of keeping its own copy — and that this had not happened yet. Did
+the `devonian`-side extraction first (see that repo's log), then this half:
+switching reflector over to depend on it.
+
+Implemented:
+
+- `src/reflect/sides.ts` (new) — `buildReflectionSide()`, the reflector-
+  specific half `devonian`'s generic engine deliberately doesn't know about:
+  builds a `devonian` `ReflectionSide` (a `syncables` client scoped to the
+  `issues` collection, a `comments(issueId)` factory, and a `setState`
+  writer) from reflector's own document/overlay/resource-model/OAuth
+  machinery. This is exactly the logic that used to live as private methods
+  (`bind()`, `commentClient()`, `setState()`) inside reflector's own
+  `ReflectionEngine` — moved out unchanged in substance, just no longer
+  bundled with the generic reflect-loop logic that went to `devonian`.
+- `src/reflect/engine.ts` — deleted; `ReflectionEngine` itself now comes from
+  `devonian/reflect`.
+- `src/reflect/marker.ts` — now a thin wrapper around `devonian/reflect`'s
+  namespaced marker codec, pinned to the `reflector` namespace so already-
+  embedded `<!-- reflector:origin ... -->` markers in production records
+  keep parsing exactly as before.
+- `src/reflect/id-map.ts` / `src/reflect/kv-store.ts` — now thin re-exports
+  from `devonian/reflect` (these were already fully generic, so nothing
+  reflector-specific was lost).
+- `src/reflect/service.ts` — `ReflectionService` keeps its exact public
+  shape (`fromConfig`/`start`/`stop`/`reflectNow`/`status`, matching
+  `src/server/app.ts` and `src/main.ts`'s existing usage unchanged) but now
+  builds two `ReflectionSide`s via `buildReflectionSide()`, constructs
+  `devonian`'s `ReflectionEngine` with `namespace: 'reflector'`, and
+  delegates the background-loop/on-demand-trigger/status behavior to
+  `devonian`'s new `ReflectionRunner` instead of reimplementing it.
+- `package.json` — added `"devonian": "^0.6.0"` as a dependency.
+- Rewrote `__tests__/unit/reflect/engine.test.ts` as an integration test
+  through reflector's *real* document/overlay/resource-discovery pipeline
+  (`buildReflectionSide` against the actual `spec/github-issues.openapi.yaml`
+  + overlay) driving `devonian`'s real engine — deliberately kept, since
+  devonian's own new engine tests (against a synthetic minimal document)
+  don't exercise reflector's actual overlay-derived `ReflectionSide`
+  construction, and this is exactly the "real engine + real overlays + fake
+  HTTP" style this project's `CLAUDE.md` asks tests to prefer.
+  `__tests__/unit/reflect/marker.test.ts`, `id-map.test.ts`, and
+  `service.test.ts` needed no changes — the public behavior they cover
+  didn't change.
+
+Verified against a locally-linked build of the `devonian` branch (not yet
+published to npm — see below): `pnpm exec tsc --noEmit`, `pnpm exec eslint .`,
+`pnpm test` (88 tests), and `pnpm test:harness` (9 tests) all pass. The local
+link was then reverted before committing (`pnpm-lock.yaml` restored to its
+prior, un-linked state) so the committed lockfile doesn't encode a private,
+machine-specific path.
+
+**Left for follow-up** (noted in the corresponding `atomic-plugins#6`
+comment): `devonian@0.6.0` needs to actually be published to npm — a
+maintainer action this session didn't take — before `pnpm install` on this
+branch will resolve the new dependency; the committed `pnpm-lock.yaml` is
+intentionally left as-is (not regenerated against a nonexistent registry
+version) rather than faking a resolution. Until that publish happens, this
+branch's `pnpm install`/CI is expected to report the lockfile as out of date
+with `package.json` — that's the correct signal, not a bug to work around.

package.json

@@ -8,6 +8,7 @@
     "node": ">= 22.11 < 23"
   },
   "dependencies": {
+    "devonian": "^0.6.0",
     "express": "~4.22",
     "js-yaml": "~4.3",
     "jszip": "~3.10",