Skip to content

Wizard sync-now step trusts exit code 3 as proof of no-destinations with no corroborating signal #1445

Description

@philcunliffe

Source

Deferred from PR #1441 (fix/issue-1437, triaged head ddeb6d2e535a1405f369aa1191cba82a73b23f9b), finding 6 of the last review round (head 3adc64b8). Severity: low.

Where

src/core/cli/wizard/sync_now.js:139 (the result.code === SYNC_HELD_NO_DESTINATIONS_EXIT branch), with the constant defined at src/core/usage-policy/first_sync_hold.js:37.

Observed behavior

The wizard's send-now step reads the spawned hyp sync child's exit code and, on 3, unconditionally reports the no-destinations outcome: it prints the "no destination is configured" closing statement and records sync_now: no-destinations, with no corroborating signal. But exit code 3 is not exclusively HypAware's: Node itself exits 3 on an internal JS parse error, and nothing structurally prevents a future runSync path from returning 3 for a different reason. In either collision the wizard prints a confident false explanation and records a wrong outcome value in the LLP 0203 telemetry.

Why deferral is safe

  • The collision is speculative: a Node internal parse-error exit means the child crashed before running any sync logic (an already-broken build where a wrong closing sentence is the least of the problems), and no current code path returns 3 for anything else (bin/hypaware.js otherwise uses 0/1/2/75).
  • Worst case is a wrong closing sentence and one mislabelled telemetry value. No data is sent, lost, or mislabelled as released: the false-release path is already guarded, because commit 3adc64b8 deliberately made code 3 outrank the fail-open marker re-read.
  • The obvious hardening (also require stillHeld !== null) was considered and declined in review: it reintroduces the worse failure mode, where a fail-open marker read turns a provably-sent-nothing run into a claimed release, the exact polarity LLP 0203 #read-back settles against. Any fix here needs a different corroborating signal, not that one.

Acceptance condition

A child process that exits 3 without having been dispatched into the held/no-destinations branch of runSync (for example, a child killed by a Node-internal error before runSync ran, or a simulated future path returning 3) is not reported as no-destinations: the wizard falls back to the marker-driven outcome (sync-declined while held), and a regression test in test/core/cli/wizard/sync_now.test.js proves it, while the existing test "a no-destinations child is not a release even when the marker reads absent" still passes.

Re-triaged at PR #1441 head 791a5d996229ce660add831648aebec3b11075f7 (a master merge over the previously triaged head; the finding and the code it names are unchanged). This issue remains the tracker.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    neutral:fixDelegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions