Adjust TH to the newest PAGI::Tools

[bbrtj]

Continued from #4.

This is a POC solution for TH supporting responses as values, same as newest PAGI::Tools. While I like this idea, awaiting response method calls were a standard occurence in the code, hence the amount of changes required. There's no easy way to update an existing application other than manually removing all await calls.

The things I don't like:

• can't set status anymore without consuming context, since status may be set by redirections, so we need to consider context consumed if response has a status. Setting headers without consuming context still works. I'd like this to be changed somehow, though that may require a map of statuses which don't require a body (redirects, 204, what else?)
• can't clear content-type on the response to have it auto-set by calling something like text again after clearing, but that is kind of okay because I can discard the response now if I don't want its data
• previously, my test set content_type to application/xml, and the charset was added automatically, but this does not happen anymore. Was this changed? I don't want to hardcode a charset in particular in my framework's code
• manually handling pagi.response.sent like your bot suggested is weird and I'm pretty sure it does not propagate upstream in my execution chain, only downstream. This seems like a pretty ugly, buggy hallucination that could fall apart in a more complex scenario with PAGI middlewares intercepting the call and stuff like that - correct me if I'm wrong and this is actually a well documented thing to mark sent responses like that

@jjn1056 I would appreciate if you could look at my changes and the points above.

[jjn1056]

@bbrtj I'm working this these one at a time.

For the content type charset issue, agree I should put it back, for some reason I thought XML was supposed to always be UTF8 and didn't need the encoding, but that's JSON. Fixing that tonight. I will check the rest as I work though it

[jjn1056]

@bbrtj Worked through all four. Three are fixed on PAGI-Tools main; the fourth you were right about, and it turned into a spec change.

3. Charset on application/xml — regression, restored. You were right, I broke it. The UTF-8 body helpers (text/html/json) now append ; charset=utf-8 to any charset-less content-type except application/json and the +json suffix types — those stay bare because JSON is always UTF-8 (RFC 8259) and defines no charset parameter. application/xml, text/*, and +xml carry charset (RFC 7303), so they get it again. Set application/xml, call a body helper, and the charset comes back automatically — nothing hardcoded in your framework.

4. Clearing content-type — now supported, and the re-default works. This was the nudge that made me extract a real header container, PAGI::Headers. You can clear it explicitly with $res->content_type(undef) (or $res->remove_header('content-type')), and because the body helpers set the type with set-if-absent semantics, calling $res->text(...) after clearing re-defaults it exactly as you described. The container also gives you set_default, remove, dehop, and remove_content_headers for proxy-style work.

5. pagi.response.sent — you were completely right, and my bot's suggestion was wrong. It doesn't propagate upstream, for exactly the middleware reason you flagged: PAGI middleware shallow-clone the scope ({ %$scope }) to isolate per-layer state, which snapshots scalars. A scalar written deeper in the chain never rises back through the clones — so a pagi.response.sent scalar is visible downstream only, never upstream. Bad suggestion on my bot's part; good catch on yours.

The right primitive is a server-owned fact on a shared reference, not a scalar: $scope->{'pagi.connection'}->response_started (and ->response_complete). pagi.connection is one object the server seeds and shares by reference through every scope clone, so the flag propagates in all directions. The server sets it on every response path — including a server-synthesized 500 backstop — so it's reliable even when the app never sent anything itself. This is now in the spec (the scope clone scalar-vs-reference behavior is documented, and response_started/response_complete are specified accessors on pagi.connection) and implemented in PAGI-Server.

While here I also closed a related spec gap your container work leans on: the spec now requires a server to reject CR/LF/NUL in emitted header names/values (response splitting), so an app-side header container can hold values as opaque bytes without pre-validating them — the server is the guaranteed chokepoint. It previously mandated that only for SSE fields; ASGI never mandated it at all, which is why uvicorn and daphne disagree on it in practice.

1. Status "consuming" the context — that's a policy you own, not a PAGI constraint. In PAGI's model, setting a status on a Response is inert: it sets a field, nothing is sent until you actually respond, and you can hold, mutate, or discard that Response freely. PAGI doesn't define "has a status ⇒ consumed" — that's TH's interpretation. If you want a cleaner rule, compose it from the facts PAGI already exposes: has_status and has_body_source (is there a body/stream/file yet?). For "statuses that don't need a body," the RFC set is well-defined — 1xx, 204, 304, and the redirect 3xx by convention — so a status-with-no-body-source can stay un-consumed until a body actually appears. I deliberately didn't bake a body-optional-status map into PAGI itself, since two frameworks could reasonably draw that line differently. Happy to talk through your dispatch flow if it still bites — that's the one point where I'd rather understand TH's model than guess.

All these are now landed on main in PAGI, PAGI-Tools and PAGI-Server; take a look and let me know what you think. I'm ideally targeting to release this later this week as I'm presenting on it next week at the Perl Austin Community Conference, but let me know if you need a bit more time.

[jjn1056]

@bbrtj

Ran your feature/pagi-tools branch against the latest PAGI stack (Tools/Server/spec all current). Good news: 113/113 tests pass across 22 files — except one spot in t/params.t, and it's a clean one-line fix on your side.

The /headers action iterates the request headers as a hash:

my $headers = $ctx->req->headers;
foreach my $key (sort keys $headers->%*) { # <-- breaks
push @response, "${key}: " . join ', ', $headers->get_all($key);
}

That worked when req->headers returned a Hash::MultiValue. It now returns a PAGI::Headers container — case-insensitive, order-preserving, multi-value — which deliberately isn't a hash and doesn't overload hash-deref (I tried adding t his and it got very ugly and fragile so leaving it out), so keys %$headers returns the object's internals instead of header names. ->get / ->get_all / ->header are unchanged; only the keys %* iteration pattern needs updating:

foreach my $key (sort $headers->names) { # <-- fix
push @response, "${key}: " . join ', ', $headers->get_all($key);
}

I applied exactly that and re-ran: t/params.t 10/10, full suite green. (There's also a new $headers->to_hash / ->to_hash(1) landing if you'd rather keep a hash shape — keys %{$headers->to_hash(1)} — but ->names is the most direct.)

One heads-up while you sweep: query_params and form_params are still Hash::MultiValue, so your /get and /post actions are fine as-is — it's specifically req->headers that changed type. Worth a grep for any other keys %{ ... ->headers } or %$headers usage.

Everything else from the thread — the application/xml charset, clearing content-type, and response_started — is already in on the Tools side, so this header-iteration line looks like the last thing between your branch and green.

Let me know if you think leaving query_params and form_params as Hash::MultiValue feels weird now that ->headers is its own object. I'm thinking it's ok, this is actually now a lot closer to what Plack::Response does for that stuff and that seemed to stand the test of time.

[bbrtj]

Great, glad my suggestions were helpful.

I was fine even without clearing the headers in the response, since I just wrote re-constructing the response in a single case where I wanted to discard its contents. I think it still makes more sense to do that there, since in that context I really want to discard it (error occurred, so it makes sense to start with a new response). But removing headers one by one can come handy one day.

I see no issue keeping params as Hash::MultiValue. They are not the same as headers in my head, so I don't think I'll mix it up.

Statuses requiring body - you're right it's my obligation. Perhaps I'm too used to getting low-level info about HTTP protocol from tools like PSGI or PAGI, without the need to hardcode it into my framework. I'll see if there is a HTTP status library that could give me that info, if not I'll just hardcode it as constants.

I'll update the code with the newest version of yours and see whether it works well for me now conceptually.

[bbrtj]

Alright, new commit replaces the old one. I like it much better, especially how Context looks now - way, way less cruft code. This still needs documentation, and some more tests of the methods I added in context, but other than that should be ready to go.

@jjn1056 if you wanted to check the code again, go ahead, though it did not change much other than the Context implementation. Good job overall - I think turning response into a value will greatly increase coding ergonomics, since Thunderhorse code will not need much async/await unless you actually do async stuff or use websocket / sse.

If you have no more big changes planned before you release, I will finalize docs/tests here and merge during the weekend.

Last thing - PAGI-Server pulled from your repository does not install for me. If it did, I could adjust my blog (https://bbrtj.eu, runs Thunderhorse) to use the new system and see if everything works well together in a live environment.

I'm getting test errors, even though I have the latest PAGI-Tools. Seems like I only have these errors repeated:

PAGI application error: Can't locate object method "AWAIT_IS_READY" via package "PAGI::Response" at t/integration/response-integration.t line 93.
String found where operator expected (Do you need to predeclare "enable"?) at t/integration/security-headers.t line 54, near "enable 'SecurityHeaders'"
syntax error at t/integration/security-headers.t line 54, near "enable 'SecurityHeaders'"

[jjn1056]

@bbrtj yeah I was too stuck thinking in the Catalyst mode and didn't grasp the point of 'Response as value' when I read the Starlette docs and code the first time. This was a case where AI actually walked me thru a pile of examples until I grasped it :). But it makes for better code overall I think, plus you get nice idioms like "return PAGI::Response->text('hello world')". It preserves the idea that PAGI is a code ref that returns a value; neatly functional.

Thanks for all the feedback, you've really helped me to shake out issues. ASGI/PAGI is vastly more complex than WSGI/PSGI but it's carrying a lot more capability. It's not just Plack with web sockets, it's an entire system for event driven programming. I'm planning things like AI Agent harnesses using PAGI as the core loop.

I found the issue you noted with PAGI-Server and pushed an update to that repo. If you get a chance to test again let me know how it goes. At this stage I'm not expecting changes this large again. I'm moving PAGI spec to version 0.3 which I hope closes most of the major gaps and PAGI-Server is basically 'feature complete' for me it terms of what I want it to support. Obviously there will be bug fixes over time and I'm still pondering some fixes to how it makes temp files and a few things that are still synchronous but most of that is stuff nobody will notice. And I doubt very much I'd change PAGI-Tools this big again.

[jjn1056]

@bbrtj just FYI I'd plan to release this like over the weekend as well, probably Saturday morning so that I give it some time to see how CPAN testers go. PAGI doesn't yet have big uptake but I'd rather release on a weekend since most people won't run into it until Monday, which will give me some time to make sure the packages are all correct, etc.

[bbrtj]

My blog is now running the newest stack. It is shown under a temporary url which is rendering the hand-modified versions of modules which I pulled into a local dir, plus a local-dir pagi-server executable. Everything is looking good.

This website is mostly async-free, so the only change I needed to make it work was to change error rendering to avoid awaiting Response functions. Since it has so little async, its not much of a proof, but it is a good signal nonetheless.

This MR will be merged soon, but I'll wait until you release new modules and check if CI approves.

[jjn1056]

@bbrtj I have a bit of free time this afternoon (Friday) and will get all this done and on CPAN today. Its been a stressful week and will be nice to work on my own thing a bit.

[jjn1056]

@bbrtj just FYI PAGI, PAGI-Tools, and PAGI-Server have shipped. I'll be watching CPAN testers over the weekend so expect a bit of churn until I see a good, green board for these

[bbrtj]

Thank you for your assistance, will be released shortly.