donotmerge: dummy inspector api#5085
Conversation
|
🚅 Deployed to the rivet-pr-5085 environment in rivet-frontend
|
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR Review: donotmerge — dummy inspector eval API
OverviewAdds a Auth coverage ✅The new endpoint falls inside the existing inspector auth guard ( Issues and suggestions1. Overly broad The getter throws exactly two errors: let db: unknown;
try {
db = actorCtx.db;
} catch (e) {
if (!(e instanceof errors.DatabaseNotEnabled)) throw e;
db = undefined;
}2. Inconsistent 400 error shape The input-validation error returns 3. Stack traces exposed on eval errors Returning 4. No size or timeout guard on the eval snippet There is no cap on 5. AsyncFunction parameter list is invisible to callers The nine positional bindings ( 6. Missing tests No coverage for the new endpoint. At minimum: a happy-path test (snippet runs, result + logs returned) and an error-path test (bad code throws, 500 response). Style nits
SummaryThe feature direction is sound — an admin eval endpoint behind inspector auth is a useful debugging primitive. Key things to address before merging: narrow the |
0c00af1 to
6932b9f
Compare

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: