Skip to content

fix: authenticate log streaming, artifact downloads, and video playback - #1531

Draft
Josh Duffney (duffney) wants to merge 2 commits into
mainfrom
duffney-authenticated-delivery-extraction
Draft

Josh Duffney (duffney) wants to merge 2 commits into
mainfrom
duffney-authenticated-delivery-extraction

Conversation

@duffney

Copy link
Copy Markdown
Collaborator

Summary

Authenticate all three browser delivery surfaces through the existing bearer transport: live/stored SSE logs, buffered artifact/archive downloads, and native progressive video playback/seeking. Bind delivery to account, project, resource and resolved attempt; cancel replaced sessions and never substitute latest artifacts for unavailable historical attempts.

Video uses a narrowly routed, client-bound service-worker bridge with authenticated range requests, backpressure and cancellation, rather than full-recording Blob buffering. The API validates ranges/selectors and streams abortably; nginx serves the actual worker asset and denies uncontrolled bridge routes. Tokens stay out of URLs and the worker; protected responses are not persisted in CacheStorage.

This is the independently usable lower layer, extracted onto 2101bd0d61c877caadb5c60d03056c67a0b08296 from final source 1f8ea3c92f03b1516c9483fb02c0c47c6c4ccd48. It does not introduce new RBAC, memberships, capability policy, authentication-mode changes, mutation idempotency or report shutdown. Existing bearer/login transport, project scoping, report functionality and no-token rollout remain intact. The separate upper RBAC contribution will depend on this branch.

Demos

User-visible delivery fixes; before/after walkthrough recordings are not attached. This remains a draft. Generated media was used for the browser checks below, not as a walkthrough or real-worker recording.

Before

Not recorded. Raw browser delivery could not consistently carry the Portal bearer to protected logs, downloads and native media requests.

After

Not recorded. The isolated browser checks below verified authenticated delivery and native progressive playback/seek.

Testing

Previously executed against this exact, unchanged extraction checkpoint; broad suites were not rerun solely for publication.

Passed:

pnpm exec vitest run apps/portal/src/lib/log-stream.test.ts apps/portal/src/hooks/use-log-stream.test.ts apps/portal/src/lib/api-client.test.ts apps/portal/src/lib/artifact-delivery.test.ts apps/portal/src/hooks/use-artifact-session.test.tsx apps/portal/src/hooks/useHarExtraction.test.tsx apps/portal/src/components/ArtifactDownloadButton.test.tsx apps/portal/src/components/artifact-consumers.test.tsx apps/portal/src/components/artifact-stories.play.test.tsx apps/portal/src/pages/RunDetail.artifacts.test.tsx apps/portal/src/lib/authenticated-media.test.ts apps/portal/src/components/VideoPlayer.test.tsx apps/portal/src/components/VideoPlayer.play.test.tsx apps/portal/src/components/LogViewer.scroll.test.tsx --reporter=dot

Result: 14 files, 203 tests passed.

pnpm exec vitest run apps/portal/src --reporter=dot

Result: 79 files, 658 tests passed.

pnpm exec vitest run apps/api/src/routes/requests/video.test.ts apps/api/src/routes/requests/archive.test.ts apps/api/src/routes/requests/logs.test.ts apps/api/src/routes/requests/har.test.ts apps/api/src/routes/requests/atif.test.ts apps/api/src/routes/requests/snapshots.test.ts apps/api/src/routes/requests/tool-calls.test.ts --reporter=default

Result: 7 files, 132 tests passed.

Exact command Result
pnpm --filter shared build Passed
pnpm --filter telemetry build Passed
pnpm --filter db-migrations build Passed; startup prerequisite, no migration changes
pnpm --filter api build Passed
pnpm --filter portal build Passed
pnpm --filter portal build-storybook Passed
pnpm notice:check Passed
pnpm install --frozen-lockfile --ignore-scripts --offline Passed
git diff --check Passed; staged equivalent also passed before commit
git merge-base --is-ancestor 2101bd0d61c877caadb5c60d03056c67a0b08296 HEAD Passed

Genuine isolated non-RBAC browser acceptance passed: Chromium 156.0.8078.12, actual Vite Portal and built API, separate MongoDB/Redis/Azurite/Entra Local fixtures, normal trusted localhost TLS, real MSAL redirect and Scope enrollment. No mocked authentication, benchmark, worker, scheduler or external deployment.

  • Actual archive download contained exact nested snapshot proof bytes.
  • A generated 13,611,229-byte, 90-second WebM advanced 20 native frames after only 201,447 upstream bytes at 250,000 bytes/s. Native seek beyond the buffered interval issued bytes=10059776-, resumed at 70.2148 seconds and canceled two obsolete transfers.
  • Setup, iteration and historical recording metadata loaded; missing historical selection exposed no latest archive. Current/historical stored logs remained isolated; live SSE rendered a synthetic Redis event through final completion.
  • Project replacement and actual logout/different-account login invalidated the old bridge; a different browser client received 403. The emulator required manually returning from its signed-out page.
  • An injected first-range 401 recovered through existing MSAL force-refresh transport; an injected media 403 displayed an error without a playable source. The real API separately returned invalid-bearer 401, disabled-user 403 and unsatisfiable-range 416 with exact length metadata.
  • All observed upstream video requests carried the bearer; CacheStorage remained empty and no page errors occurred. Temporary services and fixture containers were stopped afterward.

Limits: production nginx/prefix-hosted acceptance, Safari/Firefox, worker restart/idle expiry, every interrupted-body/delayed-history browser case, top-level one-shot recording, real-worker capture/upload and Mongo change-stream completion were not browser-established. Standalone Mongo emitted its expected change-stream limitation during live-log checking; Redis final-event completion passed. Protocol/component/API tests are not claimed as native-browser proof for these remaining cases. No upper-layer/RBAC acceptance is claimed. Existing no-token rollout remains unchanged. Existing Browserslist/bundle-size and some Portal test warnings were nonblocking.

Documentation and compatibility

Updated realtime data flow, authenticated progressive recordings, codebase delivery and the architecture index in AGENTS.md.

No database migration or new CLI capability is required: these fixes use existing API artifact/log routes and preserve CLI behavior. Archive downloads retain complete-file buffering; videos do not. The internal Portal batch-download helper now requires explicit project and cancellation context. Rebuild the Portal image to include the public worker asset; a source hot reload alone does not update a copied deployment asset. Progressive playback requires service workers, streaming responses and trustworthy localhost/HTTPS; unsupported browsers fail visibly without a full-video fallback. Prefix-hosted deployments need matching worker and deny routes as documented. No deployment was performed.

Added pinned eventsource-parser@3.0.6, its canonical lockfile entries and license notice; unrelated lockfile churn was excluded.

Checklist

  • If Portal features changed, keep CLI capabilities in sync.
  • If Portal components changed, update their Storybook stories.
  • If database changes require a migration, include up() / down() and keep it CosmosDB-compatible.
  • If dependencies changed, update the lockfile and regenerate NOTICE / NOTICE-REVIEW.txt with pnpm notice as needed.
  • Video showing the behavior before the suggested change
  • Video showing the behavior after the suggested change

Migration item: N/A, no database/model changes. CLI item: existing CLI capabilities and API contracts are retained; no new CLI command is needed. Demo items remain outstanding, not marked N/A for this user-visible fix.

Extract final delivery implementation from source 1f8ea3c onto agreed main 2101bd0. Retain existing auth, project scoping and report behavior; exclude RBAC and mutation-idempotency changes. Record isolated non-RBAC browser acceptance and remaining deployment/browser limits.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3f31285c-39b5-461e-9550-48f23fcf2882
@github-actions github-actions Bot added type: documentation Documentation additions, corrections, and improvements. type: dependencies Dependency additions, removals, and version updates. language: javascript Work involving JavaScript code, tooling, or dependencies. area: portal Scope web portal, pages, and user-facing components. topic: testing Test coverage, test infrastructure, and validation quality. topic: ui Portal, visual presentation, layout, and interface components. labels Oct 21, 2026
@duffney
Josh Duffney (duffney) added this pull request to stack #1533 October 21, 2026 20:25
@github-actions

Copy link
Copy Markdown

Test Results (Node.js 22)

test: Run #342

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
3453 3453 0 0 0 0 0 1m33s

🎉 All tests passed!

Github Test Reporter

@duffney

Copy link
Copy Markdown
Collaborator Author

Reviewed the authenticated-delivery changes and the local real-worker test results. I recommend fixing these two P2 issues before merging.

1. [P2] Recognize ky network errors when retrying artifact requests

Location: apps/portal/src/lib/artifact-delivery.ts:83–86

The retry guard checks error instanceof TypeError, but ky 2 wraps real browser network failures such as TypeError("Failed to fetch") in NetworkError. Those failures therefore escape immediately instead of entering the bounded establishment retry loop. This affects archive/HAR/codebase downloads and upstream video-range requests.

Reproduction: With the real apiClient, make fetch reject once with TypeError("Failed to fetch") and queue a successful gzip response next. fetchArtifact rejects with NetworkError after exactly one fetch call. The existing TypeError("Network unavailable") test does not exercise ky’s network-error classification.

Suggested resolution: Use ky’s isNetworkError, following the existing implementation in openLogStream. Update the regression test to use a real browser network-error shape and assert that the next attempt succeeds. Preserve the existing rule that failures after a response body has been accepted must not replay the transfer.

2. [P2] Isolate native download navigation in the interaction-story tests

Location: apps/portal/src/components/artifact-stories.play.test.tsx:21–34; related story at apps/portal/src/components/ArtifactDownloadButton.stories.tsx:36–49

The cancellation story uses a fixed 500 ms response delay. Under slower execution, it can pass even though a download has already been dispatched. Happy DOM then navigates the document to blob:nodedata:..., and the cleanup does not restore the URL. The three subsequent authorization stories fail with Invalid URL in artifactUrl instead of reaching their HTTP 403 handlers.

Reproduction: This also occurs with --maxWorkers=1. A local diagnostic logged a blob:nodedata:... document URL immediately after the passing pending/cancel story. The authorization story succeeds when run alone.

Suggested resolution: Stub native anchor download navigation in the test harness and restore the document URL between tests. Replace the timing-dependent response delay with a controlled pending response, and assert that cancellation aborts the request without dispatching a saved file—not only that the button becomes enabled again.

Validation and delivery evidence

  • Two real Copilot workspace archives were downloaded through Portal controls, decompressed through the nested snapshot archive, and matched the exact expected marker files. Current and historical archive endpoints returned HTTP 200.
  • Authenticated stored logs replayed 98 events and a completion event.
  • A real Electron-worker VP8 recording played through authenticated HTTP 206 responses in Edge; seeking within the buffered recording succeeded. Safari playback advanced after reload. The initial Safari playback error was not isolated, so it is not reported as a confirmed PR defect.
  • Focused tests: 258 passed, 3 failed after rerunning initial resource-contention timeouts sequentially. The remaining three failures are the authorization-story failures above. This was not a full repository test run.
  • pnpm --filter portal exec tsc -b: passed against the exact PR sources in a disposable container with adequate memory.

The real-worker recording was only 5.56 seconds, so this session did not independently establish throttled long-video playback or seeking beyond buffered bytes. It also did not repeat the full account/project/logout, cross-client, worker-restart, production-nginx, or prefix-deployment acceptance matrix. Earlier model/Judge compatibility failures were local environment issues, not demonstrated regressions in this PR.

Recognize wrapped browser network failures at GET establishment while preserving bounded retries, cancellation and no accepted-body replay. Keep cancellation-story responses pending until abort, prevent native download navigation in the harness and verify abort without download dispatch.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3f31285c-39b5-461e-9550-48f23fcf2882
@duffney

Copy link
Copy Markdown
Collaborator Author

Addressed both P2 findings from #1531 (comment) in 99e4156.

  1. Artifact GET establishment now recognizes ky isNetworkError, matching the log-stream transport. A regression using the real apiClient reproduced the original TypeError("Failed to fetch") → NetworkError failure before the fix. Coverage now verifies recovery, a three-attempt cap, cancellation during backoff, no retry for non-network TypeErrors, no batch POST replay, and no replay of failed accepted bodies.
  2. The cancellation story holds its response pending until the request is aborted, eliminating the 500 ms race. The harness stubs native anchor download navigation, resets the document URL between cases, and asserts the actual request signal is aborted with no saved-file dispatch. A successful-download case verifies filename/blob dispatch without changing the test URL; all three subsequent authorization stories pass. The original timing-dependent story failure did not reproduce in the initial baseline run here, so this is not claimed as a deterministic reproduction of those three failures.

Validation on the follow-up:

  • pnpm exec vitest run apps/portal/src/lib/artifact-delivery.test.ts apps/portal/src/components/artifact-stories.play.test.tsx --maxWorkers=1 --reporter=dot — passed all 62 tests, five consecutive sequential runs, plus a final rerun after type-check adjustments.
  • pnpm exec vitest run apps/portal/src --maxWorkers=2 --reporter=dot — passed 79 files / 663 tests on the final sources.
  • pnpm --filter portal build — passed, including TypeScript project checks.
  • pnpm --filter portal build-storybook — passed.
  • git diff --check — passed.

Existing MSAL/nested-button and bundle-size warnings remain nonblocking. No new native-browser/deployment acceptance is claimed for this focused follow-up; previously documented limits remain. The change is pushed without rewriting the original delivery checkpoint; the dependent upper-layer owners are being notified of the new lower SHA.

@github-actions

Copy link
Copy Markdown

Test Results (Node.js 22)

test: Run #372

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
3458 3458 0 0 0 0 0 58.8s

🎉 All tests passed!

Github Test Reporter

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: portal Scope web portal, pages, and user-facing components. language: javascript Work involving JavaScript code, tooling, or dependencies. topic: testing Test coverage, test infrastructure, and validation quality. topic: ui Portal, visual presentation, layout, and interface components. type: dependencies Dependency additions, removals, and version updates. type: documentation Documentation additions, corrections, and improvements.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant