Skip to content

fix: restore GUI request logs after ocx stop/start - #195

Closed
Wibias wants to merge 2 commits into
lidge-jun:mainfrom
Wibias:fix/request-logs-survive-restart
Closed

Wibias wants to merge 2 commits into
lidge-jun:mainfrom
Wibias:fix/request-logs-survive-restart

Conversation

@Wibias

@Wibias Wibias commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • bun test tests/request-log.test.ts (hydrate + MAX_LOG_SIZE slice)
  • Pre-push full suite
  • Manual: run a few turns, ocx stop, ocx start, confirm Logs page still shows prior requests

Fixes the log-loss side note from #186.

/api/logs only lived in an in-memory ring buffer, so stopping the proxy
wiped the Logs page even though usage.jsonl still had every request.
Hydrate the ring from the last 200 usage.jsonl rows on server start.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d442b22415

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/server/request-log.ts Resolved
Comment thread src/server/request-log.ts Resolved
Comment thread src/server/request-log.ts Resolved
Swallow usage.jsonl read failures so a bad file cannot brick ocx start,
tail only the newest ring rows instead of loading the whole append-only
log, and persist effort/tier metadata so hydrated GUI Logs keep those
columns after restart.
@lidge-jun

Copy link
Copy Markdown
Owner

Please retarget this PR to the dev branch — all contributions go to dev first, and main receives only release promotions. See our Contributing guide for details. Thanks! 🙏

@lidge-jun

Copy link
Copy Markdown
Owner

Adversarial Review Feedback

Solid approach — the hydration logic is process-idempotent and handles missing/IO-failed files gracefully.

Blockers

  1. Persisted metadata is not redacted or bounded — service_tier accepts arbitrary strings and is persisted to usage.jsonl without redactSecretString(). A crafted service_tier: "Bearer sk-secret123" appears verbatim in the file. Apply strict tier validation plus redaction/length caps to all newly persisted strings. Re-redact upstreamError during hydration for legacy rows.

  2. Corrupt-tail handling loses valid rows — The reader selects the last N physical lines before parsing, so one malformed final line permanently reduces the hydrated ring. With 201 valid rows + 1 corrupt tail, readRecentUsageEntries(200) returns only 199. Scan backward until N schema-valid entries are collected instead.

Please retarget to dev — see Contributing guide. 🙏

@lidge-jun

Copy link
Copy Markdown
Owner

Absorbed onto dev as 477f6dd — cherry-picked both commits with two fixes:

  1. Metadata string cap — all service-tier/effort/speed-label fields capped at 64 chars in normalizeUsageEntry to prevent injection through crafted values.
  2. Corrupt-tail resilience — readRecentUsageEntries now parses ALL lines before slicing to the limit, so corrupt/partial lines never reduce the hydrated ring count.

Tests pass (33/33). The hydration logic is solid — great work on this! 🙏

Co-authored-by: Wibias

lidge-jun added a commit that referenced this pull request Jul 21, 2026
Cherry-pick PR #195 by Wibias. Adds hydrateRequestLogsFromDisk() that reads
usage.jsonl on server start to repopulate the in-memory /api/logs ring buffer.
Enriches persisted usage entries with effort/service-tier metadata.

Fixes applied:
- Cap metadata strings to 64 chars in normalizeUsageEntry (prevents injection)
- Parse all lines before slicing to limit (corrupt-tail resilience)

Co-authored-by: Wibias <Wibias@users.noreply.github.com>
@lidge-jun lidge-jun closed this Jul 21, 2026
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
Cherry-pick PR lidge-jun#195 by Wibias. Adds hydrateRequestLogsFromDisk() that reads
usage.jsonl on server start to repopulate the in-memory /api/logs ring buffer.
Enriches persisted usage entries with effort/service-tier metadata.

Fixes applied:
- Cap metadata strings to 64 chars in normalizeUsageEntry (prevents injection)
- Parse all lines before slicing to limit (corrupt-tail resilience)

Co-authored-by: Wibias <Wibias@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants