fix: enforce HTTP attempt wall-clock deadlines - #35
Conversation
There was a problem hiding this comment.
Deterministic Validator approval for exact head 4789f1d34cfb77a1378d8fcdb9b9d010d0b844aa.
Ticket: ticket-007
Correlation ID: subllm-pr-35-ticket-007-merged-logs
Model: zai/glm-5.3
Reviewed diff chunks: 4
Advisory LLM verdict: REQUEST_CHANGES
Advisory summary: Reviewed all 4 diff chunk(s). Ticket-007 moves OpenAI-compatible attempts into an isolated worker process with wall-clock timeout and process-group cleanup. The visible chunk shows sound design (credential via stdin, size-capped output, strict schema validation of worker results). However, this chunk ends mid-function while validating the worker result schema, so completeness cannot be fully assessed here; the remaining chunks cover the worker implementation and tests. | Chunk 2 introduces the policy-validating OpenAI worker subprocess and strict result-validation in the client. The worker enforces a closed request schema, policy-approved provider base/wire model/credential shape, request-size cap, provider-approved request fields, and safe extra headers (blocks Authorization/Content-Type override and CRLF injection). HTTP status classification and transport error handling look sound. Tests were refactored to exercise the worker boundary, and a hanging-worker fixture verifies process-group reaping. | Test chunk refactors mocks from urllib-level interception to the new worker-based interface, asserting request payloads (api_base, wire_model, messages, request_fields, extra_headers) and structured worker error results (timeout, http_429, http_400) for failover, cooldown, and retry-policy scenarios. Adds a real-process regression proving the OpenAI worker deadline reaps descendant processes. Tests align with the PR's stated behavior; no source code in this chunk. | Chunk 4 adds openai_worker tests covering policy-owned request fields, secret-free retryable HTTP error receipts, and rejection of non-policy provider bases. Tests pass and assertions are sound.
Advisory findings: _run_openai_worker validation logic is cut off mid-expression in this chunk; if the schema check is incomplete in the final code, malformed worker output could be misclassified. Must be confirmed in chunk 2/3.
The LLM output above is advisory and was not used as the approval trust root.
Merge will be attempted after this approval when explicitly authorized.
Decision record (recomputable)
DECISION D-007-7543
TICKET ticket-007
HEAD_SHA 4789f1d34cfb77a1378d8fcdb9b9d010d0b844aa
CORRELATION_ID subllm-pr-35-ticket-007-merged-logs
ACTOR agent:ifuri-validator-agent[bot]
APPLIED_RULE P-CORE-015
INPUT author_login = "tom-sapletta-com"
INPUT observed_checks = ["onedev/local-verify=PASS"]
INPUT required_checks = ["onedev/local-verify"]
INPUT required_checks_source = "protected registry (env/request)"
INPUT reviewer_login = "ifuri-validator-agent[bot]"
INPUT superseded_checks = ["Analyze (actions)","Analyze (python)","test (3.11)","test (3.12)","test (3.13)"]
VERDICT APPROVE AUTHORITY DETERMINISTIC
REJECTED REQUEST_CHANGES BECAUSE NO_UNSAFE_CHANGE_REASON_FOUND
ADVISORY llm_verdict = "REQUEST_CHANGES" MODEL "zai/glm-5.3"
ASSERT VERDICT_AUTHORITY != "ADVISORY"
Closes #34.
Runs every OpenAI-compatible attempt in a fixed, policy-validating stdin/stdout worker and a new process session. The parent enforces the configured attempt timeout as a total wall-clock deadline, reaps the process group, records a secret-free outcome, and continues bounded failover.
Verification:
./scripts/verify— 197 passed; Ruff, compileall, wheel and sdist passed