feat: add HTTP/2 session management and events

[niteshpurohit]

• Implemented HTTP/2 session management in http2.cpp to handle client-server communication.
• Added support for HTTP/2 events such as headers, data frames, and stream management.
• Introduced new dependency operations for HTTP/2 in dependency.cpp and contract.hpp.
• Created tests for HTTP/2 functionality in http2.cpp and http2_contract.cpp to ensure reliability and correctness.
• Enhanced memory management for HTTP/2 sessions using a bounded arena to prevent memory exhaustion.

closes: #60

[Copilot]

🟡 Changes recommended

Add a resume path for paused DATA delivery and expose SETTINGS ACK information in the contract.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an nghttp2-backed HTTP/2 adapter with session management, event handling, bounded memory allocation, dependency integration, and tests.

Changes:

• Adds HTTP/2 session, stream, header, settings, and event contracts.
• Implements nghttp2 callbacks, frame operations, and bounded-arena allocation.
• Integrates build configuration, dependencies, CI coverage, and tests.

File summaries

File	Summary
tests/adapters/http2.cpp	Functional HTTP/2 adapter tests
tests/adapters/http2_contract.cpp	HTTP/2 contract assertions
src/core/contract/laghu/core/contract.hpp	HTTP/2 dependency operations
src/adapters/http2.cpp	nghttp2 HTTP/2 adapter implementation
src/adapters/dependency.cpp	HTTP/2 dependency operation mappings
src/adapters/contract/laghu/adapters/http2.hpp	Public HTTP/2 contract
CMakeLists.txt	HTTP/2 build and test targets
cmake/LaghuDependencies.cmake	Dependency build configuration
cmake/LaghuBuildIdentity.cmake	HTTP/2 build identity inputs
.github/workflows/toolchain.yml	HTTP/2 CI coverage

Review details

Suppressed comments (1)

src/adapters/http2.cpp:227

• The GOAWAY event only forwards error_code; last_stream_id and the opaque debug data are dropped, while event.stream is derived from the frame header's connection stream ID (0). Consumers therefore cannot determine which peer streams were processed or access the peer diagnostic; add fields to Http2Event and populate them here.

    case NGHTTP2_GOAWAY:
      event.kind = Http2EventKind::goaway;
      event.code = frame->goaway.error_code;
      break;

• Files reviewed: 10/10 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Moderate issues remain in DATA-event handling, memory reclamation, and explicit flow-control behavior.

Review details

Suppressed comments (5)

Previously missed (1) — in code that hasn't changed since the last review.

src/adapters/http2.cpp:82

• memory_free is a no-op, and memory_realloc allocates a replacement without reclaiming the old block. nghttp2 regularly frees and resizes session/HPACK allocations as streams and the dynamic table churn; because this monotonic arena is only reset after the whole session ends, a long-lived connection will eventually exhaust its bound even when its steady-state native memory is bounded. Use a reclaimable bounded pool/allocator for the nghttp2 memory hooks, or otherwise recycle storage during the session.

src/adapters/http2.cpp:196

• When this callback returns NGHTTP2_ERR_PAUSE for an incoming DATA frame, nghttp2 retains the frame and requires nghttp2_session_resume_data(session, stream_id) before processing can continue. Http2Session exposes no resume operation, so a sink that pauses on a data event cannot resume the stream by merely passing the unconsumed suffix back to receive() as the public contract suggests. Add a stream-resume operation to the contract/implementation (and call it before retrying input), or do not advertise pausing DATA callbacks.

  return callback_result(state, event, true, false);

src/adapters/http2.cpp:196

• The new on_data event path has no test coverage: the HTTP/2 tests exercise headers, SETTINGS, reset, GOAWAY, window updates, limits, and pause-on-header, but never deliver a DATA frame or verify its data/end_stream fields. Add a DATA-frame exchange (including a zero-length END_STREAM case and, if supported, pause/rejection) so regressions in this advertised callback cannot pass the adapter suite.

int on_data(nghttp2_session*, std::uint8_t flags, std::int32_t native_stream_id,
            const std::uint8_t* data, std::size_t length, void* user_data) noexcept {
  auto& state = *static_cast<NativeState*>(user_data);
  const auto data_view = core::ByteView::from(
      {reinterpret_cast<const std::byte*>(data), length});
  if (!data_view.has_value()) {
    return NGHTTP2_ERR_CALLBACK_FAILURE;
  }
  Http2Event event{};
  event.kind = Http2EventKind::data;
  event.stream = stream_id(native_stream_id);
  event.data = *data_view;
  event.end_stream = (flags & NGHTTP2_FLAG_END_STREAM) != 0U;
  return callback_result(state, event, true, false);

src/adapters/http2.cpp:196

• When the event sink returns reject_stream for a DATA event, rejectable is false, so callback_result maps it to NGHTTP2_ERR_CALLBACK_FAILURE and tears down the whole HTTP/2 session instead of producing the stream-scoped temporal callback failure used for header events. This prevents callers from rejecting a stream after inspecting its body and makes the public reject_stream action unsafe for data callbacks; allow the data callback to use the stream-rejection result.

  return callback_result(state, event, true, false);

src/adapters/http2.cpp:316

• Constructing the session with nullptr options leaves nghttp2's automatic receive-window updates enabled. That means consuming DATA in receive() can cause WINDOW_UPDATE frames without Laghu explicitly granting credit, while this contract exposes submit_connection_window_update/submit_stream_window_update and assigns backpressure ownership to Laghu. Configure the native session with nghttp2_option_set_no_auto_window_update and provide/use an explicit credit path so a paused or unconsumed stream cannot advertise more capacity automatically.

    result = nghttp2_session_client_new3(&native_session, callbacks, state, nullptr, &memory);
  } else {
    result = nghttp2_session_server_new3(&native_session, callbacks, state, nullptr, &memory);

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Two unresolved moderate issues affect rejected DATA-stream resets and DATA-event pause/resume behavior.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/adapters/http2.cpp:232

• on_data accepts Http2CallbackAction::pause and maps it to NGHTTP2_ERR_PAUSE, but nghttp2 pauses DATA delivery until nghttp2_session_resume_data() is called; replaying the unconsumed input suffix, as receive() promises, does not resume that stream. A consumer that pauses on a data event therefore cannot reliably continue processing. Expose an explicit data-resume operation/state, or disallow pause for data events and document the supported pause points.

int on_data(nghttp2_session*, std::uint8_t flags, std::int32_t native_stream_id,
            const std::uint8_t* data, std::size_t length, void* user_data) noexcept {
  auto& state = *static_cast<NativeState*>(user_data);
  const auto data_view = core::ByteView::from(
      {reinterpret_cast<const std::byte*>(data), length});
  if (!data_view.has_value()) {
    return NGHTTP2_ERR_CALLBACK_FAILURE;
  }
  Http2Event event{};
  event.kind = Http2EventKind::data;
  event.stream = stream_id(native_stream_id);
  event.data = *data_view;
  event.end_stream = (flags & NGHTTP2_FLAG_END_STREAM) != 0U;
  return callback_result(state, event, true, false);

tests/adapters/http2.cpp:372

• This test creates both sessions without event sinks and only checks that control-frame submission succeeds. Consequently, regressions in the new reset and window_update event paths would still pass; attach sinks to the sessions and assert both event counters after receiving the generated frames.

    auto client = Http2Session::create(*worker, Http2Role::client, client_arena, limits(),
                                       Http2EventSink{&client_events, Events::write});
    auto server = Http2Session::create(*worker, Http2Role::server, server_arena, limits(),
                                       Http2EventSink{&server_events, Events::write});

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate HTTP/2 behavior and contract issues must be addressed before approval.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/adapters/http2.cpp:169

• reject_stream is advertised as a per-stream callback action, but this branch returns a negative nghttp2 callback code and receive() has no special handling for it, so the intentional rejection is reported as a generic dependency/corrupt-data error. That is indistinguishable from a receive/protocol failure to API consumers and can cause them to tear down the connection, unlike data rejection, which queues RST and returns consumed bytes. Normalize header rejection consistently as a stream reset or expose a distinct, resumable result contract.

src/adapters/contract/laghu/adapters/http2.hpp:75

• Http2Event exposes four borrowed ByteView fields, but the contract only documents ownership of the sink context and says nothing about when these views expire. A sink can copy the trivially-copyable event and later dereference name, value, data, or debug_data after nghttp2 has invalidated the storage. Document these fields as callback-scoped (or provide owned copies) so consumers cannot mistake the event for persistent data.

struct Http2Event final {
  Http2EventKind kind{Http2EventKind::frame_received};
  Http2StreamId stream{};
  core::ByteView name{};

src/adapters/contract/laghu/adapters/http2.hpp:124

• The session stores a non-owning arena pointer and require_valid()/release() dereference it, but this contract does not state the required lifetime ordering. As with src/adapters/contract/laghu/adapters/structured_data.hpp:74-76, the caller must destroy the session before resetting or destroying the arena; otherwise a reset can release the arena reservation and the later session destructor can dereference a dangling pointer.

  [[nodiscard]] static core::Result<Http2Session> create(
      core::WorkerId worker, Http2Role role, core::BoundedArena& arena,
      Http2Limits limits, Http2EventSink event_sink = {},
      DependencyLogSink log_sink = {}) noexcept;

src/adapters/contract/laghu/adapters/http2.hpp:148

• This contract exposes inbound DATA events and flow-control updates, but no operation for submitting DATA frames or a data provider. After submit_headers(..., false), callers have no way to send a request or response body through the adapter; the tests have to hand-encode DATA frames instead. Add a bounded outbound DATA/provider API, or explicitly scope this milestone to inbound DATA and adjust the stated client-server/framing coverage.

  [[nodiscard]] core::Result<Http2StreamId> submit_headers(
      std::span<const Http2Header> headers, bool end_stream) noexcept;
  [[nodiscard]] core::Result<void> submit_headers(
      Http2StreamId stream, std::span<const Http2Header> headers,
      bool end_stream) noexcept;
  [[nodiscard]] core::Result<void> submit_settings(
      std::span<const Http2Setting> settings) noexcept;
  [[nodiscard]] core::Result<void> submit_reset(Http2StreamId stream,
                                                std::uint32_t error_code) noexcept;
  [[nodiscard]] core::Result<void> submit_goaway(std::int32_t last_stream_id,
                                                 std::uint32_t error_code,
                                                 core::ByteView debug_data) noexcept;
  [[nodiscard]] core::Result<void> submit_connection_window_update(
      std::int32_t increment) noexcept;
  [[nodiscard]] core::Result<void> submit_stream_window_update(
      Http2StreamId stream, std::int32_t increment) noexcept;

src/adapters/http2.cpp:176

• After the sink returns reject_stream for a DATA event, this path only records the stream and returns 0. If the same receive() buffer contains another DATA frame/chunk for that stream, on_data invokes the sink again before this duplicate check, so data is delivered after rejection and the RST is delayed until all input is parsed. Track rejected streams before invoking the sink or stop parsing at the first rejection.

  if (action == Http2CallbackAction::reject_stream && event.kind == Http2EventKind::data &&
      event.stream.valid()) {
    const std::int32_t rejected_stream = event.stream.wire_value();
    for (std::size_t index = 0; index < state.pending_reset_count; ++index) {
      if (state.pending_reset_streams[index] == rejected_stream) {
        return 0;
      }

src/adapters/http2.cpp:376

• Http2Role is a public enum, but every value other than client is silently treated as server. A caller that passes an invalid enum value (for example after deserialization or a cast) therefore creates a session with the wrong wire role instead of receiving an input error. Validate both enum values before selecting the native constructor, consistent with the enum validation used by src/core/mapped_regions.cpp:23-29.

  if (role == Http2Role::client) {
    result = nghttp2_session_client_new3(&native_session, callbacks, state, options, &memory);
  } else {
    result = nghttp2_session_server_new3(&native_session, callbacks, state, options, &memory);

src/adapters/http2.cpp:166

• The public contract says that a callback may pause parsing, but this path only returns NGHTTP2_ERR_PAUSE when pausable is true. The adapter passes false for headers_begin, SETTINGS, reset, GOAWAY, window-update, and stream-closed events, so returning pause for any of those events becomes NGHTTP2_ERR_CALLBACK_FAILURE and surfaces as a fatal receive error. Either support pause consistently where nghttp2 permits it or document the event-specific restriction in the contract.

  if (action == Http2CallbackAction::pause && pausable) {
    return NGHTTP2_ERR_PAUSE;
  }

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Three unresolved moderate findings remain in the HTTP/2 implementation and arena test fixture.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

tests/adapters/http2.cpp:38

• storage is the backing block returned to BoundedArena, which places NativeState and AllocationHeader objects there with alignof(std::max_align_t). std::array<std::byte, ...> only has byte alignment, so this fixture can return misaligned storage and make the placement-new/custom allocator accesses undefined on targets where the stack address is not sufficiently aligned. Align this buffer as the bounded-arena tests do.

src/adapters/http2.cpp:273

• The per-header event leaves Http2Event::end_stream at its default false, even when the containing HEADERS frame carries END_STREAM. Consumers handling header events therefore cannot rely on the event metadata to detect end-of-stream (and the field is already populated for headers_begin and frame_received). Copy the frame flag into this event as well.

  event.sensitive = (flags & NGHTTP2_NV_FLAG_NO_INDEX) != 0U;

src/adapters/http2.cpp:171

• reject_stream is documented as supported for headers_begin and header, but this branch turns those callbacks into NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE before the pending-reset path below can run. A policy rejection during header processing therefore aborts receive() without queuing/sending an RST_STREAM, unlike a DATA rejection; implement the same deferred stream-reset behavior for header events or change the public contract.

  if (action == Http2CallbackAction::reject_stream && rejectable) {
    return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A critical memory-reallocation issue and two moderate HTTP/2 behavior issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

src/adapters/http2.cpp:171

• The contract documents reject_stream as supported for header-begin and header events, but this branch returns NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE directly from the nghttp2 receive call. receive() then treats the negative result as a dependency/protocol error, unlike the DATA path below which queues a reset and returns the consumed input. A normal header rejection is therefore surfaced as a failed session operation; normalize this action consistently (queue/reset and preserve progress, or explicitly handle the expected temporal result) instead of returning it as http2_receive corruption.

  if (action == Http2CallbackAction::reject_stream && rejectable) {
    return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;

src/adapters/http2.cpp:220

• A provider can return the default more state with zero bytes, and this check accepts it. Because deferred is the explicit no-data state, next_output() can make no progress (or repeatedly emit empty DATA frames) instead of surfacing invalid provider output. Reject more/0 here so a provider cannot stall the session.

  if (produced->bytes > length ||
      (produced->state == Http2DataReadState::deferred && produced->bytes != 0U)) {
    state.data_provider_error = core_error(core::ErrorCode::invalid_range,
                                           "HTTP/2 data provider returned an invalid size");
    state.data_provider_failed = true;

tests/adapters/http2.cpp:353

• ended_frames was already incremented by the earlier "abc" DATA frame, which also has END_STREAM, so this condition is already true before the empty DATA frame is received. Capture the counter before this exchange and assert that it increases to actually cover the empty end-stream frame.

    const auto empty_stream = client->submit_headers(headers, false);
    if (!empty_stream.has_value() || !transfer(*client, *server) ||
        !receive_data_frame(*server, empty_stream->wire_value(), {}, true) ||
        server_events.ended_frames == 0U) {

tests/adapters/http2.cpp:346

• server_events.data_ended was set by the earlier inbound "abc" DATA frame and is never cleared, so this assertion can pass even if the deferred outbound provider does not emit END_STREAM. Reset a per-exchange flag or track the ending event separately before testing the outbound transfer.

        server_events.data_events <= data_events_before_resume || !server_events.data_ended) {

tests/adapters/http2.cpp:483

• This only checks that submit_connection_window_update accepts the request; its output is never drained or passed to the peer, so the connection-level WINDOW_UPDATE path is not covered by the control-frame test. Transfer the pending output to the client (and preferably assert the corresponding event) here.

    return server->submit_connection_window_update(1024).has_value();

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The HTTP/2 adapter and build/CI integration span multiple components and require final human review.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Lite