Repository navigation
Fix Request Logs clean history purge - #3751
diegosouzapw merged 4 commits into
Conversation
…prevent E415 hard-link tarball rejection
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new “purge request history” capability that clears all request log DB rows (including legacy detail rows) and removes local call-log artifact files, wiring it into the Request Logs dashboard action and documenting the new endpoint.
Changes:
- Implemented artifact-directory purge + “delete all call logs” helper and integrated it into DB cleanup.
- Added
POST /api/settings/purge-request-historyroute and updated the dashboard to call it and display results. - Added unit tests and updated docs/OpenAPI to reflect the new manual purge behavior.
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/unit/database-settings-maintenance.test.ts | Adds test coverage for purging call logs, artifacts, and legacy detail rows via cleanup + new route. |
| src/lib/usage/callLogs.ts | Introduces deleteAllCallLogs() that wipes call_logs and purges artifact directory. |
| src/lib/usage/callLogArtifacts.ts | Adds purgeCallLogArtifactDirectory() which deletes ${DATA_DIR}/call_logs. |
| src/lib/db/cleanup.ts | Updates purgeCallLogs() to also delete artifacts and report deletedArtifacts. |
| src/app/api/settings/purge-request-history/route.ts | New endpoint that purges call logs + detailed logs and returns combined results. |
| src/app/api/settings/purge-call-logs/route.ts | Extends existing response to include deletedArtifacts. |
| src/app/(dashboard)/dashboard/logs/page.tsx | Switches “Clean history” to call the new endpoint and show detailed deletion counts. |
| docs/security/COMPLIANCE.md | Documents manual request-history cleanup behavior and scope. |
| docs/reference/openapi.yaml | Documents the new purge-request-history endpoint. |
| docs/reference/API_REFERENCE.md | Adds the new endpoint to the settings endpoint table. |
| docs/guides/TROUBLESHOOTING.md | Notes what the Request Logs “Clean history” action deletes. |
| docs/getting-started/TROUBLESHOOTING.md | Same troubleshooting note for getting-started docs. |
| export function purgeCallLogArtifactDirectory(baseDir = CALL_LOGS_DIR) { | ||
| if (!baseDir || !fs.existsSync(baseDir)) return 0; | ||
|
|
||
| let deletedArtifacts = 0; | ||
| try { | ||
| deletedArtifacts = listCallLogArtifactFiles(baseDir).length; | ||
| } catch { | ||
| deletedArtifacts = 0; | ||
| } | ||
|
|
||
| fs.rmSync(baseDir, { recursive: true, force: true }); | ||
| return deletedArtifacts; | ||
| } |
| export function deleteAllCallLogs(): DeleteResult { | ||
| const db = getDbInstance(); | ||
| const result = db.prepare("DELETE FROM call_logs").run(); | ||
| const deletedArtifacts = purgeCallLogArtifactDirectory(); | ||
|
|
||
| return { | ||
| deletedRows: result.changes, | ||
| deletedArtifacts, | ||
| }; | ||
| } |
| import { NextResponse } from "next/server"; | ||
| import { buildErrorBody } from "@omniroute/open-sse/utils/error"; | ||
| import { purgeCallLogs, purgeDetailedLogs } from "@/lib/db/cleanup"; | ||
| import { isAuthenticated } from "@/shared/utils/apiAuth"; | ||
|
|
||
| export async function POST(request: Request) { |
| /api/settings/purge-request-history: | ||
| post: | ||
| tags: [Settings] | ||
| summary: Clear request log history | ||
| description: Deletes `call_logs`, legacy `request_detail_logs`, and local request artifact files under `DATA_DIR/call_logs`. | ||
| security: | ||
| - ManagementSessionAuth: [] | ||
| responses: | ||
| "200": | ||
| description: Request history cleared | ||
| "401": | ||
| description: Unauthorized | ||
|
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new endpoint /api/settings/purge-request-history to permanently clear request log rows, legacy detail logs, and local call-log artifacts, updating the UI, documentation, and unit tests accordingly. The review feedback suggests localizing the hardcoded English status messages and modal properties on the logs page using the project's translation helper.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| setCleanHistoryStatus( | ||
| deleted || deletedArtifacts | ||
| ? `Cleaned ${deleted} log entr${deleted === 1 ? "y" : "ies"} and ${deletedArtifacts} artifact${ | ||
| deleted || deletedArtifacts || deletedDetailedLogs | ||
| ? `Cleaned ${deleted} log entr${deleted === 1 ? "y" : "ies"}, ${deletedArtifacts} artifact${ | ||
| deletedArtifacts === 1 ? "" : "s" | ||
| }.` | ||
| : "No expired log history needed cleanup." | ||
| }, and ${deletedDetailedLogs} legacy detail row${deletedDetailedLogs === 1 ? "" : "s"}.` | ||
| : "No request log history was found." | ||
| ); |
There was a problem hiding this comment.
The user-facing status messages for the clean history action are hardcoded in English. Since the project uses next-intl for internationalization (as seen by useTranslations("logs")), these strings should be localized using the translation helper t(...) with proper pluralization support to adhere to the project's i18n standards.
| setCleanHistoryStatus( | |
| deleted || deletedArtifacts | |
| ? `Cleaned ${deleted} log entr${deleted === 1 ? "y" : "ies"} and ${deletedArtifacts} artifact${ | |
| deleted || deletedArtifacts || deletedDetailedLogs | |
| ? `Cleaned ${deleted} log entr${deleted === 1 ? "y" : "ies"}, ${deletedArtifacts} artifact${ | |
| deletedArtifacts === 1 ? "" : "s" | |
| }.` | |
| : "No expired log history needed cleanup." | |
| }, and ${deletedDetailedLogs} legacy detail row${deletedDetailedLogs === 1 ? "" : "s"}.` | |
| : "No request log history was found." | |
| ); | |
| setCleanHistoryStatus( | |
| deleted || deletedArtifacts || deletedDetailedLogs | |
| ? t("cleanHistorySuccess", { | |
| deleted, | |
| deletedArtifacts, | |
| deletedDetailedLogs | |
| }) | |
| : t("noHistoryFound") | |
| ); |
| title="Clean log history?" | ||
| message="This clears expired log history and prunes related artifacts using the current retention policy. The live page will refresh after cleanup." | ||
| message="This permanently clears all request log rows, legacy detail rows, and local artifact files under DATA_DIR/call_logs. The live page will refresh after cleanup." | ||
| confirmText="Clean history" | ||
| cancelText="Cancel" |
There was a problem hiding this comment.
The ConfirmModal properties (title, message, confirmText, cancelText) are hardcoded in English. These should be localized using the t(...) translation helper to ensure consistent internationalization across the application.
| title="Clean log history?" | |
| message="This clears expired log history and prunes related artifacts using the current retention policy. The live page will refresh after cleanup." | |
| message="This permanently clears all request log rows, legacy detail rows, and local artifact files under DATA_DIR/call_logs. The live page will refresh after cleanup." | |
| confirmText="Clean history" | |
| cancelText="Cancel" | |
| title={t("cleanHistoryTitle")} | |
| message={t("cleanHistoryMessage")} | |
| confirmText={t("cleanHistoryConfirm")} | |
| cancelText={t("cleanHistoryCancel")} |
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (9 files)
|
| type: integer | ||
| errors: | ||
| type: integer | ||
| error: |
There was a problem hiding this comment.
CRITICAL: Incomplete 500 response schema - the error object lacks required properties (message, type, code) that buildErrorBody returns. Should reference #/components/schemas/ErrorResponse
| fs.rmSync(baseDir, { recursive: true, force: true }); | ||
| } catch (error) { | ||
| console.error("[callLogArtifacts] Failed to purge call log artifacts:", error); | ||
| result.deletedArtifacts = 0; |
There was a problem hiding this comment.
WARNING: Lost count on deletion failure - deletedArtifacts reset to 0 when fs.rmSync fails, losing the count computed before deletion attempt
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (9 files)
Reviewed by laguna-m.1-20260312:free · 3,223,735 tokens |
…ough-clean-history
15ab183
into
cheaper-inference:release/v3.8.24
|
Thanks @rdself! 🙌 Merged into |
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (#3656), strict-mode CC defaults (#3776), emergency-fallback flag (#3752), xhigh effort (#3756), Codex memory WS (#3749), IPv6 egress (#3777), marketplace SSRF (#3774), CodeQL/Dependabot (#3778), anthropic sampling (#3780), thinking passthrough (#3775), mcp dist entry (#3765), streamed tool args (#3762), logs light-mode (#3760), clean-history purge (#3751), quality-gates (#3757), docs gaps (#3453), file-size re-baseline (#3770), E415 publish guard, i18n prune - Move misplaced #3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
Make the Request Logs Clean history action a full request-history purge instead of retention-only: clears call_logs, legacy request_detail_logs, and local JSON artifacts under DATA_DIR/call_logs (including orphans), via a dedicated POST /api/settings/purge-request-history route. Also hardens the legacy purge-call-logs route to route errors through buildErrorBody (Rule cheaper-inference#12). Integrated into release/v3.8.24.
…contributor credits - Restructure [3.8.24] into ✨ Features / 🔒 Security / 🐛 Fixed / 📝 Maintenance - Add bullets for every PR landed since v3.8.23 that was missing: marketplace (cheaper-inference#3656), strict-mode CC defaults (cheaper-inference#3776), emergency-fallback flag (cheaper-inference#3752), xhigh effort (cheaper-inference#3756), Codex memory WS (cheaper-inference#3749), IPv6 egress (cheaper-inference#3777), marketplace SSRF (cheaper-inference#3774), CodeQL/Dependabot (cheaper-inference#3778), anthropic sampling (cheaper-inference#3780), thinking passthrough (cheaper-inference#3775), mcp dist entry (cheaper-inference#3765), streamed tool args (cheaper-inference#3762), logs light-mode (cheaper-inference#3760), clean-history purge (cheaper-inference#3751), quality-gates (cheaper-inference#3757), docs gaps (cheaper-inference#3453), file-size re-baseline (cheaper-inference#3770), E415 publish guard, i18n prune - Move misplaced cheaper-inference#3775 bullet out of [Unreleased] into [3.8.24] - Date [3.8.23] header (TBD -> 2026-06-12, the release tag date)
Summary
call_logs, legacyrequest_detail_logs, and local JSON artifacts underDATA_DIR/call_logs, including orphaned artifact files.Testing
DISABLE_SQLITE_AUTO_BACKUP=true node --import tsx --import ./open-sse/utils/setupPolyfill.ts --test tests/unit/database-settings-maintenance.test.tsDISABLE_SQLITE_AUTO_BACKUP=true node --import tsx --import ./open-sse/utils/setupPolyfill.ts --test tests/unit/call-log-file-rotation.test.tsnpm run check:openapi-routesnpm run check:fabricated-docs(exits 0; reports existing doc-drift warnings)npm run typecheck:core/dashboard/logs, opened the Clean history confirmation, confirmed the destructive copy mentions DB rows, legacy detail rows, andDATA_DIR/call_logs, and observed the empty success state after confirmation.