fix(routes): take a route's handler from the last argument, not the first

[CaptainMittens]

Express, Fastify, gin and Laravel all put middleware between the route path and the handler, and the handler comes last:

routerGet("/users", requireAuth, rateLimit, listUsers);

extract_handler_arg returned the first argument that looked like a function reference, so requireAuth won. second_arg_name carries that value into pass_calls.c, which resolves it into the HANDLES edge target — so the edge pointed at the middleware instead of listUsers. That is a wrong edge rather than a missing one, and it misleads anyone tracing a request through the graph.

The scan bound hid a second fault. An arrow function matches none of the accepted node kinds, so three inline middlewares pushed the real handler past MAX_HANDLER_SCAN and no handler came back at all.

Both faults reproduced before any change

Two tests in this PR, red on current main:

extract_ts_route_handler_after_named_middleware   FAIL  "requireAuth" != "listUsers"
extract_ts_route_handler_after_inline_middleware  FAIL  c->second_arg_name is NULL

The change

The loop examines every argument and keeps the last eligible one. Nothing that follows a handler matches the accepted kinds either — an options argument is an object node — so the last match is the handler.

HANDLER_START_IDX stays, because argument 0 really is the route path. MAX_HANDLER_SCAN had no other use and is removed.

Why existing route tests do not move

A two-argument route has one eligible argument, so first and last are the same value. handles_express_ts, handles_fastify_js, handles_gin_go, handles_laravel_php and both handles_laravel_facade_*_issue952 tests pass unchanged.

Why the tests live in test_extraction.c

The test_edge_types_probe.c harness offers et_edge_present, et_routes_exact and et_calls_to_name_parallel, and none of them says which node a HANDLES edge points at. A wrong handler still yields one HANDLES edge and the right Route name, so a probe test would have passed while the bug stood. test_extraction.c already reads first_string_arg directly; these tests assert second_arg_name beside it, which is the value being changed.

Note for anyone with an existing index

This changes which function a route resolves to, so already-indexed projects carry the old handler edges until they are re-indexed.

Full C suite on this branch: 7782 passed, 7 skipped. Two test_cli.c install/uninstall tests fail on my machine with or without this change, because they read the coding agents actually installed there.

[DeusData]

Approved, and this is the more consequential of your two route fixes — a wrong edge, not a missing one.

Verified on main: extract_handler_arg (extract_calls.c:2351) returns inside the loop on the first eligible argument, and MAX_HANDLER_SCAN = 4 with HANDLER_START_IDX = 1 means it only ever examines arguments 1–3. So both faults you describe are real:

• routerGet("/users", requireAuth, rateLimit, listUsers) → returns requireAuth, which second_arg_name carries into pass_calls.c as the HANDLES target. The graph then says the route is handled by the auth middleware.
• Three inline arrow middlewares push the real handler to index 4, past the cap, and nothing comes back at all.

A wrong edge is worse than a missing one, and worth saying why: a missing HANDLES edge is visibly absent, while a wrong one is silently plausible — anyone tracing a request through the graph lands on requireAuth, sees a function that really is on that route, and has no signal that it is not the handler. That is the failure mode this project cares most about.

Taking the last eligible argument is the right rule for the frameworks named — Express, Fastify, gin and Laravel all place middleware between path and handler with the handler last — and your justification is the part that makes it safe: nothing that legitimately follows a handler matches the accepted kinds, since an options argument is an object node.

One thing to keep an eye on, not a blocker. is_string_like is among the accepted kinds, because Laravel handlers are strings ('Controller@method'). Under first-match a trailing non-handler string could never win; under last-match it can. I could not construct a real call shape where that bites — a trailing string argument after a handler is not a pattern any of the four frameworks use — but it is the one place where "last" is a weaker assumption than "first", so it is worth remembering if a Laravel route ever reports an odd HANDLES target.

Removing MAX_HANDLER_SCAN entirely rather than raising it is correct: a cap on a scan that must find the last match has no meaning. Keeping HANDLER_START_IDX because argument 0 really is the route path is the right asymmetry.

Both faults reproduced red before the change, with the distinct failure text for each ("requireAuth" != "listUsers" and second_arg_name is NULL), which is what shows they are two faults and not one.

Merging on green. Our Actions queue is heavily backlogged at the moment, so expect a wait — nothing to do with this PR. #2008 touches extract_defs.c and this touches extract_calls.c, so the two are independent and neither will need a rebase for the other.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Merged as 01bf5f23. Thank you — this was the more consequential of your two route fixes.

Worth restating why: extract_handler_arg returned the first eligible argument, so routerGet("/users", requireAuth, rateLimit, listUsers) produced a HANDLES edge pointing at the auth middleware. That is a wrong edge, not a missing one — and a wrong edge is the worse failure, because anyone tracing a request lands on a function that really is on that route and has no signal it is not the handler. A missing edge is visibly absent; a wrong one is silently plausible.

Removing MAX_HANDLER_SCAN rather than raising it was right too: a cap on a scan that must find the last match has no meaning. And keeping HANDLER_START_IDX because argument 0 really is the route path is the correct asymmetry.

On verification

Your green was 35/35, but main had taken six merges since your branch's base — three of which touched tests/test_extraction.c, including #1824, which is also route-related. Rather than argue that the production files were disjoint, I rebuilt the actual merge locally against current main and ran extraction and edge_types_probe: clean build, 388 passed, 0 failed. That cost our badly-backlogged Actions queue nothing and settled the question properly.

The one thing I noted at review still stands as a watch item, not a blocker: is_string_like is among the accepted kinds for Laravel string handlers, so "last match" is a weaker assumption than "first match" for a trailing string argument. I could not construct a real call shape where it bites, but if a Laravel route ever reports an odd HANDLES target, that is the first place to look.

That is your fourth merge today, after #2008, #1986 and #1877.