Conversation
- Add @true-and-useful/janee as local dependency for direct config access - Rewrite janee-config.ts to use Janee library (loadYAMLConfig/saveYAMLConfig) with proper secret masking, replacing manual YAML parsing - Add CRUD API routes: POST/PUT/DELETE for services, capabilities, and capability agent access, each followed by SIGHUP to reload Janee - Add dashboard API mutation functions (addJaneeService, deleteJaneeService, etc.) - Make JaneeSection fully interactive: inline forms for adding services and capabilities, hover delete buttons with confirmation, editable agent access chips with add/remove - Fix duplicate "restart" key in package.json Made-with: Cursor
…opdown - Permission state is now the primary element on each capability row (right-aligned, same line as name/mode) - Shows correct state based on server.defaultAccess: "No access" (amber) when restricted, "All creatures" (green) when open - Agent picker uses dropdown of known creatures instead of free-text input - Creature chips show short names (alpha not creature:alpha) - Remove duplicate "restart" key from package.json Made-with: Cursor
lucamorettibuilds
left a comment
There was a problem hiding this comment.
Nice — this is a big step toward making Janee feel like a first-class managed subsystem rather than a black-box child process. The Janee library integration + SIGHUP reload is the right architecture. Some notes:
file:../../janee dependency path
"@true-and-useful/janee": "file:../../janee" assumes a specific disk layout. This will break in CI or any clone that doesn't have janee sitting alongside openseed. Options:
- Use the published npm package (
@true-and-useful/janee) — even if you lag a version behind, the config YAML functions are stable - Or use a workspace protocol with a
pnpm-workspace.yamlif you want to develop them side by side
Route matching: startsWith vs regex
The services routes use p.startsWith('/api/janee/services/') for both PUT and DELETE, which means /api/janee/services/foo/bar/baz would match and p.split('/')[4] would be foo. Meanwhile the capabilities routes use p.match() with proper regex bounds. Worth making these consistent — I'd go regex for all of them:
if (p.match(/^\/api\/janee\/services\/[^/]+$/) && req.method === 'PUT') {No input validation on mutations
The mutation handlers pass body.name, body.baseUrl, etc. straight through to config writes without validation. A POST to /api/janee/services with {"name": "", "baseUrl": ""} would create a nameless service. Minimum: check that name is a non-empty string and baseUrl is a valid URL before writing to disk.
updateCapability — Object.assign(cap, patch)
This accepts any fields from the request body and writes them to config. Malformed or unexpected fields ({"__proto__": ...}, arbitrary keys) would be persisted to config.yaml. Consider either picking known fields or validating the patch against CapabilityConfig.
Race condition on concurrent mutations
loadConfig() → modify → saveYAMLConfig() isn't atomic. Two concurrent requests (e.g. rapid UI clicks) could race and one write would clobber the other. Probably fine for a single-user dashboard, but worth a comment or a simple mutex if this ever gets multi-session usage.
reloadJaneeConfig() result ignored
The SIGHUP call returns false if Janee isn't running, but the API routes don't check it. The config gets saved to disk but the user gets a 200 with no indication that the running Janee didn't actually pick up the change. Minor, but could add a reloaded: boolean field to the response.
Dashboard UI looks solid — the permission state chips and inline add forms are a nice touch. The cascade delete on service removal is the right call.
There was a problem hiding this comment.
Solid idea — Janee config CRUD from the dashboard is the right direction. But there are three real problems that need fixing before this can merge.
1. Regression: image stripping removed from event logs
src/shared/image-utils.ts is deleted, and EventStore.append() now writes events directly without stripping image data. PR #75 was specifically about this — multi-MB base64 images from see tool calls were bloating event logs. This PR reintroduces that regression.
This looks accidental — image-utils.ts probably got caught in a rebase or merge conflict resolution. It needs to come back.
2. Type union regressed (and papered over with as any)
src/shared/types.ts removes several event types from the union:
budget.exceeded,budget.resetcreature.spawning,creature.spawned,creature.spawn_failednarrator.entrycreature.subconscious
But src/host/index.ts still emits all of these (lines ~504, ~518, ~809–815). The compiler errors are silenced with as any casts:
await this.emitEvent(name, { t: new Date().toISOString(), type: 'budget.reset' } as any);
await this.emitEvent(name, { type: 'creature.spawning', t: new Date().toISOString() } as any);This is the PR breaking its own types and then suppressing the errors. The GenomeEvent template literal (added in PR #79 specifically for union narrowing) is also reverted back to type: string.
If these event types are intentionally being removed, the emit sites need to be removed too. If they're not intentional, restore the types.
3. No authentication on mutation endpoints
The 7 new routes (POST /api/janee/services, DELETE /api/janee/services/:name, etc.) have no auth check. The orchestrator server binds without a host argument:
server.listen(this.port, () => { ... });That's 0.0.0.0 — reachable from anywhere with network access to the port. Janee config contains API keys and secrets. Anyone who can reach that port can now add or delete services.
The creature-auth PR (#16) added isLocalhost / authenticateCreatureRequest for exactly this kind of scenario. The Janee mutation endpoints should require either localhost origin or a valid token — same pattern as the creature control actions.
Minor: fragile path parsing
const name = decodeURIComponent(p.split('/')[4]);This is used in both the service and capability routes. It works today but silently breaks if the URL structure changes. p.slice('/api/janee/services/'.length) is more explicit and self-documenting. Or use a proper URL parser. Not blocking, but worth fixing while you're in here.
The CRUD logic in janee-config.ts looks right. reloadJaneeConfig() with SIGHUP is clean. The dashboard UI changes I haven't reviewed in detail. The backend issues above are what need addressing first.
- Add input validation on service/capability creation (name, baseUrl required) - Use regex route matching consistently for all service routes (was startsWith) - Use p.slice() for path param extraction instead of fragile split()[4] - Whitelist known CapabilityConfig fields in updateCapability (was Object.assign) - Return reloaded: boolean in mutation responses (SIGHUP result) - Extract shared janeeReply/janeeFail helpers to reduce route boilerplate - Add comment noting single-user load→mutate→save is intentionally non-atomic Made-with: Cursor
|
Pushed fixes for the valid feedback. Here's the breakdown: @lucamorettibuilds
@openseed-patch[bot]
|
Summary
@true-and-useful/janeeas a local dependency so the orchestrator can call Janee's config management functions directly (no REST API needed)janee-config.tsto use Janee's ownloadYAMLConfig/saveYAMLConfigwith secret masking, replacing manual YAML parsingTest plan
Made with Cursor