Skip to content

fix(photon): recover degraded sidecar streams - #50974

Closed
tusharaggarwa1 wants to merge 2 commits into
NousResearch:mainfrom
tusharaggarwa1:fix/photon-sidecar-error-classification
Closed

fix(photon): recover degraded sidecar streams#50974
tusharaggarwa1 wants to merge 2 commits into
NousResearch:mainfrom
tusharaggarwa1:fix/photon-sidecar-error-classification

Conversation

@tusharaggarwa1

@tusharaggarwa1 tusharaggarwa1 commented Jun 22, 2026

Copy link
Copy Markdown

Summary

  • classify Photon sidecar control failures into safe retryable/non-retryable errors, including upstream transient failures, invalid credentials, and Photon project policy failures
  • restart/reconnect Photon when the sidecar process crashes or reports degraded upstream stream health, and avoid reporting an idle stream as healthy before real traffic arrives
  • make gateway runtime adapter failures queue immediate background reconnection instead of waiting for an inbound message or another startup cycle
  • let slow-starting adapters declare a longer connect timeout so Photon sidecar startup is not killed by the generic platform timeout
  • resolve restarted Photon DM spaces with space.get(any;-;+E164) before falling back to phone-based space.create, so established iMessage DMs remain sendable after sidecar restart
  • retry standalone Photon sends when the sidecar reports a transient retryable send failure, and avoid blank standalone-send errors when Python/httpx raises an exception with an empty string
  • upgrade the Photon sidecar to spectrum-ts@5.2.0 and skip the mixed-attachment patch when the newer inbound mapper is present

Related PRs / Expected Merge Path

After #51161, the intended remaining value of this PR is the live-repro follow-up surface: structured sidecar send classification, adapter-specific Photon startup timeout, DM-space rehydration before phone-based creation, standalone send retry/error clarity, and the spectrum-ts@5.2.0 upgrade/patch guard if those are not already merged elsewhere.

Happy to split those remaining pieces further if maintainers prefer a narrower review path.

Validation

  • ./scripts/run_tests.sh tests/plugins/platforms/photon/test_overflow_recovery.py tests/plugins/platforms/photon/test_spectrum_patch.py tests/gateway/test_platform_reconnect.py
  • node --check plugins/platforms/photon/sidecar/index.mjs
  • node --check plugins/platforms/photon/sidecar/patch-spectrum-mixed-attachments.mjs
  • /home/tushar/.hermes/hermes-agent/venv/bin/python -m py_compile plugins/platforms/photon/adapter.py gateway/run.py
  • git diff --check
  • /home/tushar/.hermes/hermes-agent/venv/bin/python scripts/check-windows-footguns.py --diff origin/main

Live Runtime Check

  • restarted the local systemd gateway by SIGTERMing the main process and letting systemd revive it
  • reproduced the old failure mode: Photon startup was cut off around the sidecar readiness timeout, leaving systemd green with no Photon sidecar child
  • verified the patched gateway gives Photon the longer adapter-specific startup window; local Photon startup took about 35s and then connected
  • observed a real inbound Photon text reach Hermes and trigger a gateway response
  • upgraded the live sidecar to spectrum-ts@5.2.0; gateway restarted and connected with the newer Spectrum runtime
  • verified direct sidecar /send can return a real Spectrum message id; also observed intermittent upstream send drops now surface as classified upstream_transient retryable errors

@tusharaggarwa1 tusharaggarwa1 changed the title fix(photon): classify sidecar delivery failures fix(photon): recover and classify sidecar failures Jun 22, 2026
@tusharaggarwa1
tusharaggarwa1 force-pushed the fix/photon-sidecar-error-classification branch from 794b6b9 to 8354568 Compare June 22, 2026 18:58
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have labels Jun 22, 2026
@tusharaggarwa1
tusharaggarwa1 force-pushed the fix/photon-sidecar-error-classification branch from 8354568 to bc00e0e Compare June 22, 2026 19:49
@tusharaggarwa1 tusharaggarwa1 changed the title fix(photon): recover and classify sidecar failures fix(photon): recover degraded sidecar streams Jun 22, 2026
@huntsyea

Copy link
Copy Markdown
Contributor

This PR matches a real live failure mode we hit today: the local sidecar process was healthy enough to mask the fact that the upstream Photon stream was degraded/dead.

Observed state:

Photon sidecar process: running
127.0.0.1:8789: listening
spectrum-ts: 5.2.0
Gateway health: ok

But the stream itself was failing repeatedly:

[spectrum.stream] ERROR stream persistently failing; still retrying
spectrum.stream.label: imessage.messages:shared
spectrum.stream.phase: catch-up
spectrum.stream.attempt: 16
spectrum.error.type: ConnectionError
spectrum.error.message: upstream connect error or disconnect/reset before headers. reset reason: connection termination
grpcCode: 14

Earlier in the same incident we also saw:

CatchUpEvents PERMISSION_DENIED: [upstream] Authentication failed. grpcCode: 7
SpectrumCloudError: Invalid credentials status=401

Best-practice confirmation: /healthz needs to include stream health, not just local HTTP/process liveness. A live index.mjs and open /inbound loop are not sufficient proof that iMessage inbound is actually flowing.

@tusharaggarwa1
tusharaggarwa1 force-pushed the fix/photon-sidecar-error-classification branch from bc00e0e to 00bce48 Compare June 23, 2026 02:34
@tusharaggarwa1
tusharaggarwa1 marked this pull request as ready for review June 23, 2026 02:41
@tusharaggarwa1

tusharaggarwa1 commented Jun 23, 2026

Copy link
Copy Markdown
Author

For reviewer context, this PR now intentionally overlaps with two related Photon PRs:

What this PR changes:

  • Adds safe, machine-readable Photon sidecar error classification so Hermes can distinguish transient upstream failures from permanent auth/project-policy failures.
  • Promotes degraded Photon upstream stream health into a retryable adapter fatal error, so the gateway reconnect path can restart Photon instead of leaving a live local sidecar with dead inbound delivery.
  • Avoids marking the sidecar stream healthy before actual inbound traffic is observed, which prevents /healthz from reporting a false healthy state while the stream is only idle/starting.
  • Queues retryable runtime adapter failures for immediate background reconnection.
  • Lets slow-starting adapters declare a longer connect timeout; this fixes a reproduced local failure where systemd showed the gateway as active, but Photon had no sidecar child because startup was killed before Spectrum finished initializing.
  • Resolves established Photon DM ids with space.get(any;-;+E164) before falling back to phone-based space.create, preserving sendability after sidecar restart.
  • Retries standalone Photon sends when the sidecar marks a send failure as transient/retryable, and reports timeout/transport exception class names instead of returning blank errors.
  • Upgrades the Photon sidecar to spectrum-ts@5.2.0 and skips the mixed-attachment patch when the newer inbound mapper is present.

Relationship to #51105:

  • Same core problem area: a local Photon sidecar can be alive while the upstream Spectrum stream is degraded or recovering.
  • This PR incorporates the important fix(photon): recover degraded upstream stream #51105 stream-health semantics, especially not declaring the stream healthy until real traffic is observed.
  • This PR keeps the broader /healthz degradation-monitoring path and adapter fatal/reconnect integration from the original branch.

Relationship to #51075:

  • Same outbound recovery problem area: restarted sidecars need to rehydrate existing DM spaces before trying phone-based creation, and sidecar send failures should carry retryability/classification.
  • This PR now incorporates the relevant DM-resolution and retry-classification pieces, plus the spectrum-ts@5.2.0 sidecar upgrade and patch-script guard.
  • This PR still does not take the broader HEIC/HEIF image transcoding from fix(photon): harden iMessage outbound recovery #51075. That looks useful, but it is separable from the gateway/sidecar liveness issue this PR is trying to fix.

Validation run on this branch:

  • ./scripts/run_tests.sh tests/plugins/platforms/photon/test_overflow_recovery.py tests/plugins/platforms/photon/test_spectrum_patch.py tests/gateway/test_platform_reconnect.py → 60 passed
  • node --check plugins/platforms/photon/sidecar/index.mjs
  • node --check plugins/platforms/photon/sidecar/patch-spectrum-mixed-attachments.mjs
  • /home/tushar/.hermes/hermes-agent/venv/bin/python -m py_compile plugins/platforms/photon/adapter.py gateway/run.py
  • git diff --check
  • /home/tushar/.hermes/hermes-agent/venv/bin/python scripts/check-windows-footguns.py --diff origin/main

Live verification:

  • Reproduced the pre-fix state where the gateway service was active but Photon had no sidecar child.
  • Verified Photon startup needed about 35s locally; the old generic timeout path could cut it off before startup completed.
  • Observed a real inbound Photon text reach Hermes and trigger a gateway response.
  • Verified direct sidecar /send can return a real Spectrum message id.
  • Observed intermittent Photon upstream send drops now surface as classified upstream_transient retryable errors instead of generic/blank failures.

Happy to split this if maintainers prefer a narrower merge path. The natural split would be:

  1. stream health + reconnect behavior (fix(photon): recover degraded upstream stream #51105-equivalent),
  2. outbound DM/error classification behavior (fix(photon): harden iMessage outbound recovery #51075-overlap),
  3. adapter-specific connect timeout for slow Photon startup,
  4. spectrum-ts@5.2.0 sidecar upgrade.

Classify Photon sidecar control failures, surface retryability to the adapter, and queue retryable runtime adapter failures for immediate gateway reconnection.

Add sidecar stream-health reporting so a live local sidecar with a degraded upstream Photon stream no longer looks healthy forever. The adapter polls /healthz and promotes degraded upstream stream state into the same retryable fatal path used for sidecar crashes.

Regression coverage verifies retryable sidecar errors, sidecar crash recovery, degraded stream health recovery, and gateway reconnect wake-up behavior.
@tusharaggarwa1

Copy link
Copy Markdown
Author

Coordination update after reviewing the newer related Photon PRs, especially #51161 and #49653.

I agree that #51161 looks like the clean stream-recovery landing path if maintainers prefer it. It salvages #51105/#50071 and adds an important follow-up fix this branch should not ignore: Spectrum WARN/INFO stream lines can land on console.log, not only console.error, so the degraded-stream detector needs to observe both channels.

How I see the current split:

I am not trying to supersede the maintainer salvage in #51161. The maintainer-friendly path I expect is:

  1. Let fix(photon): recover degraded upstream stream (salvage #51105 + #50071) #51161 land first if maintainers prefer it as the stream-recovery base.
  2. Rebase this PR on current main.
  3. Drop any duplicate stream-recovery hunks already covered by fix(photon): recover degraded upstream stream (salvage #51105 + #50071) #51161.
  4. Keep only the remaining outbound/timeout/DM/runtime-classification fixes, with focused tests and the live repro evidence already captured here.

Happy to split those remaining pieces further if that is easier to review.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for preserving the residual Photon recovery work and documenting the overlap with #51161.

Problems

  • Current main already contains the stream-recovery path from 06cbc3bae and the console.log follow-up from 7f1c278db (plugins/platforms/photon/sidecar/index.mjs:171-204,526-559). Those hunks should not be salvaged again.
  • The PR's spectrum-ts 5.2.0 pin conflicts with current main's v8 pin at plugins/platforms/photon/sidecar/package.json:16, introduced by 4345b3e. The v5 patch guard/layout needs a v8-targeted port rather than a downgrade.
  • The newly adjustable timeout/health settings are environment-backed in the diff, but AGENTS.md:102-106,620-623 requires non-secret behavioral settings to be config.yaml-backed.

Suggested changes

  • Preserve and re-target only the still-live behavior: current main still has the 15-second readiness deadline (plugins/platforms/photon/adapter.py:984-1005), phone-create-before-DM-get resolution (plugins/platforms/photon/sidecar/index.mjs:661-683), and unclassified sidecar reply handling (plugins/platforms/photon/adapter.py:1552-1561).
  • Keep Spectrum v8 and add config-resolution coverage for any new knobs.

Automated hermes-sweeper review.

},
"dependencies": {
"spectrum-ts": "3.1.0"
"spectrum-ts": "5.2.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current main is now pinned to spectrum-ts 8.0.0 by 4345b3e. Please port the remaining recovery behavior to the v8 sidecar and retain that dependency rather than restoring this 5.2.0 pin and its incompatible lockfile/patch layout.

).lower() not in ("0", "false", "no")
self._sidecar_health_interval = max(
0.0,
_coerce_float(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This introduces a user-facing timeout/threshold through environment variables. AGENTS.md:102-106,620-623 requires non-secret behavioral settings to be config.yaml-backed; bridge to environment only internally if the sidecar requires it.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/streaming Streaming responses: gateway delivery, provider wire labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing — the healthz stream-health and degraded→fatal→reconnect core of this landed earlier (adapter health monitoring on main), the spectrum 5.2.0 upgrade is obsolete against the 8.0.0 pin, and the structured-error/retry work landed via #73563. The DM get-before-create idea remains a good one — resubmit that narrowly if you're up for it.

@teknium1 teknium1 closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/streaming Streaming responses: gateway delivery, provider wire comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants