Repository navigation
Upstream/model catalog json - #339
Closed
aaroncoville wants to merge 30 commits into
Closed
aaroncoville wants to merge 30 commits into
aaroncoville wants to merge 30 commits into
Conversation
An isolated worktree does not include the base checkout's dependencies, so project validation can fail before it checks an agent's change. Link the dependencies after creating the worktree, while preserving existing entries and allowing the spawn to continue if linking fails. Before deciding whether a worker worktree is dirty, remove only a dependency link that resolves to the base checkout's node_modules. This keeps unignored links from preventing automatic cleanup while preserving real directories and foreign links. Use Windows junctions so directory links work without elevated privileges. The regression coverage verifies link creation, skip and failure paths, cleanup without a .gitignore, Windows link selection, and that git worktree removal leaves the base dependencies intact.
Tearing down a PTY forgets the agent's circuit-breaker state but leaves its telemetry sessions in place. Because live usage is summed across every retained session for an agent id, a replacement agent reusing that id starts with its dead predecessor's tokens already counted against it, and a per-agent token cap can trip on spend that belongs to a session that no longer exists. Forget the agent's live telemetry sessions alongside the breaker state, so a replacement starts from its own usage. Only the in-memory live accumulation is cleared; the on-disk cost ledger and the transcript fallback are untouched, so lifetime spending is still reported.
Toggling a default MCP server writes the new grant to disk, but the control goes on showing the old one. Leaving the Connections section and coming back does not clear it either, so the panel can claim a server is off while it is granted on disk — the wrong direction for a consent control to be wrong in. The app loads its configuration once at start-up and passes that snapshot down, and the toggle discarded the value `updateConfig` returns, so the component had nothing newer to render than the prop it was given. Remounting the panel re-seeded it from that same unchanged snapshot, which is why leaving and returning did not help. Read the persisted configuration on mount and again after `updateConfig`, so the control renders what the config says actually landed rather than what was intended. A failed read keeps the prop-seeded value rather than guessing, and a failed write leaves the control showing the unchanged grant. Changes: - McpDefaultsSettings.tsx: seed from the prop, re-read on mount, and re-read the persisted configuration after each write - mcpToggleLogic.ts: extract the toggle's merge and re-read as pure functions so both the success and failure paths are testable - test/render-hooks.cjs: a small React host for node:test, so a component can be mounted and driven without jsdom — a pure-function test alone cannot show the component still calls it - test/load-ts.cjs: resolve .tsx and use the automatic JSX runtime - test/mcp-toggle-component.test.cjs, test/mcp-toggle-state.test.cjs: cover the merge, the re-read, a failed write, and the remount that used to bring the stale value back
The evidence check reads each section with a lazy capture that ends at `$` under the `m` flag, so it stops at the first line end it reaches. A heading followed by a blank line yields an empty section, and in practice only the single line directly beneath a heading is ever read. The pull request template puts an instructional comment and a blank line under `Before` and `After`, and comments are stripped before matching — so a description that follows the template exactly is reported as having no evidence, and the failure comment asks the author to do what they already did. Stop the capture at the next heading line start or at true end of input. An immediately adjacent heading then ends a section at width zero, so an empty heading cannot be satisfied by the media under a later one. Strip fenced code blocks alongside HTML comments. Test output pasted under a heading otherwise ends the section at its first `#` line, hiding an image below it, and Markdown that looks like media inside a fence otherwise counts as evidence though it never renders as media. Evidence still has to sit under its own heading: a section stops where the next one starts, and a heading with nothing under it still fails.
fix(ci): scope PR evidence to visible sections
Consolidates the section-by-section approved design: MemoryBackend interface extracted from MemoryManager, MemPalace and Hindsight in-process adapters, the backend-neutral hive-memory shim, config migration, degraded-mode semantics, and the four-commit rollout.
fix: show the MCP consent grant that actually landed
Pure extraction. MemoryManager keeps the backend-neutral mine loop, mtime tracking, and single-writer serialization; every mempalace-specific behavior (bin discovery, env + device pin, quarantine backoff, palace reaping, CLI search/wake-up) moves verbatim into the adapter. No behavior change: the untouched test suite is the proof.
Switching between agents on the office floor eventually blacks out a terminal or
the scene. Chromium allows only a limited number of live WebGL contexts (~16)
and silently evicts the oldest — often the floor's own — once that cap is passed.
Each switch leases a webgl renderer for the terminal being shown and releases the
one for the terminal being hidden. @xterm/addon-webgl's dispose() tears down its
renderer and removes its canvases but never calls loseContext(), so the
underlying GPU context lingers past the addon that owned it until garbage
collection (xterm/xterm.js#6068, still open). On a floor that is switched
repeatedly the orphaned contexts accumulate and blow the cap.
After disposing the addon, explicitly release its context via
getExtension('WEBGL_lose_context').loseContext(). The context is read from the
terminal's own element before dispose (dispose may detach the canvas) and matched
by a live webgl2 context, so the release is scoped to that one terminal and never
touches another terminal's or the scene's context. Best-effort and idempotent: a
no-op when the canvas, the extension, or a live context is absent.
Measured with a probe counting live WebGL2 contexts across 30 renderer
switch-cycles on @xterm/addon-webgl 0.19.0: before, the count climbs and plateaus
at Chromium's cap (where eviction — the crash — begins); after, it stays flat at
zero.
…ardown fix: release the WebGL context when a terminal's renderer is torn down
…encies fix: make dependencies available in isolated worktrees
Recall returns per-result scores as a `scores` object (`final` is the value the ranking is based on), not as a flat `score` field. The shim read `hit.score`, which is always undefined against a real server, so every recalled memory printed without its score and there was no way to tell a strong hit from a weak one. Read `scores.final` and keep the flat `score` as a fallback so simpler responses still render.
…d picker Semantic memory could only ever be the local MemPalace CLI, so a hive spread over more than one machine had no way to share what its agents remembered. Adds a second backend behind the existing MemoryBackend interface: a Hindsight server reached over HTTP. Every request shape was taken from the server's OpenAPI document rather than guessed, and several differ from what a reasonable guess would produce: retain takes `content` (not `text`) per item, recall's result budget is `max_tokens` and there is no `top_k` at all, a recalled hit carries `scores.final` rather than a flat score, and a bank is created by PUT on its own id because no POST for creating one exists. Mining derives each document id from a hash of the agent, the section heading and its body, so re-mining an unchanged memory.md updates the same documents instead of stacking duplicate copies in the bank. Settings gain a backend choice alongside the existing on/off switch. They have never been stored as one object — they are flat config keys — so migrateMemorySettings is a read-time normaliser rather than a rewrite-on-disk migration: a config written by an older build still loads, and adding a backend does not invalidate it. Switching backend clears the mined-file cache, because the new backend holds none of the old one's memories and every agent's notes have to be mined into it from scratch. Two things a local CLI backend gets for free need care for a remote one: availability is only knowable after an async probe (so the manager arms the backend rather than waiting for it to report available, which would never happen), and a server URL is not a filesystem path (so status reports it as `location` and leaves `palacePath` null). The panel's "Test connection" result is rendered straight from the IPC response, including the address it actually reached, so a verdict about one server can never be shown beside an address that has since been edited. The renderer has no test harness, so the UI itself is covered by typecheck and a production build only; the probe behind its button is unit-tested against a stub server.
The shim is a standalone script that learns its backend and its owning agent entirely from the environment. Nothing set either variable, so it always took its "no backend configured" path: every agent running `hive-memory search` got the unavailable line back, however healthy the memory backend actually was, and the prompt instructing them to run it pointed at a command that could not work. Each backend now names itself in the environment it injects into agent spawns, and the hive hands each agent its own id alongside AGENT_ID so recalls are scoped to that agent's memories.
The manager treats every backend interchangeably, so the parts it relies on are a contract rather than each adapter's private business. One parametrised spec now runs over both, covering the awkward halves that are easy to get right for one backend and wrong for the other: a broken backend reports failure instead of throwing (a throw aborts the whole mine pass and takes the other agents' memories down with it), status answers before a hive home has been chosen because the settings panel polls it then, and each backend names itself in the environment agents are spawned with.
The Codex preset recommends gpt-5-codex, which is not offered by model/list verified under ChatGPT login on 2026-08-25. A recorded first-message error also rejects that model under ChatGPT login, so a new orchestrator can fail after launch. Use gpt-5.6-sol, the default from that probe, and refresh the curated picker and add-agent example to the same observed catalog. Model availability is provider-controlled; the catalog verification is scoped to ChatGPT login on that date.
fix(codex): use the verified Codex default
fix: reset usage when a PTY exits
…ttended A Codex worker spawned without a human at the keyboard would sit at its very first turn and never start. Codex gates every working directory behind an interactive "do you trust this directory?" prompt, and the answer is persisted per config home as a `[projects."<dir>"] trust_level = "trusted"` table. Each worker gets its OWN isolated CODEX_HOME (so the user's ~/.codex is never mutated), and that home is generated fresh: it has never answered the prompt for anything. So every worker met the prompt on turn one, and an unattended one had nobody to press Yes. Pre-seeding that table is the only suppression that exists. No flag or environment variable skips the prompt — `--dangerously-bypass-approvals-and- sandbox` governs command approval, not directory trust — and a `-c projects…` override is not persisted, so it does not read as an answer either. So append the trust entry to the generated config.toml before the CLI starts, seeded from the user's config as before, keyed on: - the FINAL working directory. Git isolation has already resolved by the time spawn prep runs, so this is the worker's own worktree when one was created and the original repo when isolation fell back — always the directory the CLI will actually open in, and one entry is enough. - its canonical, realpath-resolved spelling. Codex resolves the directory before looking it up, so an entry keyed `/tmp/x` never matches its `/private/tmp/x` and the prompt comes back. The path goes through a TOML basic-string escaper rather than string interpolation: a directory name may legally contain a quote or a backslash on POSIX, and an unparseable config.toml would be a worse failure than the prompt. The short symlinked home that remote control binds its control socket through needs nothing extra — it aliases the same directory, so it sees the same file.
The node:test TypeScript loader hands every resolved local import to ts.transpileModule, including `.json` ones. TypeScript cannot emit for JSON input and throws "Debug Failure. Output generation failed", so any module that imports a JSON file was untestable, and the failure surfaced as a crashed test file rather than a readable error. The app compiles with resolveJsonModule and the bundler parses JSON imports directly, so do the same here: JSON.parse the file instead of transpiling it.
Every provider's model picker was a hardcoded ModelOption[] in the renderer store, so shipping a model — one string — meant editing renderer source, type-checking and rebuilding. The arrays also could not say which releases a model belongs to: a build whose CLI never shipped a model still offered it, and a retired model stayed in the picker forever. The twelve arrays now live in src/shared/modelCatalog.json, ported verbatim, and each entry carries inclusive minAppVersion/maxAppVersion bounds. Every ported entry has null for both, so this build offers exactly what it offered before. The catalog is imported at BUILD time — no fs, no network, offline-safe — and modelsForProvider filters it by the running app version, which electron-vite already inlines into the renderer as __APP_VERSION__ for the update badge (esbuild folds the accessor down to `return "0.4.5"`, so no round trip to main is involved). An unparseable bound or version is ignored rather than hiding the model: a picker that silently loses every model is far worse than one offering a model the CLI cannot run, and the command field stays editable either way. Adding a model is now a one-line JSON edit, and a release can introduce or retire one without touching TypeScript. The comments that explained why each list looked the way it did are kept above the import, since JSON cannot carry them.
A symlink inside the selected workspace let the file IPC read and overwrite files outside it. `<workspace>/notes.txt -> ../elsewhere/target` reads as an ordinary in-workspace relative name, and the read/write/list call then reached the external target. Root cause: the shared containment guard was lexical only. `resolve`, `normalize` and `relative` are string math and know nothing about symlinks, while the `readFile`/`writeFile`/`readdir` that runs afterwards is resolved by the kernel, which does follow them. Every consumer of the guard was affected — the text read, the binary read, the write, the directory listing, and both git path operations — because they all share it. The guard becomes an async `safeResolve` that all of them call, in three layers: 1. canonicalize the workspace root and the target with `realpath` and re-check containment on the canonical paths. The not-yet-existing tail of a path is re-attached to its deepest existing ancestor, so creating a new file in a real workspace directory still works. 2. reject any path component that is a symlink, via an `lstat` walk. Not redundant with (1): `realpath` throws ENOENT on a dangling symlink, so canonicalization alone treats it as a name to be created and the write follows the link and creates the file at its external target. 3. open reads and writes with `O_NOFOLLOW` (plus `O_NONBLOCK`, so a FIFO cannot park the open and with it the IPC call), so containment does not rest on the path still meaning the same thing between the check and the open. Those flags are POSIX-only, which is why (2) is a check in its own right and not merely a pre-filter. Layer 3 is why the git diff's working-side read had to change shape rather than just gain a call to the guard. It was a plain `stat` + `readFile` on the cleared path, which is a second and third resolution of a name the kernel is free to look up differently — anything that swaps that final component for a symlink after the check is what the read actually gets, and the external file then travels to the renderer as diff text. It now opens through the shared `openForRead` and stats through the returned handle, like every other read of a confined path, so the flags are stated in exactly one place. The boundary is the WORKSPACE, not symlinks. A link whose target exists and is itself inside the workspace is followed, and what comes back is the canonical path of that target: it reaches nothing the caller could not already reach by the target's real name, and refusing it would make an ordinary `node_modules` or monorepo checkout unbrowsable for no gain. What is refused is a link that leaves the workspace, and a dangling link, whose target does not exist yet and so cannot be shown to land inside it. Both directions of that are pinned by tests, so the policy cannot be flipped by accident later. The lexical form is no longer exported — a caller reaching for it would reintroduce exactly this bug.
HIGH security: confine the file IPC to the workspace when symlinks are involved
fix(codex): pre-trust a worker's working directory so it can boot unattended
Contributor
|
✅ Evidence received. Before and after are both attached. Thanks — this is what makes a PR reviewable in one pass. |
chaitanyagiri
added a commit
that referenced
this pull request
Aug 26, 2026
The twelve hardcoded ModelOption[] arrays leave the renderer store for src/shared/modelCatalog.json, imported at BUILD time (no fs, no network, offline-safe) and filtered per running build through inclusive minAppVersion/maxAppVersion bounds. Every ported entry ships null/null, so the picker is byte-identical today: verified all 12 providers, 76 entries, exact id+label match against the arrays this replaces. Adding a model becomes a one-line JSON edit instead of a renderer source change. The codex sandbox flag is untouched: this PR's only change to agentProvider.ts is a two-line comment rewording, and autoModeFlag stays '-a never -s workspace-write'.
# Conflicts: # src/renderer/src/store/config.ts
chaitanyagiri
added a commit
that referenced
this pull request
Aug 27, 2026
…ward or 4) RELEASE.md contradicted itself: line 16 said "four merged pull requests" (only tonight's four integration merges), the Thanks said "23 community pull requests landed in this release" (the 0.4.5 number copied forward), and the historical tail correctly says those 23 landed in v0.4.5. Recounted from the actual 0.4.6 changeset (main b9f34e7..release tip, linear so it is exactly this release). Method: a community PR is one authored by someone other than the founder (@chaitanyagiri) whose work landed here, whether merged or re-implemented. That gives 16 community PRs from 13 contributors: #156 #205 #213 #225 #242 #243 #248 #270 #271 #282 #284 #286 #310 #317 #323 #339. Fifteen landed as merges; #213 (Arabic) landed by re-implementation, which is why a merge-only count returns 15 and misses it. Line 16 and the Thanks now both say 16, the Thanks names all 13 contributors inline so the credit matches the count, and the 0.4.5 sections (its intro and its historical table) are left at 23 untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDbmAwrgtGskFGCRkYZBx4
Collaborator
|
Shipped in v0.4.6. Your commits Note this PR's branch also carried This closes rather than shows as merged because the commits went in through the 0.4.6 release branch instead of this PR's branch. https://github.com/chaitanyagiri/munder-difflin/releases/tag/v0.4.6 Thank you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Every provider's model picker was a hardcoded
ModelOption[]in the renderer store, soshipping a model — one string — meant editing renderer source, type-checking and
rebuilding. The arrays also could not say which releases a model belongs to: a build
whose CLI never shipped a model still offered it, and a retired model stayed in the
picker forever.
The twelve arrays now live in
src/shared/modelCatalog.json, ported verbatim, each entrycarrying inclusive
minAppVersion/maxAppVersionbounds. Every ported entry hasnullfor both, so this build offers exactly what it offered before — the version fields are the
mechanism, not a behaviour change. The catalog is imported at build time (no fs, no
network, offline-safe) and
modelsForProviderfilters it by the running app version, whichelectron-vite already inlines into the renderer as
__APP_VERSION__for the update badge.Adding a model is now a one-line JSON edit.
Type of change
Evidence
No visible UI change: the pickers offer the same models, in the same order, with the same
labels. The evidence is the test suite going red → green, plus proof that the built bundle
really carries the version into the filter.
Reproduce both halves:
Before
The catalog-backed API does not exist, so the version-bound assertions fail. The three
tests that do pass are the ones pinning today's picker output — they pass against the
hardcoded arrays, which is what makes them a faithful record of what shipped:
After
Same file, same assertions — the picker-output tests still pass (so nothing a user sees
changed), and the version filter now works:
And the production bundle proves the version reaches the filter — esbuild folds the
build-time define straight through, so there is no round trip to main:
How I tested it
npm run typecheck— 0 errors (node + web).npm run test:focused— 560 of 560 pass; the base commit is 552 of 552, and the8 new tests are the whole difference.
npm run build— succeeds; bundle checked as above.minAppVersioncheck, dropping themaxAppVersioncheck, editing one catalog label, and making the version accessorignore
__APP_VERSION__each turn the matching test red.Checklist
npm run typecheckpasses.npm run test:focusedpasses.npm run buildsucceeds.commented-out code, or unrelated formatting churn in it.
DESIGN.md/tokens.ts— no ad-hoc colors, spacing,or fonts. (No UI in this change.)
ATTRIBUTION.md. (No art.)How the catalog works & how to maintain it
The provider model lists live in
src/shared/modelCatalog.jsonand are importedat build time.
modelsForProvider(provider)returns that provider's list,filtered at runtime by the running app version.
There are two independent "version" concepts — they do not map to each other:
"version"is the file schema version. Nothing reads it today;it exists so that if the file's structure ever changes, code can branch on
it. Bump it only when the shape of the JSON changes — never for adding or
removing a model.
minAppVersion/maxAppVersionare the app-version bounds(inclusive;
null= unbounded). A model is shown only when the running appversion falls within its bounds. Every model currently ships
null/null, sothe picker is identical to the previous hardcoded lists.
Add a new model
Add an entry under the provider in
modelCatalog.json:{ "id": "gpt-6-nova", "label": "GPT-6 Nova", "minAppVersion": null, "maxAppVersion": null }If the model only works from a future release, set
minAppVersionto thatrelease (e.g.
"0.5.0") so users on older builds — whose CLI can't run it —don't see it. Otherwise leave both bounds
null.Retire an old model
Set
maxAppVersionto the last release that should still offer it:{ "id": "gpt-5.6-sol", "label": "GPT-5.6 Sol", "minAppVersion": null, "maxAppVersion": "0.4.9" }Builds newer than
0.4.9stop offering it, while users still on0.4.xkeepseeing it. To remove a model everywhere at once (all versions), just delete
its entry instead.