Skip to content

fix(fresh): forward durable State-Protocol query parameters through the chat stream proxy - #1556

Merged
rickylabs merged 2 commits into
mainfrom
fix/1457-chat-proxy-query-forwarding
Aug 12, 2026
Merged

fix(fresh): forward durable State-Protocol query parameters through the chat stream proxy#1556
rickylabs merged 2 commits into
mainfrom
fix/1457-chat-proxy-query-forwarding

Conversation

@rickylabs

@rickylabs rickylabs commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Summary

Forwards durable State-Protocol query parameters through the Fresh chat stream proxy. Incoming parameters are forwarded by default except id; resolved upstream query values remain authoritative, repeated keys survive, and callers can replace the default with a documented query mapper.

Scope

Slices

  • S1 Forward and merge chat proxy query parameters with focused regression tests — 3baaaf820
  • S1 evidence Format the owned source and record final gates — 27a5bb50d

Validation

  • Fresh scoped check — PASS: 182 files, 0 findings
  • Fresh scoped lint — PASS: 182 files, 0 findings
  • Fresh scoped format — PASS: 182 files, 0 findings
  • deno task quality:gate — PASS, exit 0; configured roots omit packages/fresh
  • Explicit Fresh quality scan — PASS: 0 findings
  • Explicit Fresh doctrine scan — PASS: FAIL=0 with pre-existing warnings
  • deno task --cwd packages/fresh test — PASS: 223 passed, 0 failed
  • deno task doc:lint --root packages/fresh --pretty — exit 0; changed AI entrypoint 0 findings, 44 pre-existing package findings in untouched query/route/streams files
  • Full command output is pasted in the [PHASE: IMPL] comment and summarized in the slice worklog.

Harness

  • Run dir: .llm/runs/release-0.0.6-features--orchestration/
  • Slice artifacts: slices/implement-1457.md, slices/worklog-1457.md
  • Phase: impl
  • PLAN-EVAL: N/A — small fully locked D1–D4 contract; no open implementation decisions.
  • IMPL-EVAL: pending orchestrator-owned separate session; this PR remains draft.

Drift / Debt

  • No architecture debt or scope drift.
  • Live issue fix(fresh/ai): chat stream proxy drops durable State-Protocol query parameters #1457 currently contains no markdown acceptance checkboxes, so there are no truthful box-index entries to map. The required fenced block is retained with an empty entry list; no issue text was invented.
  • The first format check was red on one owned expression; formatting only stream-proxy.ts made the required wrapper green. The post-format grep confirmed all introduced names survived.
  • Doc lint exits 0 despite 44 legacy findings outside the changed AI surface; this is reported, not silently treated as a clean package.

Definition of Done

  • Default forwarding preserves offset, live, handle, and cursor while dropping id.
  • Resolved upstream values win collisions and repeated incoming keys remain repeated.
  • The documented query hook replaces default forwarding and no-query URLs remain unchanged.
  • All required package, quality, test, and documentation gates ran with recorded output.
  • Separate-session harness evaluation is handed back to the orchestrator.

No acceptance-evidence block here: issue #1457 states its expectations as prose and contains no acceptance checkboxes, so the mirror has nothing to tick. Emitting the block with an empty entry list is not a harmless no-op — the mirror's parser rejects that line outright and fails the job, which is what made close-gate red on the first two runs.

Copy link
Copy Markdown
Owner Author

[PHASE: IMPL]

Landed #1457 query forwarding in 3baaaf8200fd7fb79edf6ad2d383e2cefa5c8c1a; formatted the owned source and recorded final evidence in 27a5bb50da058162085afbcdc5cfaef15bdde484.

Scope

Gate output

$ deno run --allow-read --allow-run .llm/tools/run-deno-check.ts --root packages/fresh --ext ts,tsx
{"source":{"mode":"selection","cwd":"/home/codex/repos/ns006-1457"},"command":"deno check --unstable-kv <files>","selection":{"filesSelected":182,"batches":2,"failedBatches":0},"summary":{"totalOccurrences":0,"uniqueOccurrences":0,"uniqueCodes":0,"uniquePaths":0},"groups":[]}
exit code: 0

$ deno run --allow-read --allow-run .llm/tools/run-deno-lint.ts --root packages/fresh --ext ts,tsx
{"source":{"mode":"command","cwd":"/home/codex/repos/ns006-1457","exitCode":0},"selection":{"filesSelected":182,"batches":1},"summary":{"totalOccurrences":0,"uniqueOccurrences":0,"uniqueRules":0,"uniquePaths":0},"groups":[]}
exit code: 0

$ deno run --allow-read --allow-run .llm/tools/run-deno-fmt.ts --root packages/fresh --ext ts,tsx
{"command":"deno fmt --check","cwd":"/home/codex/repos/ns006-1457","mode":"check","summary":{"filesSelected":182,"batches":1,"failedBatches":0,"findings":0,"ignoredFindings":0},"findings":[]}
exit code: 0

$ deno task quality:gate
Task quality:scan deno run --allow-read .llm/tools/quality/scan-code-quality.ts
{"ok":true,"mode":"repository","scanned":["packages/cli/src","plugins"],"findings":[],"allowCount":7,...}
...
# Doctrine readiness — ai
  FAIL=0 WARN=5 INFO=1
exit code: 0

$ deno run --allow-read .llm/tools/quality/scan-code-quality.ts --root packages/fresh
{"ok":true,"mode":"repository","scanned":["packages/fresh"],"findings":[],"allowCount":1,"allowances":[{"file":"packages/fresh/src/application/builders/define-page/builder/route-support.ts","line":96,"reason":"DefinePageWithRouteContract preserves prior path/search output when either optional schema is omitted, but BoundRouteContract maps an omitted schema to EmptyRecord; TypeScript cannot equate those conditional states without presence-specific legacy builder overloads"}]}
exit code: 0

$ deno run --allow-read .llm/tools/fitness/check-doctrine.ts --root packages/fresh
# Doctrine readiness — fresh
  FAIL=0 WARN=3 INFO=1
  WARN A8/AP-1/F-1: file is 685 lines (cap 500) — split into smaller single-reason files (src/runtime/ai/create-chat-connection.ts)
  WARN A8/AP-1/F-1: file is 604 lines (cap 500) — split into smaller single-reason files (src/application/route/manifest.ts)
  WARN F-16: directory has 13 immediate children; doctrine cap is 12 (src/runtime/ai)
  INFO A9: docs/architecture.md missing — required when public symbols > 25
exit code: 0

$ deno task --cwd packages/fresh test
ok | 223 passed | 0 failed (4s)
exit code: 0

$ deno task doc:lint --root packages/fresh --pretty
"summary": {
  "totalPackages": 1,
  "totalErrors": 44,
  "totalPrivateTypeRef": 27,
  "totalMissingJSDoc": 17,
  "totalOther": 0
}
"./src/runtime/ai/mod.ts": privateTypeRef=0 missingJSDoc=0 other=0 total=0
exit code: 0

The configured quality:gate roots omit packages/fresh (#1542), so the explicit Fresh quality and doctrine scans above are the package verdict. Doc lint exits 0 but reports 44 pre-existing findings in untouched query/route/streams files; the changed AI entrypoint reports zero.

The first format check was red on the new forwardedQuery expression. I formatted only stream-proxy.ts, reran to green, and re-grepped offset, live, handle, cursor, forwardedQuery, resolvedQueryNames, and the query option; all remained present.

Next

  • Orchestrator: run the separate-session review/evaluation and decide when to transition the draft. This implementation session did not mark ready or merge.

@rickylabs

Copy link
Copy Markdown
Owner Author

[PHASE: REVIEW]

Tier-A slice review by the orchestrator. Claims re-verified in the worktree, not read off the report.

Conformance to the locked decisions

Decision Implemented
D1 forward all incoming params except id by default yes — [...incomingQuery].filter(([name]) => name !== 'id')
D2 merge onto the resolved URL, resolved value wins, repeated keys preserved yes, and correctlyresolvedQueryNames is captured before the append loop, so client input can never override a key the resolved streamPath already set, and append (not set) keeps repeats
D3 optional query hook replaces the default yes, documented on the option
D4 headers, auth, body, streaming, abort, sanitization untouched yes — the accept-encoding: identity and duplex: 'half' paths are unchanged

D2 is the one worth calling out: the anti-forgery direction is the part a reasonable implementation could have got backwards, and getting it backwards would let a client override a configured streamPath parameter. The set is snapshotted before mutation, which is what makes it hold.

Verified independently

  • deno task --cwd packages/fresh test223 passed, 0 failed.
  • Negative case demonstrated. Reverting the forwarding (resolved URL only, incoming query dropped) → 217 passed, 6 failed, including preserves repeated incoming query keys and query hook replaces default forwarding…. Restored; tree clean; 223/223 again.
  • Notably one of the 6 is the pre-existing supports eis-chat per-session stream paths and survives identity-negotiated gzip mislabel (#219) test, so the existing suite now genuinely covers the forwarded query rather than only the new tests doing so.

Findings

None blocking.

Next

Flipping to ready, which fires the automatic IMPL-EVAL per the label-driven policy (drift D-4/D-5). No manual OpenHands dispatch, no local evaluator. impl-eval:skip is deliberately not applied even though this is the small-specified class — the automatic policy is what was asked for.

@rickylabs
rickylabs marked this pull request as ready for review August 12, 2026 11:13
@rickylabs

Copy link
Copy Markdown
Owner Author

@openhands-agent model=openrouter/deepseek/deepseek-v4-flash-0731 output=pr-comment iterations=800 phase=impl head=27a5bb50da058162085afbcdc5cfaef15bdde484

Trusted base SHA: f99cb4f
Evaluated head SHA: 27a5bb5

use harness

SKILL

  • netscript-harness — apply the formal IMPL-EVAL protocol and verdict vocabulary.
  • openhands-handoff — publish one machine-readable OpenHands verdict.
  • netscript-tools — run the smallest decisive repository-native gates without mutating source.
  • netscript-doctrine — apply package/plugin doctrine when the changed surface requires it.

Act as the formal IMPL-EVAL session for this pull request. Do not edit files, create commits, push,
or repair findings. The trigger metadata supplies the trusted base SHA and immutable head SHA: read
the evaluator protocol, verdict definitions, and selected profiles from that base commit, then
evaluate the PR body, linked issues, run artifacts, final diff, review threads, and architecture
debt at the immutable head. Verify the approved plan or recorded PLAN-EVAL: N/A, design
checkpoint, acceptance criteria, static/runtime/consumer gates, public surface, lock hygiene, and
false-done states. For documentation changes, also read every changed document fully and hand-test
representative executable claims.

Return concise, severity-ranked findings with exact evidence and required action. End with exactly
one supported verdict line using OPENHANDS_VERDICT: PASS, OPENHANDS_VERDICT: FAIL_FIX,
OPENHANDS_VERDICT: FAIL_RESCOPE, OPENHANDS_VERDICT: FAIL_DEBT, or
OPENHANDS_VERDICT: FAIL_PLAN. Write the same verdict to OPENHANDS_SUMMARY_PATH.

@rickylabs

rickylabs commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

OpenHands Agent — Completed

OPENHANDS_VERDICT: PASS

Model: openrouter/deepseek/deepseek-v4-flash-0731
Provider: OPENROUTER
Job status: success
Verdict source: summary-file
Verdict comment: #1556 (comment)
Summary source: agent
Housekeeping: ack=success trace=success commit-artifacts=skipped replies=skipped draft-pr=skipped
Trace: .llm/tmp/run/openhands/pr-1556/run-31590876488-1 (in the Actions artifact)
Artifact: openhands-agent-31590876488-1

IMPL-EVAL — PR #1556 (fix fresh chat proxy query forwarding)

Verdict: OPENHANDS_VERDICT: PASS

Summary

Formal IMPL-EVAL of PR #1556 fix(fresh): forward durable State-Protocol query parameters through the chat stream proxy, closing issue #1457. Evaluated at immutable head 27a5bb50d against trusted
base f99cb4fbf. Implementation surface is packages/fresh/src/runtime/ai/stream-proxy.ts plus
focused tests; run artifacts under .llm/runs/release-0.0.6-features--orchestration/slices/.

Changes

  • stream-proxy.ts: resolved upstream URL is dereferenced to URL; incoming query is forwarded by
    default except id; resolvedQueryNames is snapshotted before the append loop so resolved
    streamPath query wins collisions; repeated client keys are preserved via append; optional
    documented query(URLSearchParams): URLSearchParams hook replaces the default and can suppress or
    add parameters. Auth/header/body/duplex/abort/sanitization paths untouched.
  • stream-proxy_test.ts: 6 new regression tests (default forwarding of offset/live/handle/cursor,
    id dropped, resolved-value-wins collision, repeated keys preserved, hook replacement, no-query
    stability). The pre-existing eis-chat test expectation was updated so the upstream URL now carries
    the forwarded ?session=eis-123 (per locked decision D1, everything except id is forwarded).

Validation (independently re-run)

  • deno check --unstable-kv src/runtime/ai/stream-proxy.ts — PASS.
  • Focused deno test src/runtime/ai/stream-proxy_test.ts — 9/9 passed.
  • Full deno task --cwd packages/fresh test223 passed, 0 failed.
  • PR [PHASE: IMPL] evidence records scoped check/lint/fmt (182 files, 0 findings), quality:gate,
    explicit Fresh quality scan (0 findings), doctrine scan (FAIL=0, pre-existing warnings), Full
    doc:lint exit 0 with 44 pre-existing untouched findings; changed AI entrypoint 0 findings.

Responses to review comments

The Tier-A REVIEW comment independently re-verified D1–D4 and demonstrated the negative case
(reverting forwarding → 217 passed, 6 failed, restoring → 223). No unresolved review threads. No
manual responses required from this evaluator session.

Findings (severity-ranked)

  • [Info] session routing param now forwarded upstream in eis-chat path. The eis-chat target
    resolves sessionId from the incoming session query while the proxy now also forwards session
    upstream, duplicating it alongside the path-embedded session id. This is the documented D1 default
    (drop only id; the issue names only id as droppable routing metadata) and the upstream simply
    ignores the duplicate. Not a defect; callers who mind may use the query hook. No action required.
  • [Info] eis-chat PR chore(release): harden release:cut + preflight for deterministic alpha cuts (#147) #169 adapter cleanup is a consumer-repo follow-up. Issue fix(fresh/ai): chat stream proxy drops durable State-Protocol query parameters #1457 notes the
    adapter "Remove it once the published helper owns the behavior"; that removal lives in the
    consumer repo and is explicitly out of this PR's scope (Excludes). Not a blocking gate.
  • No doctrine violation introduced; no lock-file/dependency churn (additive optional callback only,
    no export or dependency changes).

Remaining risks

  • Upstream services that cannot tolerate unknown/duplicate query parameters would need the query
    hook; current durable-streams behavior is tolerant.
  • No documentation-only hand-testing needed; docs are option-level JSDoc, reviewed in full.

Verdict

OPENHANDS_VERDICT: PASS

Run: https://github.com/rickylabs/netscript/actions/runs/31590876488

rickylabs added a commit that referenced this pull request Aug 12, 2026
…ble-trigger

All three evaluations running on the automatic label-driven policy with exactly
one summary comment each. Records that flipping #1556 to ready and then also
swapping to status:impl-eval produced two dispatcher runs -- no duplicate spend
observed, but avoidable, and not repeated for the other two.

Records the #1459 pre-dispatch rescope: its client-bundle regression criterion
moved to #1557 rather than being ticked on evidence that does not prove it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TKxrWGp5uxEHQ2NyZiZSMF
@rickylabs

Copy link
Copy Markdown
Owner Author

OPENHANDS_VERDICT: PASS

Formal IMPL-EVAL — PR #1556 (head 27a5bb50d vs trusted base f99cb4fbf), closing #1457.

Verdict: PASS. Approved small-scope fix fully satisfies the locked contract and issue acceptance criteria. No blocking findings.

Verified independently (run, not just read):

  • deno check --unstable-kv on stream-proxy.ts — PASS.
  • Focused stream-proxy_test.ts — 9/9 passed (6 new regression tests: default forward of offset/live/handle/cursor, id dropped, resolved-value-wins collision, repeated keys preserved, query hook replacement/suppression, no-query URL stability).
  • Full deno task --cwd packages/fresh test223 passed, 0 failed.
  • PR [PHASE: IMPL] evidence records scoped check/lint/fmt (182 files, 0 findings), quality/doctrine scans (FAIL=0; pre-existing warnings only), and doc:lint exit 0 (changed AI entrypoint 0 findings; 44 pre-existing out-of-scope).
  • Tier-A REVIEW thread independently re-verified D1–D4 including the negative case (revert → 217/6, restore → 223). No unresolved threads.

Plan/process gates: PLAN-EVAL: N/A recorded and justified (owner brief locks D1–D4); Design checkpoint present in worklog-1457.md; run artifacts updated.

Acceptance (issue #1457): default forwarding + documented query mapping hook present, with tests for offset, live, handle, cursor. id remains removable routing metadata.

Surface/hygiene: additive optional query callback with explicit types + public JSDoc; no export/dependency changes; no lock-file churn.

Non-blocking observations: (1) eis-chat session routing param is now also forwarded upstream (documented D1 default; dedup unneeded, harmless); (2) eis-chat PR #169 adapter removal is a consumer-repo follow-up explicitly excluded from this scope. Both recorded as info, no action required.

No findings requiring remediation. Created by an AI agent (OpenHands) on behalf of the user.

@rickylabs rickylabs added status:augment-review Advisory augment/review pass status:ready-merge and removed status:impl-eval status:augment-review Advisory augment/review pass labels Aug 12, 2026
rickylabs added a commit that referenced this pull request Aug 12, 2026
@rickylabs
rickylabs merged commit 5db37e7 into main Aug 12, 2026
55 of 58 checks passed
@github-actions github-actions Bot added the canary:0.0.6-canary.2 Published NetScript prerelease 0.0.6-canary.2 label Aug 12, 2026
@rickylabs
rickylabs deleted the fix/1457-chat-proxy-query-forwarding branch August 13, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(fresh/ai): chat stream proxy drops durable State-Protocol query parameters

1 participant