Skip to content

feat(api): reconcile response recording HTTP with current main - #415

Open
seonghobae wants to merge 60 commits into
mainfrom
reconcile/response-http-main-20260826
Open

feat(api): reconcile response recording HTTP with current main#415
seonghobae wants to merge 60 commits into
mainfrom
reconcile/response-http-main-20260826

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Why

Protected main@5f0a5346d60602d4bdfbca526d125f9504d594d3 can create/reload sessions and read/export immutable results, but participants still cannot record an answer through the public HTTP surface. Historical Draft #195 carries the original response-HTTP implementation lineage but is rooted at stale ancestry and predates the current authoritative-session response ledger contract.

Reconciliation

This branch is reconciled onto current protected main without force-push or destructive rebase. The historical #195 response handler, contract test, and OpenAPI operation were replayed, then the source was updated to current ResponseLedger::record(&AssessmentSession, ...) semantics rather than reintroducing detached SessionState authority. Session-reference validation requires exact normalized spelling, and SessionMismatch maps to a stable fail-closed problem response.

Historical #195 exact source head used for lineage: 0f2e49533110938d8b9330452f2015ac66cf20e6.

Behavior

  • POST /v1/sessions/{session_ref}/responses records one item response only for an Active session bound to the published release containing that exact item version.
  • Idempotency-Key is the client-event identity; exact replay returns the original event and conflicting reuse fails closed.
  • Response recording passes the authoritative AssessmentSession aggregate into the response ledger, preserving the protected-main session/ledger ownership invariant.
  • Encoded, padded, normalized aliases, numeric-like session identities, invalid response references, foreign items, malformed digests, and unknown sessions fail closed with RFC 9457 problem details that do not echo response payloads.
  • openapi/responses.yaml is limited to the implemented response-write operation and does not claim unrelated session, command, result, or persistence behavior.
  • response_http_boundary.rs is the sole socket-framing owner; the application module no longer carries a second divergent accept/read/write loop.
  • Direct handler framing fails closed when byte Content-Length would split a UTF-8 scalar instead of slicing a Rust string at a non-character boundary.

RED → GREEN evidence lineage

  • RED c8706c9758045f6b38c69085d5d72ca5e4130106 replays the historical tests/source onto current-main lineage, where the old detached-state call is incompatible with the protected-main ResponseLedger::record signature.
  • GREEN a13b60293ae7c0e6fbc004f344a54ce348552d81 binds new writes to the authoritative session aggregate and covers SessionMismatch problem mapping.
  • RED b2c4361379e692b9e8bf0fbd065b4c874e87c349 requires one response-HTTP socket framing owner.
  • GREEN afc9ee7346611a7e66b2a57709fbca54d3d4fe90 removes the duplicate legacy framing loop from src/response_http.rs and keeps framing in src/response_http_boundary.rs.
  • Regression 146453aaf188208c531afccfc372c15128f15d7b proves direct-handler byte lengths that split UTF-8 fail closed.
  • ADR-0014 and canonical TRACEABILITY are reconciled on this branch without promoting this active PR to protected-main truth.
  • Exact-head CI/review evidence is authoritative; historical feat(api): record active-session responses over HTTP #195 checks do not transfer.

Required before merge

  • Unchanged exact-head Runtime CI, security/SAST, exact owned coverage/docstrings, supply-chain evidence, and any live repository-required checks must pass.
  • Address only valid current-head findings and leave no valid unresolved review thread.
  • Obtain a qualifying independent non-author approval under the live ruleset; administrator bypass is not acceptable.

Supersession

This PR supersedes stale Draft #195 as the current-main response-HTTP landing vehicle. Do not merge both lanes.

Summary by CodeRabbit

  • 새로운 기능

    • 활성 세션에 응답을 기록하는 API를 추가했습니다.
    • 인증된 참가자와 유효한 익명 자격 증명만 응답을 기록할 수 있습니다.
    • 멱등성 키를 사용한 안전한 재전송을 지원합니다.
    • 표준 오류 응답과 Allow: POST 헤더를 제공합니다.
    • HTTP 헤더, 본문 길이, UTF-8 및 요청 프레이밍을 엄격히 검증합니다.
  • 문서

    • OpenAPI, 추적성 문서 및 변경 로그를 최신 구현 상태에 맞게 갱신했습니다.
  • 테스트

    • 인증, 멀티세션, 중복 요청, 프레이밍 및 입력 검증 계약 테스트를 확장했습니다.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bdaa8ed9-7258-4fa0-b41a-9fdf429bdcb9

📥 Commits

Reviewing files that changed from the base of the PR and between 9f0e448 and 7b351b4.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • docs/DOCUMENTATION_ASSESSMENT.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.md
  • docs/DOCUMENTATION_ASSESSMENT.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

활성 세션의 응답 이벤트 기록을 위한 OpenAPI 계약과 인증된 HTTP/1.1 구현이 추가되었다. 멱등성, 세션 권한, 이벤트 참조 충돌, RFC 9457 오류 및 요청 프레이밍을 검증하는 계약 테스트도 추가되었다.

Changes

응답 기록 HTTP 전송

Layer / File(s) Summary
API 계약과 추적 기준
openapi/responses.yaml, docs/TRACEABILITY.md, docs/adr/..., docs/DOCUMENTATION_ASSESSMENT.md, CHANGELOG.md
응답 기록 API와 관련 스키마를 추가했다. protected-main과 Active PR의 구현 증거 및 추적 기준을 갱신했다.
권한 기반 응답 기록
src/lib.rs, src/response_http.rs
인증된 참가자와 익명 자격을 검증한다. 권한이 확인된 세션만 응답 이벤트를 기록한다. 멱등 재생은 200, 신규 기록은 201, 권한 실패는 404를 반환한다.
HTTP 요청 경계와 프레이밍
src/response_http_boundary.rs, src/response_http_metadata.rs
권한 없는 경로와 권한 검증 경로를 분리했다. CRLF, 헤더 이름, 중복 헤더, Content-Length, Transfer-Encoding, 제한시간 및 Allow: POST 메타데이터를 처리한다.
HTTP 계약 검증
tests/response_http*_contract.rs
권한, 멱등성, 이벤트 참조 충돌, 세션 격리, 헤더 위생, UTF-8 본문 길이 및 TCP 프레이밍 계약을 검증한다.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🔵 Low · up to 7b351

The response-recording behavior is mergeable, but the documentation assessment still lists a future evaluation date, which makes the baseline evidence timing inaccurate and should be corrected or explicitly acknowledged by the owner.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant response_http_boundary
  participant handle_authorized_response_http_request
  participant ResponseHttpRuntime
  Client->>response_http_boundary: POST 응답 요청 전송
  response_http_boundary->>handle_authorized_response_http_request: 프레이밍 검증 요청 전달
  handle_authorized_response_http_request->>ResponseHttpRuntime: 참가자 권한과 세션 검증
  ResponseHttpRuntime-->>handle_authorized_response_http_request: 권한 결과 반환
  handle_authorized_response_http_request->>ResponseHttpRuntime: 멱등성 검사와 이벤트 기록
  response_http_boundary-->>Client: HTTP 응답 전송
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 138 functions across 15 files. (2 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 현재 main에 맞춘 response recording HTTP 구현 조정이라는 주요 변경 사항을 정확히 요약합니다. 간결하고 구체적이며 변경 내용과 일치합니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 138 functions across 15 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch reconcile/response-http-main-20260826

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 26, 2026 09:53
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae marked this pull request as draft August 26, 2026 13:07
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Devin Review

Comment thread src/response_http.rs
Comment thread src/response_http.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant