Skip to content

feat(plugins): resident network-access providers with proxy-node support - #1096

Merged
Quick104 merged 19 commits into
mainfrom
feat/network-access-plugins
Sep 20, 2026
Merged

Quick104 merged 19 commits into
mainfrom
feat/network-access-plugins

Conversation

@Quick104

@Quick104 Quick104 commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Related issue: #1001

Network access plugins need to stay running, retain their network identity across restarts, and route playback through proxy addresses reachable on the client's network. Administrators also need to see which network and nodes a playback session selected.

Changes

  • Run enabled network-access providers as supervised resident plugins on the API server and enabled proxy nodes. Reconcile installation changes, rehydrate cached plugin archives, validate binary checksums and platform compatibility, and restart failed processes with bounded backoff.
  • Provide host/listener information, encrypted per-host instance state, and network-status reporting through the plugin SDK. Validate per-process ingress tokens and use the resulting access path for native playback, Jellyfin-compatible playback, downloads, and WebSocket origins.
  • Select proxy origins for the request's provider; fall back through the API when no eligible proxy is available. Keep transcode workers on their existing internal connection path.
  • Add network settings with provider names, per-host status, and independent connect/disconnect progress. Show the selected provider and named execution/egress nodes in activity views, with explicit default and unknown network states.
  • Preserve route observations through session sync, signed stream tokens, recipe recovery, and replans. Repair missing execution-node identities and Jellyfin child-segment recovery after restart. Expose the observation through additive v2 admin fields and a capability; keep v1 session JSON unchanged.

Validation

Route and UI validation at c415b1a45:

  • Build, vet, formatting, scoped Go lint, frontend lint/format/build, settings bindings, playback fixtures, route inventory, migration ledger, scenario catalogs, offline routes, API v2 OpenAPI/contract/fixtures/web types, committed-router artifact, and local-path checks passed. Full-tree router-recovery lint passed.
  • make test-web: 583 files and 4,379 tests passed.
  • New migration applied to an isolated PostgreSQL database. Database-backed admin session and reconciler tests passed. Instance-state/generation tests, the two-process proxy-origin test, and nil-supervisor launch regression passed under the race detector.
  • Full Jellyfin package passed after the child-segment recovery fix. Its new regression failed before the fix for default and Tailscale routes and passed afterward.
  • make test-go failed in two packages on macOS. TestAdminResourceCapabilitiesAndScope reproduces unchanged on main (d2596927e): it expects resource support on a platform reporting unsupported. TestResolveViewerScopeParity hit its 25 ms policy timeout; the full policy package passed on rerun. Other packages in that run passed. This is not a claim that the full local Go gate is green.
  • That revision subsequently passed both hosted Go and Web CI.

Route coverage includes default/overlay/unknown persistence, native and Jellyfin preparation, recipe/token recovery, rollback, admin v2 serialization, and independent host controls.

Shared development testing with a real Tailscale plugin observed playback using a transcode worker and proxy over the overlay. Browser checks covered the activity presentation and network settings. This is not full Apple/Android TV handoff or onboarding acceptance.

Review summary: #1096 (comment)

Review fixes at ffdd3e2a7

Replica presence now uses a unique process marker even when replicas share a logical node name. The Redis regression reproduced the undercount before the fix and verifies independent cleanup afterward. The admin restart regression now waits for reconciliation and its launches to finish instead of sleeping for two seconds; temporarily restoring the duplicate restart made the revised test fail.

The cache and plugin packages passed under the race detector with Redis and PostgreSQL enabled. The restart test passed five race runs. Build, vet, scoped lint, and the local-path check passed. Go and Web CI passed on this latest head. Codex re-reviewed the same commit and reported no major issues. Both corresponding review threads are resolved. Macroscope also resolved its nil-supervisor finding after the updated source and race-test evidence. All 40 review threads are resolved, with no outstanding changes-requested review. The PR remains open and has not been merged.

Limits and follow-up

AI disclosure

Original implementation and earlier repairs used Claude Code (Claude Agent SDK) in T3 Code, Workflow subagents, OpenAI Codex CLI and Codex desktop. Previously disclosed models: claude-fable-5-1, Claude Opus 5 workflow subagents, and gpt-6-astra.

The rebase, route visibility, UI repairs, current validation and follow-up issue drafting were AI-assisted using gpt-6 in Codex through T3 Code, GitHub CLI, and the code-review and repository unslop skills. A separate Codex agent reviewed route persistence/recovery, API boundaries, frontend state, and resident lifecycle/propagation. It found a Jellyfin child-segment recovery gap; a regression failed before the fix and passed afterward. The nil-supervisor review allegation was checked against the explicit nil-receiver guard. No physical cross-platform TV acceptance is claimed.

Current review repairs and replies: gpt-6 in Codex through T3 Code, using the babysit, Modern Go Guidelines and repository unslop skills.

@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown

Too many files changed for review (147 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 185 files, which is 85 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d99f00a6-2919-44ad-89e7-c1d813f6fe78

📥 Commits

Reviewing files that changed from the base of the PR and between d259692 and ffdd3e2.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (185)
  • cmd/silo/abs_listener.go
  • cmd/silo/artwork_delivery_test.go
  • cmd/silo/main.go
  • cmd/silo/network_access_multinode_test.go
  • cmd/silo/proxy_plugins.go
  • cmd/silo/proxy_plugins_test.go
  • cmd/silo/session_sync.go
  • contracts/api/v2/fixtures/admin_network_access_connect_accepted.json
  • contracts/api/v2/fixtures/admin_network_access_disconnect_accepted.json
  • contracts/api/v2/fixtures/admin_network_access_status_not_found.json
  • contracts/api/v2/fixtures/admin_network_access_status_ok.json
  • contracts/api/v2/fixtures/admin_network_access_status_unavailable_host.json
  • contracts/api/v2/fixtures/admin_network_access_unknown_host.json
  • contracts/api/v2/fixtures/admin_playback_sessions.json
  • contracts/api/v2/fixtures/get_system_info_ok.json
  • contracts/api/v2/fixtures/index.json
  • contracts/api/v2/fixtures/network_access_capabilities_ok.json
  • contracts/api/v2/migration.json
  • contracts/api/v2/openapi.json
  • contracts/api/v2/route-inventory.json
  • docker-compose.yml
  • docs/admin-api.md
  • docs/architecture/network-access.md
  • docs/architecture/worker-http-protocol.md
  • docs/network-access-api.md
  • go.mod
  • internal/api/handlers/admin_logs_socket_v2.go
  • internal/api/handlers/admin_node_commands.go
  • internal/api/handlers/admin_node_commands_test.go
  • internal/api/handlers/admin_node_network_access.go
  • internal/api/handlers/admin_node_network_access_test.go
  • internal/api/handlers/admin_plugin_lifecycle.go
  • internal/api/handlers/admin_plugin_lifecycle_test.go
  • internal/api/handlers/downloads.go
  • internal/api/handlers/downloads_network_access_test.go
  • internal/api/handlers/events_socket_v2.go
  • internal/api/handlers/nodes.go
  • internal/api/handlers/nodes_test.go
  • internal/api/handlers/playback.go
  • internal/api/handlers/playback_control_socket_v2.go
  • internal/api/handlers/playback_sessions.go
  • internal/api/handlers/playback_v3.go
  • internal/api/handlers/playback_v3_network_access_test.go
  • internal/api/handlers/playback_v3_test.go
  • internal/api/handlers/playback_v3_tokenless_test.go
  • internal/api/handlers/plugins.go
  • internal/api/handlers/socket_origin_overlay_test.go
  • internal/api/handlers/socket_origin_proxy_test.go
  • internal/api/handlers/watch_together_socket_v2.go
  • internal/api/handlers/ws_shared.go
  • internal/api/router.go
  • internal/api/testdata/media_routes.txt
  • internal/apiv2/admin_nodes_read.go
  • internal/apiv2/admin_plugin_installations.go
  • internal/apiv2/admin_plugin_installations_test.go
  • internal/apiv2/admin_plugin_lifecycle.go
  • internal/apiv2/admin_plugin_lifecycle_test.go
  • internal/apiv2/admin_sessions.go
  • internal/apiv2/admin_sessions_postgres_test.go
  • internal/apiv2/admin_sessions_test.go
  • internal/apiv2/direct_download_test.go
  • internal/apiv2/document.go
  • internal/apiv2/document_test.go
  • internal/apiv2/fixtures_test.go
  • internal/apiv2/network_access.go
  • internal/apiv2/network_access_fixtures_test.go
  • internal/apiv2/network_access_test.go
  • internal/apiv2/router.go
  • internal/apiv2/worker_protocols.go
  • internal/apiv2/worker_streaming_test.go
  • internal/cache/redis.go
  • internal/cache/replicas.go
  • internal/cache/replicas_test.go
  • internal/contractledger/ledger_test.go
  • internal/jellycompat/handlers_playback.go
  • internal/jellycompat/network_access_test.go
  • internal/jellycompat/router.go
  • internal/jellycompat/routing_policy_test.go
  • internal/jellycompat/server.go
  • internal/jellycompat/streams.go
  • internal/jellycompat/streams_segment_error_test.go
  • internal/jellycompat/streams_test.go
  • internal/netaccess/broker.go
  • internal/netaccess/broker_test.go
  • internal/netaccess/middleware.go
  • internal/netaccess/netaccess_test.go
  • internal/netaccess/node.go
  • internal/netaccess/path.go
  • internal/netaccess/registry.go
  • internal/netaccess/status.go
  • internal/nodepool/admin_configuration_test.go
  • internal/nodepool/health.go
  • internal/nodepool/health_stats_test.go
  • internal/nodepool/health_test.go
  • internal/nodepool/network_access_test.go
  • internal/nodepool/planner.go
  • internal/nodepool/planner_test.go
  • internal/nodepool/proxy_pool.go
  • internal/nodepool/repository.go
  • internal/nodepool/repository_capabilities_test.go
  • internal/nodepool/repository_last_stats_test.go
  • internal/nodepool/transcode_pool.go
  • internal/playback/recipecard.go
  • internal/playback/recipecard_test.go
  • internal/playback/session.go
  • internal/playback/session_test.go
  • internal/playback/transcode_manager.go
  • internal/pluginhost/client.go
  • internal/pluginhost/client_test.go
  • internal/pluginhost/exit_watcher_test.go
  • internal/pluginhost/handshake.go
  • internal/pluginhost/host.go
  • internal/pluginhost/host_info.go
  • internal/pluginhost/host_info_test.go
  • internal/pluginhost/instance_state.go
  • internal/pluginhost/metrics.go
  • internal/pluginhost/runtime_host_server.go
  • internal/pluginhost/testdata/exitingplugin/main.go
  • internal/pluginhost/testdata/exitingplugin/manifest.json
  • internal/plugins/archive_cache.go
  • internal/plugins/archive_cache_test.go
  • internal/plugins/auto_update.go
  • internal/plugins/binary_platform.go
  • internal/plugins/binary_platform_test.go
  • internal/plugins/host_adapter.go
  • internal/plugins/installation.go
  • internal/plugins/installation_resident_query_test.go
  • internal/plugins/instance_state.go
  • internal/plugins/instance_state_test.go
  • internal/plugins/lifecycle_events.go
  • internal/plugins/lifecycle_generation_test.go
  • internal/plugins/network_access.go
  • internal/plugins/network_access_broker_test.go
  • internal/plugins/network_access_generation_test.go
  • internal/plugins/network_access_nodes_test.go
  • internal/plugins/network_access_test.go
  • internal/plugins/node_service.go
  • internal/plugins/resident.go
  • internal/plugins/resident_generation_test.go
  • internal/plugins/resident_ingress_test.go
  • internal/plugins/resident_test.go
  • internal/plugins/runtime_config.go
  • internal/plugins/service.go
  • internal/plugins/service_admin_config.go
  • internal/plugins/service_connection.go
  • internal/plugins/service_hot_reload_test.go
  • internal/plugins/service_singleflight_test.go
  • internal/plugins/testdata/residentplugin/main.go
  • internal/plugins/testdata/residentplugin/manifest.json
  • internal/proxy/network_access.go
  • internal/proxy/network_access_health_test.go
  • internal/proxy/network_access_test.go
  • internal/proxy/protocol_registry.go
  • internal/proxy/server.go
  • internal/proxy/testdata/media_routes.txt
  • internal/routeinventory/classify.go
  • internal/streamtoken/token.go
  • internal/worker/reconciler.go
  • internal/worker/reconciler_postgres_test.go
  • internal/worker/reconciler_test.go
  • migrations/sql/20260914071026_plugin_instance_state.sql
  • migrations/sql/20260914084130_node_network_access.sql
  • migrations/sql/20260914210643_plugin_runtime_generation.sql
  • migrations/sql/20260920210643_add_session_network_provider.sql
  • web/src/api/types.ts
  • web/src/api/v2/operations.ts
  • web/src/api/v2/schema.ts
  • web/src/components/PlaybackRouteBadges.test.tsx
  • web/src/components/PlaybackRouteBadges.tsx
  • web/src/hooks/admin/useSettingsOverview.test.ts
  • web/src/hooks/admin/useSettingsOverview.ts
  • web/src/hooks/queries/admin/networkAccess.test.ts
  • web/src/hooks/queries/admin/networkAccess.ts
  • web/src/hooks/queries/admin/pluginLifecycle.test.ts
  • web/src/hooks/queries/admin/plugins.ts
  • web/src/hooks/queries/keys.ts
  • web/src/lib/adminSettingsSearch.ts
  • web/src/lib/pluginStatusIndicator.ts
  • web/src/pages/AdminPlugins.test.tsx
  • web/src/pages/AdminPlugins.tsx
  • web/src/pages/admin-settings/AdminSettingsLayout.tsx
  • web/src/pages/admin-settings/NetworkAccessSettings.test.tsx
  • web/src/pages/admin-settings/NetworkAccessSettings.tsx
  • web/src/pages/adminActivityPresentation.test.ts
  • web/src/pages/adminActivityPresentation.ts

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-20T22:25:36.140371Z ffdd3e2 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Comment thread internal/netaccess/broker.go
Comment thread internal/plugins/resident.go Outdated
Comment thread internal/plugins/network_access.go
Comment thread internal/plugins/instance_state.go
@macroscopeapp

macroscopeapp Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a broad network-access platform spanning resident plugin processes, proxy nodes, encrypted state, ingress authentication, new APIs, migrations, and access-path-aware playback and download routing. The unresolved High-severity nil-supervisor dereference, together with the feature's security-sensitive and cross-cutting runtime impact, requires human review.

Not approved because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c847065d2d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread cmd/silo/proxy_plugins.go
Comment thread internal/plugins/network_access.go
Comment thread internal/plugins/instance_state.go Outdated
Quick104 added a commit that referenced this pull request Sep 14, 2026
Review findings on #1096, each with a regression test:

- netaccess.Broker serializes Issue and Revoke so an old process's revoke
  can no longer forget the status a replacement pushed between the
  registry revoke and the cache forget.
- A superseded successful launch is stopped when its entry is parked
  (stopped or failed), not only when the entry is gone or the supervisor
  halted; a launch a newer starting generation may adopt is still left
  to that generation's start-sequence check.
- A failed provider RPC on a running process reports unavailable to the
  status sink so a dead overlay origin stops being advertised to the
  origin check and the node health report.
- Instance-state writes take a per-scope transaction advisory lock so
  concurrent first writes of distinct keys cannot overshoot the 256-key
  budget.
- Duplicate provider slugs resolve to the lowest enabled installation id
  everywhere (provider list, commands, node health map) with a warning,
  instead of the first-listed installation on one path and map order on
  another.
- A proxy rehydrating a stored archive checks the binary's platform
  against its own and refuses a foreign one with a clear error; the
  archive was resolved for the API server's platform at install time.
  Documented as a same-platform requirement for this release.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f21d6dd62e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/apiv2/network_access.go
Comment thread internal/api/handlers/admin_node_network_access.go
Quick104 added a commit that referenced this pull request Sep 14, 2026
…ed proxies

Second round of review findings on #1096:

- {"hosts": null} on connect and disconnect decoded to the same nil as
  omission, which means every host. The command body now records an
  explicit null in its decoder and the operation answers 422 for it;
  RawBody was not used because it would make the optional body required
  in the document.
- A proxy's resident gate only checked that its stream_nodes row existed,
  so a disabled or retyped node kept serving overlay ingress while the
  API no longer listed it as a host that could be disconnected. The gate
  now requires an enabled proxy row.
- Every unavailable answer from applyNetworkAccess reports to the status
  sink, not only the RPC-failure path, so a parked or gated instance's
  cached origin is dropped as well.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26fc03da85

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/network_access.go
Comment thread web/src/pages/admin-settings/NetworkAccessSettings.tsx
Comment thread internal/plugins/archive_cache.go

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa2db25a1f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/network_access.go
Comment thread internal/plugins/network_access.go Outdated
Quick104 added a commit that referenced this pull request Sep 14, 2026
…available

Review findings on #1096:

- The resident set excluded nothing, so a second enabled installation
  declaring an already-owned provider slug kept running and serving
  ingress while every command and status read addressed the owner. The
  supervisor now derives ownership the same way the provider list does
  and does not start the duplicate on any host.
- An unavailable answer synthesized by the host carried updated_at set
  to now, but the contract defines that field as when the host last
  heard from the provider and says it is absent while unavailable. The
  cache still stamps its own copy; the wire status carries none.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b15a66af40

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/netaccess/broker.go
Comment thread cmd/silo/proxy_plugins.go Outdated

@Quick104 Quick104 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Two additional P2 defects are confirmed. I am implementing both fixes and their regression tests. Review and repairs use gpt-6-astra through Codex in T3 Code.

Comment thread web/src/hooks/queries/admin/networkAccess.ts
Comment thread internal/plugins/lifecycle_events.go
Quick104 added a commit that referenced this pull request Sep 14, 2026
…ode identity change

Review findings on #1096:

- A status push in flight while its process was being stopped, crashed,
  or replaced could land after the revoke and write a stale connected
  origin back into the cache; on a proxy the next health sweep then
  persisted a dead origin. The broker now accepts a push only from the
  process holding the installation's current ingress token, checked
  under the same lock Revoke takes, and the host RPC drops the rest.
- A proxy whose stream_nodes row was deleted and re-registered resolved
  a new id, and the per-call state scope followed it while the running
  provider kept the old identity in memory. The supervisor now tracks
  the host identity each resident was started under and replaces every
  running resident when it changes; the proxy supplies its node scope.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread internal/plugins/resident.go
Quick104 added a commit that referenced this pull request Sep 14, 2026
Bind provider status to process generations, pin proxy state scopes, recover missed lifecycle events, and expose resident restart controls. Add provider-specific status fan-out and refresh generated contracts.\n\nReviewed PR: #1096\nValidation: focused Go tests, database-backed generation tests, web tests (579 files/4318 tests), Go build, go vet, route/OpenAPI/fixture/ledger gates.\n\nAI disclosure: OpenAI Codex CLI using gpt-6-astra through T3 Code; implementation and review fixes were AI-assisted and human verified.
@Quick104

Copy link
Copy Markdown
Contributor Author

Codex review follow-up — 36ef0b5

Reviewed the updated PR head and addressed five confirmed issues:

  • provider status reports and RPC results are bound to the process ingress token;
  • proxy host identity and encrypted state scope are pinned per process and replaced when the node row changes;
  • cached proxy binaries receive platform validation;
  • provider status fan-out uses a provider-specific worker route;
  • failed resident plugins have an admin restart action, and persisted runtime generations let polling recover missed lifecycle events;
  • admin status query caches include the complete profile authority.

Validation passed: focused Go tests across netaccess, pluginhost, plugins, proxy, nodepool, Jellyfin, handlers, and apiv2; database-backed instance-state and lifecycle-generation tests; go build ./...; go vet ./...; route inventory, OpenAPI, fixtures, migration ledger, settings bindings, offline routes, and local-path gates; and the web suite (579 files, 4318 tests), lint, format check, and production build.

The full Go suite still reports the repository's documented merge-base failures in unrelated adminjob/catalog/metadata/handler/database packages; it also encountered resource-sensitive timeouts in unrelated tests during the broad run. Those results are recorded in the PR validation notes and are not called green.

Mergeability: 8/10 — conditional. The reviewed head has no remaining actionable findings from this pass; merge requires the hosted Go check to complete successfully or for maintainers to classify any baseline failure. Real-provider overlay QA remains the planned follow-up because the provider plugin does not exist yet.

Comment thread internal/pluginhost/host.go
Comment thread internal/plugins/instance_state.go Outdated
Comment thread internal/plugins/resident.go Outdated
Comment thread internal/plugins/resident.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36ef0b5b77

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/resident.go
Quick104 added a commit that referenced this pull request Sep 14, 2026
…ir the merged tree

- Reconcile is serialized end to end (Macroscope finding on #1096): the
  desired set and gate result are computed outside the state lock, so two
  overlapping reconciles could apply results in the wrong order and an
  older one could resurrect a resident a newer one had removed. Test
  races eight reconciles against a disable under the race detector.
- RestartInstallation now advances runtime_generation before restarting
  locally, so a proxy whose lifecycle subscription missed the event
  replaces its process on the next poll and a failed entry's budget is
  cleared there too. This makes TestResidentPollRecoversMissedRestartOfFailedProvider
  from the previous commit pass; it had no writer for the generation it
  waited on.
- The worker route inventory count includes the per-provider proxy
  status route the previous commit added.
- gofmt under the toolchain CI pins (go.mod says 1.26.4) reflows one
  struct the previous commit left misaligned; the local 1.26.5 accepted
  it, which is why it slipped through.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a242cb4439

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/resident.go Outdated
Comment thread internal/plugins/network_access_broker_test.go Outdated
Quick104 added a commit that referenced this pull request Sep 14, 2026
Review findings on #1096:

- With the restart recorded as a runtime_generation bump, publishing a
  restart event as well made a following proxy restart on the event and
  then replace the fresh process again on the reconcile that saw the new
  generation: two overlay outages and two token rotations per admin
  restart. The event is now a plain reconcile when the generation was
  persisted, and it is published whether or not this host runs the
  resident itself. Test: TestAdminRestartReplacesFollowerProcessOnce.
- The broker idle test documents why it sleeps (go-plugin's five-second
  pending-stream window is a constant with no seam or signal) and that a
  late cleanup can only make it pass vacuously, never fail; it is skipped
  under -short.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b11d176599

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/resident.go Outdated
Quick104 added a commit that referenced this pull request Sep 14, 2026
…adapter

Review finding on #1096: the supervisor's superseded-launch check read
NextStartSeq through a type assertion the production hostAdapter did not
satisfy, so outside the test fake the floor was always zero and a newer
generation could adopt a launch an older one had started. NextStartSeq
is now part of the plugins.Host interface and the adapter forwards it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da7193faf7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/apiv2/network_access.go
Comment thread internal/api/handlers/plugins.go Outdated
Comment thread internal/api/handlers/admin_plugin_lifecycle.go Outdated
Quick104 added a commit that referenced this pull request Sep 14, 2026
…he restart receipt

Review findings on #1096:

- A literal null request body on connect and disconnect decoded to a nil
  body, which means every host. The input now records whether a body was
  sent at all and answers 422 for null; omission still means every host.
- The admin installation view flagged every enabled installation with a
  resident capability as resident, including a duplicate provider slug
  the supervisor deliberately does not own, so the web page offered a
  restart that could not start anything. Once the supervisor is armed
  its entries decide; the capability is used only before boot finishes.
- The restart receipt was built from the pre-restart row and carried a
  stale updated_at after the generation bump; it now reloads the row.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Silo-Server Silo-Server deleted a comment from kody-ai Bot Sep 14, 2026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50ed025fc5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/plugins/archive_cache.go
Comment thread internal/apiv2/network_access.go Outdated
Quick104 and others added 14 commits September 20, 2026 15:39
…ed proxies

Second round of review findings on #1096:

- {"hosts": null} on connect and disconnect decoded to the same nil as
  omission, which means every host. The command body now records an
  explicit null in its decoder and the operation answers 422 for it;
  RawBody was not used because it would make the optional body required
  in the document.
- A proxy's resident gate only checked that its stream_nodes row existed,
  so a disabled or retyped node kept serving overlay ingress while the
  API no longer listed it as a host that could be disconnected. The gate
  now requires an enabled proxy row.
- Every unavailable answer from applyNetworkAccess reports to the status
  sink, not only the RPC-failure path, so a parked or gated instance's
  cached origin is dropped as well.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ter idle

Live testing with the stub provider on a sandbox (API server plus one
proxy and one transcode node) found that a provider connected more than
five seconds after its process started could never reach the host:
go-plugin drops the pending broker stream after five seconds and the SDK
dialed it lazily on the first Host() call. Connect then timed out on
every host. Silo-Server/silo-plugin-sdk#22 (v0.16.1) dials at bind time.

The resident fixture's GetStatus now makes a host call and reports
whether it worked, and a new test idles six seconds after start before
the first callback. It fails against v0.16.0 and passes against v0.16.1.
The resident test host binds the RuntimeHost broker so fixtures can call
back at all.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…available

Review findings on #1096:

- The resident set excluded nothing, so a second enabled installation
  declaring an already-owned provider slug kept running and serving
  ingress while every command and status read addressed the owner. The
  supervisor now derives ownership the same way the provider list does
  and does not start the duplicate on any host.
- An unavailable answer synthesized by the host carried updated_at set
  to now, but the contract defines that field as when the host last
  heard from the provider and says it is absent while unavailable. The
  cache still stamps its own copy; the wire status carries none.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ode identity change

Review findings on #1096:

- A status push in flight while its process was being stopped, crashed,
  or replaced could land after the revoke and write a stale connected
  origin back into the cache; on a proxy the next health sweep then
  persisted a dead origin. The broker now accepts a push only from the
  process holding the installation's current ingress token, checked
  under the same lock Revoke takes, and the host RPC drops the rest.
- A proxy whose stream_nodes row was deleted and re-registered resolved
  a new id, and the per-call state scope followed it while the running
  provider kept the old identity in memory. The supervisor now tracks
  the host identity each resident was started under and replaces every
  running resident when it changes; the proxy supplies its node scope.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bind provider status to process generations, pin proxy state scopes, recover missed lifecycle events, and expose resident restart controls. Add provider-specific status fan-out and refresh generated contracts.\n\nReviewed PR: #1096\nValidation: focused Go tests, database-backed generation tests, web tests (579 files/4318 tests), Go build, go vet, route/OpenAPI/fixture/ledger gates.\n\nAI disclosure: OpenAI Codex CLI using gpt-6-astra through T3 Code; implementation and review fixes were AI-assisted and human verified.
…ir the merged tree

- Reconcile is serialized end to end (Macroscope finding on #1096): the
  desired set and gate result are computed outside the state lock, so two
  overlapping reconciles could apply results in the wrong order and an
  older one could resurrect a resident a newer one had removed. Test
  races eight reconciles against a disable under the race detector.
- RestartInstallation now advances runtime_generation before restarting
  locally, so a proxy whose lifecycle subscription missed the event
  replaces its process on the next poll and a failed entry's budget is
  cleared there too. This makes TestResidentPollRecoversMissedRestartOfFailedProvider
  from the previous commit pass; it had no writer for the generation it
  waited on.
- The worker route inventory count includes the per-provider proxy
  status route the previous commit added.
- gofmt under the toolchain CI pins (go.mod says 1.26.4) reflows one
  struct the previous commit left misaligned; the local 1.26.5 accepted
  it, which is why it slipped through.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review findings on #1096:

- With the restart recorded as a runtime_generation bump, publishing a
  restart event as well made a following proxy restart on the event and
  then replace the fresh process again on the reconcile that saw the new
  generation: two overlay outages and two token rotations per admin
  restart. The event is now a plain reconcile when the generation was
  persisted, and it is published whether or not this host runs the
  resident itself. Test: TestAdminRestartReplacesFollowerProcessOnce.
- The broker idle test documents why it sleeps (go-plugin's five-second
  pending-stream window is a constant with no seam or signal) and that a
  late cleanup can only make it pass vacuously, never fail; it is skipped
  under -short.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…adapter

Review finding on #1096: the supervisor's superseded-launch check read
NextStartSeq through a type assertion the production hostAdapter did not
satisfy, so outside the test fake the floor was always zero and a newer
generation could adopt a launch an older one had started. NextStartSeq
is now part of the plugins.Host interface and the adapter forwards it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…he restart receipt

Review findings on #1096:

- A literal null request body on connect and disconnect decoded to a nil
  body, which means every host. The input now records whether a body was
  sent at all and answers 422 for null; omission still means every host.
- The admin installation view flagged every enabled installation with a
  resident capability as resident, including a duplicate provider slug
  the supervisor deliberately does not own, so the web page offered a
  restart that could not start anything. Once the supervisor is armed
  its entries decide; the capability is used only before boot finishes.
- The restart receipt was built from the pre-restart row and carried a
  stale updated_at after the generation bump; it now reloads the row.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Quick104

Quick104 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor Author

Latest head: ffdd3e2a7b40bb87afa0e5445cb19f74e0967f1c.

Babysit update: fixed and resolved the two new findings about colliding API replica presence keys and the fixed restart quiet-period wait. The cache/plugin race suites with Redis/PostgreSQL, build, vet, scoped lint and path checks passed. Duplicate names failed the Redis census before the fix; restoring a duplicate restart made the revised completion-based test fail. Macroscope resolved the older nil-supervisor allegation after the updated evidence reply. All 40 current review threads are resolved. Go and Web CI passed on this head. Codex re-reviewed this commit and reported no major issues. No reviewer has an outstanding changes-requested review. No merge was performed.

The earlier route/UI review and its evidence apply to c415b1a45:

The rebase and route/UI fixes are pushed, and GitHub reports no merge conflicts. The change fits the network-plugin scope: resident lifecycle, provider-aware routing, and visible playback network/node information. Client address identity and TV pairing recovery are separate follow-ups.

Resolved P2: initial Jellyfin local HLS startup persisted its recipe before its route assignment. Child-segment recovery now overlays the durable assignment before reconstructing the session. The endpoint regression reproduced missing provider/route facts before the fix and passes afterward for default and Tailscale routes. The full Jellyfin package passes.

Independent review covered route persistence/recovery, native/Jellyfin callers, v1/v2 boundaries, frontend command state, resident lifecycle, archive rehydration, and proxy propagation. It found no further verified blocker. The existing nil-supervisor allegation remains a false positive: ResidentSupervisor.State handles a nil receiver; the directly constructed service regression passed under the race detector. Macroscope subsequently resolved that thread.

Validation:

  • Build, vet, formatting, scoped Go lint, frontend lint/format/build, all contract/fixture/documentation gates, settings bindings, and full-tree router-recovery lint passed.
  • Web: 583 test files, 4,379 tests passed.
  • Isolated PostgreSQL migration, session/reconciler round trips, and plugin state/generation plus two-process proxy-origin race tests passed.
  • Full local Go run failed in two packages: the resource-capability test reproduces unchanged on main on macOS; the policy package's 25 ms timeout passed on rerun. The other packages passed. The full local Go gate is therefore not claimed green.
  • Earlier PR CI passed both jobs. Latest-head CI also passed Go and Web. Automated review coverage is limited: CodeRabbit skipped this 185-file PR, and Macroscope checks were skipped.

Follow-ups addressing the mixed-address and TV onboarding discussion: server #1268, Silo-Server/silo-apple#341, Silo-Server/silo-android#352. They link to #1169 and #1192. The current activity changes expose provider and named execution/egress nodes for the concern raised against #1205. Physical cross-platform TV acceptance remains unperformed.

Mergeability: 9/10, at ffdd3e2a7. No known code blocker remains in the reviewed scope: Go/Web CI passed, Codex re-review found no major issues, and all 40 threads are resolved. The documented deployment limits and unperformed physical TV acceptance still apply. The PR remains open; no merge was performed.

AI assistance: gpt-6 in Codex through T3 Code, GitHub CLI, and an independent Codex review agent. Applied the code-review and repository unslop skills. Review and fixes were source- and test-based; no physical TV validation is claimed.

Babysit repairs: gpt-6 in Codex through T3 Code, using the babysit, Modern Go Guidelines and repository unslop skills.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c415b1a453

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread cmd/silo/main.go
Comment thread internal/plugins/lifecycle_generation_test.go Outdated
@Quick104

Copy link
Copy Markdown
Contributor Author

@zZebrahz The activity views now show the selected network provider alongside named execution and egress nodes, including an explicit API-server label, in c415b1a. Default and unknown routes are distinguished; this reports the prepared playback route rather than inferring network access from a client IP. That covers the route-visibility concern raised for #1205.

The #1169/#1192 check found a separate client issue: URL-derived server matching treats public and plugin addresses as different servers, and companion pairing supplies the phone's address even when the TV cannot reach it. Coordinated follow-ups are open: server #1268, Silo-Server/silo-apple#341, and Silo-Server/silo-android#352. They cover verified server identity across addresses, TV-side reachability, provider setup guidance, and an explicit configured-public-address fallback. SiloRemote remains same-LAN; physical cross-platform acceptance is still outstanding.

AI-assisted source review and reply: gpt-6 in Codex through T3 Code, using the babysit and repository unslop skills.

@Quick104

Copy link
Copy Markdown
Contributor Author

@codex review

Both findings from the review of c415b1a are fixed in ffdd3e2 and their threads include reproduction and test evidence. Please re-review the updated head. This PR is being monitored without merging.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: ffdd3e2a7b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Quick104
Quick104 merged commit 74fb014 into main Sep 20, 2026
11 checks passed
@Quick104
Quick104 deleted the feat/network-access-plugins branch September 20, 2026 23:04
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.

2 participants