Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions apps/desktop/src/main/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export const CHANNELS = {
findings: "check:findings",
locate: "sources:locate",
drop: "sources:drop",
/** What is sitting in `raw/_inbox/` (3.7), and taking it when asked. */
inboxWaiting: "sources:inbox-waiting",
inboxDrain: "sources:inbox-drain",

// The credential (8.3), the launcher (8.4), the content language (8.12) and
// the run 6.3 starts.
Expand All @@ -47,6 +50,20 @@ export const CHANNELS = {

/** Main → renderer, for 8.10. */
changed: "project:changed",
/** Main → renderer, for the inbox doorway of 3.7. */
inbox: "sources:inbox",
} as const;

export type Channel = (typeof CHANNELS)[keyof typeof CHANNELS];

/**
* The channels the main process pushes on, which therefore take no handler.
*
* Named as a set rather than checked one by one in `index.ts`: registering an
* `ipcMain.handle` for a push channel is harmless until the day something
* invokes it, and then it is a handler nobody wrote answering `undefined`.
*/
export const PUSH_CHANNELS: ReadonlySet<string> = new Set<string>([
CHANNELS.changed,
CHANNELS.inbox,
]);
97 changes: 91 additions & 6 deletions apps/desktop/src/main/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { app, BrowserWindow, ipcMain, shell } from "electron";
import { join, resolve } from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
import { CHANNELS, createApi, dispatch } from "./ipc.js";
import { CHANNELS, createApi, dispatch, INBOX_STABILITY_MS } from "./ipc.js";
import { PUSH_CHANNELS } from "./channels.js";
import { asDropOutcome, inboxFailure } from "./ingest.js";
import { resolveProject } from "./project.js";
import { RecorderSession, resolveRecorder, spawnTransport } from "./recorder.js";
import { applyPackagedBinaries } from "./resources.js";
import { serveQueries } from "@open-wiki/access/socket";
import { drainInbox, watchInbox, type InboxOutcome, type InboxWatcher } from "@open-wiki/access";
import { isOpenableExternally } from "../renderer/navigation.js";
import { watchProject } from "./watcher.js";

Expand Down Expand Up @@ -66,12 +69,53 @@ function createWindow(projectRoot: string | null): BrowserWindow {
},
};

const api = createApi({ projectRoot, recorder });
// 3.7 — the doorway's watcher, which arrives asynchronously; see below.
let inbox: InboxWatcher | null = null;
let closed = false;

// The window's own watcher once its initial scan has finished, so an explicit
// drain and an event cannot both read the same file and both try to register
// the same id. Standalone until then: a drain must still work in the seconds
// between the window opening and the scan completing.
const inboxDrain = (root: string): Promise<InboxOutcome[]> =>
inbox ? inbox.drain() : drainInbox(root, { stabilityMs: INBOX_STABILITY_MS });
Comment on lines +76 to +81

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Explicit inbox drain reports every outcome twice to the renderer.

inboxDrain returns inbox.drain()'s result. inbox.drain() (the InboxWatcher from packages/access/src/sources/inbox.ts) internally calls announce(outcome) for every outcome and returns the same outcomes. announce invokes the onOutcome handler registered at line 151, which pushes the outcome to the renderer over CHANNELS.inbox. The renderer already appends every CHANNELS.inbox push through its onInbox subscription (apps/desktop/src/renderer/App.tsx), then separately appends the array returned by the inboxDrain() IPC call through the onTaken prop (apps/desktop/src/renderer/App.tsx), which does not deduplicate.

The result: clicking "Add them" while the watcher is active shows each newly ingested or refused file twice in the "X of Y added" report. This only affects the explicit on-request drain; live add/change events are reported once, as expected.

Return an empty array from the watcher branch of this helper, since those outcomes are already delivered through onOutcome.

🐛 Proposed fix to stop the double delivery
   const inboxDrain = (root: string): Promise<InboxOutcome[]> =>
-    inbox ? inbox.drain() : drainInbox(root, { stabilityMs: INBOX_STABILITY_MS });
+    inbox
+      ? // `inbox.drain()` already reports every outcome through `onOutcome`
+        // (`CHANNELS.inbox`) below. Returning them here too would hand the
+        // renderer the same outcomes a second time.
+        inbox.drain().then(() => [])
+      : drainInbox(root, { stabilityMs: INBOX_STABILITY_MS });

Consider adding a regression test in apps/desktop/tests/sources.spec.ts that wires a fake InboxControl whose drain() both resolves outcomes and simulates the onOutcome push, then asserts the resulting drop report has no duplicate entries — the current tests only exercise inboxDrain() without a live deps.inbox, so this path is untested.

Also applies to: 147-169

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/index.ts` around lines 76 - 81, Update the inboxDrain
helper so the active watcher branch still invokes inbox.drain() but returns an
empty outcome array, since the watcher’s onOutcome handler already delivers
those results to the renderer; preserve drainInbox(root, ...) and its returned
outcomes when no watcher exists. Add a regression test covering a fake
InboxControl whose drain both emits and resolves outcomes, verifying the
resulting report contains no duplicates.


const api = createApi({
projectRoot,
recorder,
...(projectRoot ? { inbox: { drain: () => inboxDrain(projectRoot) } } : {}),
});
for (const channel of Object.values(CHANNELS)) {
if (channel === CHANNELS.changed) continue; // main → renderer only
if (PUSH_CHANNELS.has(channel)) continue; // main → renderer only
ipcMain.handle(channel, (_event, ...args: unknown[]) => dispatch(api, channel, args));
}

/**
* Tell the window something.
*
* **Buffered until the document has loaded.** `webContents.send` before that
* is dropped on the floor with no queue and no error, and the things pushed
* here are reports — a file that arrived, a watcher that died. A report that
* silently goes nowhere is the failure this channel exists to prevent.
*/
let loaded = false;
const waiting: Array<{ channel: string; payload: unknown }> = [];
const send = (channel: string, payload: unknown): void => {
if (window.isDestroyed()) return;
if (!loaded) {
waiting.push({ channel, payload });
return;
}
window.webContents.send(channel, payload);
};
window.webContents.once("did-finish-load", () => {
loaded = true;
for (const message of waiting) {
if (!window.isDestroyed()) window.webContents.send(message.channel, message.payload);
}
waiting.length = 0;
});

// 9.14 — the CLI asks here rather than starting a process, when this
// window already has the project open. Read and validate only; the socket
// never carries a write.
Expand All @@ -80,13 +124,54 @@ function createWindow(projectRoot: string | null): BrowserWindow {
// 8.10 — whoever wrote it, the screen follows. A launcher window has no
// project to watch.
const watcher = projectRoot
? watchProject(projectRoot, (change) => {
if (!window.isDestroyed()) window.webContents.send(CHANNELS.changed, change);
})
? watchProject(projectRoot, (change) => send(CHANNELS.changed, change))
: null;

// 3.7 — the doorway. An agent that fetched something writes it into
// `raw/_inbox/` with its own tools, and it becomes a source through the same
// registration a dropped file goes through. This is the process that holds
// the watcher open; until it existed the doorway only worked when something
// called `drainInbox` by hand.
//
// Started asynchronously — `watchInbox` waits for chokidar's initial scan, and
// a window that blocked on it would be a window that does not open. So the
// handle arrives late, and a window closed before it does has to close it
// anyway or the watcher outlives its window.
//
// **`ingestExisting: false`.** What is already in the doorway when a window
// opens is listed and left alone; only what arrives while it is open is taken
// on sight. `raw/` comes with a clone, so the alternative is a repository
// shipping `raw/_inbox/x.pdf` and this application parsing a stranger's bytes
// in the main process — and deleting the file out of the user's tree — before
// anybody clicked anything.
if (projectRoot) {
void watchInbox(
projectRoot,
{
onOutcome: (outcome) => send(CHANNELS.inbox, asDropOutcome(outcome)),
onError: (error) => send(CHANNELS.inbox, inboxFailure(error)),
},
{ ingestExisting: false },
)
.then((started) => {
// `return`, not `void`: a discarded promise here escapes the `.catch`
// below and becomes an unhandled rejection in the main process.
if (closed) return started.close();
inbox = started;
return undefined;
})
.catch((error: unknown) => {
send(
CHANNELS.inbox,
inboxFailure(error instanceof Error ? error : new Error(String(error))),
);
});
}

window.on("closed", () => {
closed = true;
void watcher?.close();
void inbox?.close();
queries?.close();
session?.dispose();
for (const channel of Object.values(CHANNELS)) ipcMain.removeHandler(channel);
Expand Down
32 changes: 32 additions & 0 deletions apps/desktop/src/main/ingest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
uploadPdfSource,
uploadTextSource,
TakenIdError,
type InboxOutcome,
} from "@open-wiki/access";

/**
Expand Down Expand Up @@ -96,3 +97,34 @@ export async function ingestDrop(
for (const path of paths) outcomes.push(await ingestFile(projectRoot, path));
return outcomes;
}

/**
* What the inbox watcher saw (plan 3.7), said the way a drop says it.
*
* The doorway and the drop zone are two ways into the same registration, so
* they are worth reporting through one shape: the window already knows how to
* show "three of four added, and here is the fourth". `removed` is dropped on
* the way through — whether the file left the doorway is the watcher's
* bookkeeping, and a reader who was not told the doorway exists cannot be told
* something stayed in it.
*/
export function asDropOutcome(outcome: InboxOutcome): DropOutcome {
return outcome.ok
? { name: outcome.name, ok: true, id: outcome.id }
: { name: outcome.name, ok: false, reason: outcome.reason };
}

/**
* A doorway that stopped working, as an outcome (plan 3.7).
*
* Reported rather than logged, because a watcher that goes quiet is
* indistinguishable from an inbox nobody is using — and the failure it hides is
* material an agent believes it handed over.
*/
export function inboxFailure(error: Error): DropOutcome {
return {
name: "raw/_inbox",
ok: false,
reason: `the inbox stopped being watched: ${error.message}`,
};
}
46 changes: 44 additions & 2 deletions apps/desktop/src/main/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
type SaveInput,
type SaveResult,
} from "./edit.js";
import { ingestDrop, type DropOutcome } from "./ingest.js";
import { asDropOutcome, ingestDrop, type DropOutcome } from "./ingest.js";
import { drainInbox, listInbox, type InboxOutcome } from "@open-wiki/access";
import {
createProject,
credentialState,
Expand All @@ -39,6 +40,7 @@ import {
locateCitation,
sourceDetail,
sourcesOfPage,
type PageSource,
type SourceLocation,
type SourceRow,
} from "./sources.js";
Expand Down Expand Up @@ -82,10 +84,24 @@ export interface Deps {
*/
projectRoot: string | null;
recorder?: RecorderControl;
/** The window's inbox watcher (3.7), when its initial scan has finished. */
inbox?: InboxControl;
/** Injected so a test does not depend on today's date. */
now?: () => Date;
}

/** What a window offers of its inbox watcher — draining, and nothing else. */
export interface InboxControl {
drain(): Promise<InboxOutcome[]>;
}

/**
* How long a file's size must hold steady before an explicit drain reads it.
* The same wait `watchInbox` applies, because a file half-copied is half a
* source whichever path reached it.
*/
export const INBOX_STABILITY_MS = 400;

/** What a window reports when nothing is being recorded. */
export const IDLE_STATUS: RecorderStatus = {
state: "idle",
Expand Down Expand Up @@ -131,11 +147,21 @@ export interface DesktopApi {
undo(id: string): void;

sourceDetail(id: string): SourceRow;
sourcesOfPage(slug: string): string[];
sourcesOfPage(slug: string): PageSource[];
retitle(id: string, title: string): void;
findings(): Finding[];
locate(id: string, fragment: string): SourceLocation;
drop(paths: readonly string[]): Promise<DropOutcome[]>;
/**
* What is waiting in the doorway (plan 3.7), and taking it.
*
* **Asked for rather than pushed**, which is what makes the report reliable:
* a window reports live arrivals over `CHANNELS.inbox`, but what was already
* there when the window opened would be announced before the renderer had
* subscribed and vanish. The renderer asks instead, whenever it likes.
*/
inboxWaiting(): string[];
inboxDrain(): Promise<DropOutcome[]>;

credential(): CredentialState;
saveCredential(input: SaveCredentialInput): Promise<CredentialCheck>;
Expand Down Expand Up @@ -217,6 +243,18 @@ export function createApi(deps: Deps): DesktopApi {
findings: () => findings(root()),
locate: (id, fragment) => locateCitation(root(), id, fragment),
drop: (paths) => ingestDrop(root(), paths),
inboxWaiting: () => listInbox(root()),
// Through the window's watcher when there is one, so an explicit drain and
// an event cannot both read the same file and both try to register the same
// id. Standalone otherwise — a drain must still work in the window between
// opening and the watcher finishing its initial scan.
async inboxDrain() {
const projectRoot = root();
const outcomes = deps.inbox
? await deps.inbox.drain()
: await drainInbox(projectRoot, { stabilityMs: INBOX_STABILITY_MS });
return outcomes.map(asDropOutcome);
},

credential: () => credentialState(root()),
saveCredential: (input) => saveCredential(root(), input),
Expand Down Expand Up @@ -312,6 +350,10 @@ export async function dispatch(
// The renderer hands over paths Chromium gave it for a drop. Anything
// that is not a string is not a path.
return api.drop((Array.isArray(args[0]) ? args[0] : []).filter((p) => typeof p === "string"));
case CHANNELS.inboxWaiting:
return api.inboxWaiting();
case CHANNELS.inboxDrain:
return api.inboxDrain();

default:
throw new Error(`unknown channel "${channel}"`);
Expand Down
9 changes: 9 additions & 0 deletions apps/desktop/src/main/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ const api = {
findings: () => ipcRenderer.invoke(CHANNELS.findings),
locate: (id: string, fragment: string) => ipcRenderer.invoke(CHANNELS.locate, id, fragment),
drop: (paths: readonly string[]) => ipcRenderer.invoke(CHANNELS.drop, paths),
inboxWaiting: () => ipcRenderer.invoke(CHANNELS.inboxWaiting),
inboxDrain: () => ipcRenderer.invoke(CHANNELS.inboxDrain),

credential: () => ipcRenderer.invoke(CHANNELS.credential),
saveCredential: (input: unknown) => ipcRenderer.invoke(CHANNELS.saveCredential, input),
Expand Down Expand Up @@ -82,6 +84,13 @@ const api = {
ipcRenderer.on(CHANNELS.changed, listener);
return () => ipcRenderer.removeListener(CHANNELS.changed, listener);
},

/** 3.7 — something arrived through `raw/_inbox/`, or the doorway broke. */
onInbox: (handler: (outcome: unknown) => void) => {
const listener = (_event: unknown, outcome: unknown): void => handler(outcome);
ipcRenderer.on(CHANNELS.inbox, listener);
return () => ipcRenderer.removeListener(CHANNELS.inbox, listener);
},
};

contextBridge.exposeInMainWorld("ow", api);
Expand Down
Loading
Loading