fix(photon): persist sidecar token for standalone sends

[kramersharp]

Summary

• persist the Photon sidecar control token under Hermes runtime state so standalone sends / cron jobs can authenticate without inheriting gateway env
• lazily load bundled platform CLI commands so hermes photon ... resolves before plugin CLI iteration

Tests

• python -m pytest tests/plugins/platforms/photon tests/hermes_cli/test_startup_plugin_gating.py -q (149 passed)

Note

• Removed the unrelated Codex Telegram noise-filter change from this PR so it stays scoped to Photon/CLI startup.

[teknium1]

Thanks for targeting a real standalone-delivery gap. Current main still requires PHOTON_SIDECAR_TOKEN in _standalone_send (plugins/platforms/photon/adapter.py:1677-1684), while the gateway-only adapter generates and passes that token to its child sidecar (plugins/platforms/photon/adapter.py:303-305, 936-945).

Problems

• The new runtime record is written immediately after Popen, before the existing readiness check. If startup fails, current connect() returns at plugins/platforms/photon/adapter.py:435-445 without calling _stop_sidecar(), so the record can remain although no usable sidecar was established.
• The added 0o600 assertion is not native-Windows-safe. The project requires Windows guards for POSIX-mode assertions; stat.S_IMODE availability does not establish that chmod(0o600) is enforceable there.

Suggested changes

• Publish only after /healthz succeeds, or remove the record on all startup-failure paths.
• Add a test that unsets the env token and proves _standalone_send uses the persisted record in its HTTP authorization header; guard the POSIX-mode assertion on Windows.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Nine PRs address or reference Photon sidecar lifecycle problems: six overlap on suppressing Windows console windows, #56514 adds standalone-send token persistence and deferred CLI discovery, and #66940/#66956 move two distinct blocking subprocess paths off the gateway event loop. The Windows runtime-spawn cause has already been resolved by merged #65441; the remaining open PRs address separate causes and should not be treated as duplicates.

Related pull requests

• #54565 [closed] related — (+30/-0) — superseded by merged #65441: adds the shared windows_hide_flags() helper to both the Spectrum patch run and persistent sidecar spawn, with behavioral coverage for both. Despite the keep_open review on #54565, its requested test coverage was present in the final diff and these commits were cherry-picked with authorship into #65441, so the closed PR remains the source implementation.
• #56514 related — (+136/-4) — keep open, but require fixes before merge: persists the gateway-generated Photon token for standalone sends and lazily discovers bundled platform CLI commands, directly addressing cron/out-of-process authentication and hermes photon resolution. The keep_open review identifies concrete remaining gaps in the diff: the token record is published before readiness and can become stale after startup failure, the mode assertion is not Windows-safe, and no test proves that standalone-send authorization actually consumes the persisted token.
• #56610 [closed] related — (+3/-0) — duplicate resolved by #65441: hides only the persistent sidecar Popen and therefore leaves the Spectrum patch subprocess exposed. Despite the keep_open review on #56610, its requested expansion to both spawn sites plus regression coverage is already implemented in merged #65441, making the closed PR relevant as a narrower duplicate.
• #57042 [closed] related — (+82/-0) — duplicate resolved by #65441: applies a win32-gated CREATE_NO_WINDOW flag to both Node spawn sites and adds AST-based invariants. The merged implementation uses the repository's shared windows_hide_flags() helper and behavioral kwargs coverage instead, while also covering npm dependency self-heal spawns.
• #65083 [closed] related — (+109/-1) — duplicate with an unsafe process-lifecycle variant: hides the persistent sidecar but combines DETACHED_PROCESS with stdin/stdout pipes and leaves the patch-script spawn unprotected. Despite the keep_open review on #65083, the diff conflicts with the repository's stdio-preservation contract, while merged #65441 fixes both spawn sites with CREATE_NO_WINDOW only; closure as a duplicate is therefore appropriate.
• #65299 [closed] related — (+39/-0) — superseded by merged #65441: covers the patch run, persistent sidecar, and both npm self-heal subprocesses using windows_hide_flags(), closely matching the merged runtime fix. Despite the keep_open review on #65299, its additional request concerned separate CLI installer subprocesses not changed by this PR, while the complete scope of this diff was already merged through #65441.
• #65441 [merged] related — (+36/-0) — merged reference implementation: applies windows_hide_flags() to the Spectrum patch run, persistent sidecar, and both npm self-heal paths, preserving stdio by avoiding DETACHED_PROCESS and adding behavioral regression coverage. It incorporates #54565's earliest implementation and supersedes the competing Windows-console PRs.
• #66940 related — (+75/-1) — keep open, but fix the test before merge: moves the synchronous Spectrum patch subprocess.run into asyncio.to_thread, directly eliminating a reconnect-time event-loop stall of up to ten seconds without changing subprocess semantics. Consistent with its keep_open review, the regression test must mock a successful health probe and remove the broad exception handler so failures are deterministic rather than hidden.
• #66956 related — (+61/-3) — merge candidate: moves listener discovery and per-PID sidecar inspection into one asyncio.to_thread call, eliminating the separate 5 + 5·N-second event-loop blocking path while preserving stale-versus-foreign classification. Its focused thread-identity test and the keep_open review both support the diff; it complements rather than duplicates #66940.

Duplicates

#54565, #56610, #57042, #65083, and #65299 substantially overlap with the Windows hidden-process change consolidated in merged #65441; #56610 is only the Popen subset, while #65083 uses the rejected DETACHED_PROCESS variant. #56514, #66940, and #66956 address distinct causes, and #66940/#66956 are complementary rather than duplicates.

Suggested consolidation

Merge #66956 — its focused, reviewed diff cleanly removes the listener/PID-inspection event-loop stall. Keep #56514 open until token publication is tied to successful readiness or cleaned up on every failure path, its permission test is Windows-safe, and standalone-send authorization is covered; keep #66940 open until its health probe is mocked and the broad exception suppression is removed. Treat merged #65441 as the canonical Windows-console implementation and leave #54565, #56610, #57042, #65083, and #65299 closed as superseded duplicates.

Cross-PR triage: Reviewed 9 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 38 kB of PR diffs, 20 kB of issue/PR text, 12 kB of discussion (19 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[GottZ]

This was generated by AI during triage.

Delta since our previous triage comment

@teknium1 confirmed that the two previously open event-loop fixes were consolidated into #73563 with authorship preserved: #66940 moves the Spectrum patch spawn off the gateway loop, while #66956 moves listener/PID inspection off it. Both source PRs are now closed because their complementary changes were merged through that consolidation PR, superseding our earlier keep-open/merge recommendations for them.

Changed pull requests

• #66940 [closed] related — (+75/-1) — merged via #73563: the diff moves the blocking Spectrum patch subprocess into asyncio.to_thread. Despite the keep_open review on #66940 requesting deterministic health-probe scaffolding, #73563 reconstructed the shared scaffolding as an independent test; the closed PR remains relevant as the authored source change.
• #66956 [closed] related — (+61/-3) — merged via #73563: the diff moves listener discovery and per-PID inspection into one asyncio.to_thread hop while preserving stale-versus-foreign classification. This fulfills the keep_open review's supported direction, and the closed PR remains relevant as the authored source change consolidated alongside #66940.

Suggested consolidation

The recommendation changes only for #66940 and #66956, which now require no further action because both were merged via #73563; the recommendation for #56514 is unchanged.

Complex graph unchanged since our previous triage comment.

Cross-PR triage: Reviewed 9 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 38 kB of PR diffs, 20 kB of issue/PR text, 13 kB of discussion (21 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.