[codex] Add versioned admin agent markdown editor

[furukama]

What changed

• add an admin console page for browsing agents, selecting allowlisted workspace markdown files, editing them, and restoring saved revisions
• add gateway admin endpoints and shared types for reading, saving, listing revisions, previewing revisions, and restoring revisions for agent workspace bootstrap markdown files
• store per-agent markdown revisions as snapshot files and expose revision metadata/content to the console
• follow up on review feedback by centralizing byte-based write validation, preventing refetches from clobbering dirty editor drafts, and splitting the admin agent HTTP handler into subresource-specific handlers
• add backend and console tests for markdown editing, revision restore, oversized-content rejection, and dirty-draft preservation

Why it changed

Admins needed a narrow, backend-managed way to update agent bootstrap markdown without going through the filesystem directly. Version history was added so those edits are recoverable, and the follow-up review fixes tighten correctness and keep the route/editor logic easier to maintain.

User and developer impact

• admins can edit allowlisted agent markdown files from /admin/agents
• each save or restore records a revision that can be previewed and restored later
• oversized multibyte content and oversized revisions now fail before mutating the workspace
• in-progress editor changes are no longer overwritten by query refetches
• the admin agent API route is split by resource type, reducing validation drift

Root cause

The initial implementation duplicated write-path validation between save and restore and coupled local editor draft state directly to query refreshes. The route handler also accumulated multiple resource concerns in a single function.

Validation

• npx biome check --write src/gateway/gateway-http-server.ts src/gateway/gateway-service.ts tests/gateway-service.agent-markdown.test.ts console/src/routes/agents.tsx console/src/routes/agents.test.tsx
• npx vitest run tests/gateway-service.agent-markdown.test.ts tests/gateway-http-server.test.ts
• npx vitest run --config console/vitest.config.ts console/src/routes/agents.test.tsx
• npm run typecheck

Notes

• revisioning remains intentionally limited to the allowlisted workspace bootstrap markdown files
• the existing gateway-http-server test stderr noise around runtime secret permissions and startup DB access is unchanged; the suites still pass

[furukama]

@claude Do a thorough review of this PR in terms of: KISS, YAGNI, DRY, Fail Fast, Secure by Default and performance/speed

[claude]

Claude finished @furukama's task in 3m 45s —— View job

---

PR Review: KISS, YAGNI, DRY, Fail Fast, Secure by Default & Performance

• Gather context and read changed files
• Review gateway-service.ts (backend logic)
• Review gateway-http-server.ts (routing/handlers)
• Review gateway-types.ts (shared types)
• Review console routes (agents.tsx, agents.test.tsx)
• Review console API client and types
• Review tests (gateway-service.agent-markdown.test.ts, gateway-http-server.test.ts)
• Compile findings and post review

---

Overall: the implementation is solid, well-tested, and the direction (allowlist, byte-safe writes, dirty-draft preservation) is all correct. The findings below are genuine issues worth addressing, not stylistic nitpicks.

---

DRY — Repeated fs.stat / content-read within a single request

gateway-service.ts — saveGatewayAdminAgentMarkdownFile

A single save triggers the target file's stats 3 times and its content twice, plus all 10 workspace markdown file stats in the final return value:

1. readGatewayAdminAgentMarkdownFileContent internally calls getGatewayAdminAgentMarkdownFileStats (stat favicon fehlt #1) then fs.readFileSync (read favicon fehlt #1)
2. The explicit getGatewayAdminAgentMarkdownFileStats(resolved.filePath) call right after (stat ogg image wäre schön #2)
3. The final getGatewayAdminAgentMarkdownFile(...) → mapGatewayAdminAgent → 10× mapGatewayAdminAgentMarkdownFile → 10× getGatewayAdminAgentMarkdownFileStats (stat looses his bot during onboarding #3 for the target file, plus 9 others)
4. writeGatewayAdminAgentMarkdownFileContent calls assertGatewayAdminAgentMarkdownContentSize (byte-count ogg image wäre schön #2 on params.content; favicon fehlt #1 was already done at the top of saveGatewayAdminAgentMarkdownFile)

The stat results from step 1 could be threaded through instead of discarded, and the final getGatewayAdminAgentMarkdownFile call could be replaced by a direct assembly using already-available values. Same pattern applies to restoreGatewayAdminAgentMarkdownRevision.

Fix this →

---

DRY — Redundant condition in restoreGatewayAdminAgentMarkdownRevision

// early return already handles currentContent === revision.content:
if (currentStats.exists && currentContent === revision.content) { return ...; }

// so this `&& currentContent !== revision.content` is always true here:
if (currentStats.exists && currentContent !== revision.content) {
  writeGatewayAdminAgentMarkdownRevision(...);
}

The second condition can be simplified to just if (currentStats.exists). [gateway-service.ts ~line 1153]

Fix this →

---

DRY — Mock agent shape repeated 4× in gateway-http-server.test.ts

getGatewayAdminAgentMarkdownFile, getGatewayAdminAgentMarkdownRevision, saveGatewayAdminAgentMarkdownFile, and restoreGatewayAdminAgentMarkdownRevision mocks each inline the full agent shape with identical agentId === 'writer' ? ... : ... ternaries. A shared makeTestAgent(agentId) helper would halve the test fixture size and make future field additions a one-line change.

Fix this →

---

Performance — mapGatewayAdminAgent issues 10 fs.statSync calls per agent on every list request

getGatewayAdminAgents() is called for every GET /api/admin/agents. Each agent triggers mapGatewayAdminAgent → 10× mapGatewayAdminAgentMarkdownFile → 10× fs.statSync. With N agents that's 10N blocking stat calls on the main event loop.

For the agents list, callers only need names and existence counts — not full per-file metadata. Options:

1. Omit markdownFiles from the list response and add it only to the per-agent detail endpoint.
2. Keep it but do a single fs.readdirSync on the workspace dir and check membership, rather than a stat per file.

The same 10-stat burst is repeated in the save and restore return values via the final getGatewayAdminAgentMarkdownFile call. [gateway-service.ts ~line 913]

Fix this →

---

Performance — listGatewayAdminAgentMarkdownRevisions reads all revision JSON files, unbounded

Each GET /api/admin/agents/:id/files/:name, save, and restore reads and parses every .json file in the revision directory. With frequent saves this grows without bound, and the sort re-parses createdAt strings on every comparison instead of pre-computing once. There is no cap or pagination.

The sort could pre-compute timestamps once: entries.map(e => ({ ...e, ts: Date.parse(e.createdAt) })).sort(...). More importantly, a revision cap (e.g. keep last 50) would prevent unbounded disk growth. [gateway-service.ts ~line 1058]

Fix this →

---

Security — Revision ID uses Math.random()

id: `${now.getTime().toString(36)}-${Math.random().toString(36).slice(2, 8)}`,

Math.random() is not cryptographically secure. The ID is exposed via the API. While revisions are admin-only and the IDs are not used for authorization, using crypto.randomUUID() (already imported in the file for other uses) is simpler and produces a standard format with no collision risk. [gateway-service.ts ~line 983]

Fix this →

---

Security — Regex-based error-to-status-code inference

const status =
  error instanceof GatewayRequestError
    ? error.statusCode
    : /not found/i.test(message)
      ? 404
      : 400;

Matching "not found" in a free-form error message to decide HTTP 404 vs 400 is fragile. Service functions already throw plain Errors with "was not found" messages (e.g. "Revision … was not found."). If any future error message coincidentally contains "not found", it will silently map to 404. Use GatewayRequestError with explicit status codes throughout, or at minimum limit the regex to the known not-found message patterns. [gateway-http-server.ts sendApiAdminAgentError]

Fix this →

---

Security / YAGNI — Redundant path-traversal guard after allowlist validation

const fileName = normalizeGatewayAdminAgentMarkdownFileName(params.fileName);
// ^ already throws if not in WORKSPACE_BOOTSTRAP_FILES (e.g. "AGENTS.md")

const filePath = path.resolve(workspacePath, fileName);
if (
  filePath !== path.join(workspacePath, fileName) ||  // redundant
  path.dirname(filePath) !== workspacePath              // also redundant
) { throw ... }

WORKSPACE_BOOTSTRAP_FILES entries are simple bare filenames with no separators, so path.resolve(workspacePath, "AGENTS.md") is identical to path.join(workspacePath, "AGENTS.md") by definition — the double-check can never trigger after a valid normalizeGatewayAdminAgentMarkdownFileName. The guard adds noise without real protection. If kept for defence-in-depth, it deserves a comment explaining that. [gateway-service.ts ~line 841]

---

KISS — requireAdminAgentIdPathValue checks for literal "agents" string

function requireAdminAgentIdPathValue(agentId: string): string {
  const normalizedAgentId = agentId.trim();
  if (!normalizedAgentId || normalizedAgentId === 'agents') {

The === 'agents' guard is a specific workaround for the old monolithic handler. With the new parseApiAdminAgentsRoute router, kind: 'agent' is only reached for length-4 segments, so agentId === 'agents' would indicate a routing bug, not user error. The guard is now dead code and should be removed. [gateway-http-server.ts]

Fix this →

---

KISS / Fail Fast — parseApiAdminAgentsRoute returns empty strings for missing segments

const agentId = segments[3] ? decodeApiPathSegment(segments[3]) : '';

The function returns { kind: 'agent', agentId: '' } if segment 3 is somehow empty after split('/'). Downstream requireAdminAgentIdPathValue then catches it. Fail-fast here with an 'unknown' match is cleaner. The current approach relies on the ordering of two functions to compose safety. [gateway-http-server.ts parseApiAdminAgentsRoute]

---

YAGNI — buildGatewayAdminAgentMarkdownRevision injectable now parameter

function buildGatewayAdminAgentMarkdownRevision(
  params: { ... },
  now = new Date(),
): StoredAdminAgentMarkdownRevision {

This injectable clock is not used in any test and is not referenced outside this single internal function. The service-level tests use real timestamps. Remove the parameter and call new Date() inline. If clock injection is needed later it can be added then.

---

YAGNI — restoreAdminAgentMarkdownRevision sends an empty body

// console/src/api/client.ts
body: {},

POST .../restore needs no body. Sending {} causes an unnecessary Content-Type: application/json header and JSON serialization. The server ignores it, but removing body from the requestJson call avoids the round-trip overhead and false suggestion of a payload. [client.ts restoreAdminAgentMarkdownRevision]

Fix this →

---

Fail Fast — mapGatewayAdminAgent calls resolveAgentConfig twice per request

In getGatewayAdminAgentMarkdownFile, resolveGatewayAdminAgentMarkdownFile calls resolveAgentConfig(agent.id) to get workspacePath, and then mapGatewayAdminAgent(resolved.agent) (called in the same function) calls resolveAgentConfig again for the same agent. These two calls should share the resolved config. [gateway-service.ts]

---

Minor — isDirty has a redundant || '' fallback

// agents.tsx
const isDirty =
  Boolean(fileQuery.data) &&
  draftContent !== (fileQuery.data?.file.content || '');

Boolean(fileQuery.data) already ensures fileQuery.data is truthy, so ?.file.content || '' can never fall to '' within this expression. Simplify to fileQuery.data.file.content. Small but noisy.

---

Summary table

Principle	Severity	Location
DRY	Medium	saveGatewayAdminAgentMarkdownFile — 2 stats + 2 byte-checks for same data
DRY	Low	restoreGatewayAdminAgentMarkdownRevision — redundant && condition
DRY	Low	gateway-http-server.test.ts — mock agent shape repeated 4×
Performance	Medium	mapGatewayAdminAgent — 10N fs.statSync on every list call
Performance	Medium	listGatewayAdminAgentMarkdownRevisions — unbounded reads + O(N log N) Date.parse in comparator
Secure by Default	Low	Revision ID uses Math.random() instead of crypto.randomUUID()
Secure by Default	Low	sendApiAdminAgentError — regex-based error-to-status inference
KISS	Low	Path traversal guard is dead code after allowlist validation
KISS	Low	requireAdminAgentIdPathValue has dead === 'agents' check
YAGNI	Low	Injectable now param in buildGatewayAdminAgentMarkdownRevision unused in tests
YAGNI	Low	restoreAdminAgentMarkdownRevision sends empty body: {}
Fail Fast	Low	mapGatewayAdminAgent calls resolveAgentConfig twice per request

[Copilot]

Pull request overview

Adds a versioned admin-console markdown editor for allowlisted agent workspace bootstrap files, backed by new gateway admin APIs for read/save/preview/restore and snapshot-based revision history.

Changes:

• Introduces gateway admin endpoints + shared/console types for agent markdown files and revisions.
• Implements revision snapshot storage and byte-based validation for save/restore flows.
• Adds admin console /admin/agents UI + tests (backend + console) for editing, restoring, oversized rejection, and dirty-draft preservation.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/gateway-service.agent-markdown.test.ts	Adds gateway-service tests for save/load, allowlist enforcement, and oversized rejection.
tests/gateway-http-server.test.ts	Adds HTTP handler tests for new admin agent markdown routes and input validation.
src/workspace.ts	Exports WORKSPACE_BOOTSTRAP_FILES for reuse as the markdown allowlist.
src/gateway/gateway-types.ts	Adds gateway admin response/type definitions for agent markdown files and revision metadata.
src/gateway/gateway-service.ts	Implements read/save/list revision/preview/restore logic and revision persistence on disk.
src/gateway/gateway-http-server.ts	Splits /api/admin/agents handling into subresource handlers (agent/file/revision/restore).
console/src/routes/agents.tsx	Adds Agent Files page UI with editor, revision list/preview, and dirty-draft protection.
console/src/routes/agents.test.tsx	Adds UI tests for loading/switching agents, saving, restoring, and draft preservation.
console/src/router.tsx	Registers /agents route in the admin console router.
console/src/components/sidebar/navigation.ts	Adds “Agent Files” navigation item.
console/src/api/types.ts	Adds console API types for admin agent markdown file + revision payloads.
console/src/api/client.ts	Adds client functions for new admin markdown endpoints.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.