fix(app): validate agentId once for every route (#162) - #176
Merged
Conversation
Twenty routes called getAgentId() inside their main try block, so the
TypeError it throws for a malformed agent id fell through to the generic
500 branch — where fail() masks any 5xx message as "Internal server
error". A caller sending /memory/bad%20agent/entities got a 500 with no
indication the id was the problem, while 34 sibling routes returned a
correct 400. The filed issue named 2 routes; it was 20.
Fixes it in one place instead of twenty: an app.param('agentId', ...)
validator covering all 55 agent-scoped routes, including the /pool
routes carrying both :poolId and :agentId. Every route name-checks the
same param, so coverage is complete with no per-route edits, and a route
added later inherits correctness rather than depending on its author
copying the idiom. Express resolves param callbacks at match time, so
registration position does not limit coverage.
getAgentId() stays as defense in depth. It should now be unreachable,
but removing it would make handler correctness depend on middleware
registration surviving future edits.
Behavior change worth knowing: on the 34 already-correct routes, agentId
is now validated before per-route body/query validation. The status is
400 either way — only which 400 message wins changes. Pinned by a test
so it reads as a decision rather than a surprise.
Also adds the two implemented /query parameters missing from the OpenAPI
spec: `epistemic` (v3.9, the filed half of the issue) and `max_tokens`
(found while checking — same array, no extra review surface).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011xCqQo49d3CEbn6oEvb3Ru
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #162.
The defect was 10× wider than filed
The issue named
/entitiesand/graph. In fact 20 routes calledgetAgentId()inside their maintry, so itsTypeErrorfell to the generic 500 branch — andfail()masks every 5xx message as"Internal server error". Callers got a 500 with no hint that the agent id was malformed, while 34 sibling routes correctly returned 400.One validator instead of twenty edits
app.param('agentId', …)covers all 55 agent-scoped routes (including/poolroutes carrying both:poolIdand:agentId) because every one of them names the param identically — verified before choosing this approach. A route added next year inherits correctness instead of depending on its author remembering the idiom.getAgentId()is kept as defense in depth: it should now be unreachable, but deleting it would make handler correctness depend on middleware registration surviving future edits.Behavior change, pinned by a test
On the 34 routes that were already correct, agentId now validates before per-route body/query validation. Status is 400 either way; only which 400 message wins changes. There's a test asserting the precedence so it's a documented decision rather than a surprise during debugging.
Also included
The OpenAPI
/querypath was missing two implemented parameters:epistemic(v3.9 — the filed half of the issue) andmax_tokens(found while checking; same array, no extra review surface).Tests
New
agentId validation (#162)suite: previously-broken routes, previously-correct routes, POST routes, over-long ids, the precedence change, and a valid id still reaching its handler.test:httptest:securitytest:integrationtest:bootstrap/abstractions/causal-graphtest:explainable-memory/epistemic-confidence/multi-device🤖 Generated with Claude Code
https://claude.ai/code/session_011xCqQo49d3CEbn6oEvb3Ru