Skip to content

refactor(cli): replace probe/decider/detail split with single typed extractor - #5191

Merged
wpfleger96 merged 3 commits into
mainfrom
duncan/cli-archive-fail-closed
Aug 7, 2026
Merged

refactor(cli): replace probe/decider/detail split with single typed extractor#5191
wpfleger96 merged 3 commits into
mainfrom
duncan/cli-archive-fail-closed

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Replaces the four-helper auth resolution path with two focused functions and adds production async tests that count relay round-trips.

Before: resolve_auth called resolve_auth_from_profile (warn-emitting probe into a throwaway sink) → resolve_auth_deciding (re-classified the same profile) → handle_auth_failureauth_failure_detail (third classification). Option<Option<&Value>> encoded a sentinel for unreachable state; tests exercised only the pure sync helper, not the actual fetch count.

After:

  • extract_auth(profile, target, signer) -> Result<[String;4], AuthFailure> — pure typed extractor; AuthFailure now covers NoProfile and NoTagsArray inline, no separate helper needed
  • resolve_auth() is now the linear state machine: self-check → fetch + extract → on failure: fetch again → route final Err to CliError::Usage (default) or one admin warning (--admin). No throwaway sinks, no duplicate classification, no sentinel type.
  • Five async tests drive the production resolver through a counted Axum test server on POST /query and assert on both return value and exact fetch count: first success (1), retry success (2), double failure / no --admin (2 + Err), double failure / --admin (2 + Ok(None) + one warning), self path (0). Two parser tests pin --admin on both archive and unarchive.
  • --admin short help text corrected to describe when the flag takes effect (after extraction fails, not unconditionally).

341 tests passing, clippy clean, fmt clean.

When owner-auth extraction fails for a non-self target, the CLI now:

1. Retries the kind:0 fetch once (transient profile republish is the
   dominant failure mode and the retry resolves it silently).
2. If the retry also fails, returns a hard error naming the extraction
   failure and the --admin escape hatch — the bare request is NOT sent.

This replaces the previous behaviour where extraction failure emitted a
warning and then sent a bare request that the relay would reject with
400 unless the signer happened to be a relay admin.

The --admin flag is added to both archive and unarchive. It restores
the old warn-and-proceed-bare path for genuine relay-admin callers who
legitimately archive without owner attestation. The self path (target
== signer) is unchanged and always sends bare.

Ten new unit tests cover resolve_auth_deciding and auth_failure_detail:
first-success, retry-success, both-fail-no-admin (Err), both-fail-admin
(Ok(None)+warning), no-kind0-fail, no-retry-sentinel, and four
auth_failure_detail precision cases.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 requested a review from a team as a code owner August 7, 2026 15:23
…xtractor

Replace the four-helper auth resolution path (resolve_auth_from_profile →
resolve_auth_deciding → handle_auth_failure → auth_failure_detail) with
two focused functions:

- extract_auth(profile, target, signer) -> Result<[String;4], AuthFailure>:
  pure typed extractor; AuthFailure now covers NoProfile and NoTagsArray
  (previously handled via warn-emitting branches) alongside the existing
  tag-validation variants.

- resolve_auth(): now the linear state machine itself — self-check → fetch
  + extract → on failure: fetch again → route final Err to CliError::Usage
  or one admin warning. No throwaway sinks, no duplicate classification,
  no Option<Option<_>> sentinel.

Replace the sync resolve_auth_deciding/auth_failure_detail unit tests with
five async tests that drive the production resolver through a counted Axum
test server on POST /query. Each test asserts on both the return value and
the exact number of /query calls:
  - first success → 1 fetch
  - retry success → 2 fetches
  - double failure, no --admin → 2 fetches + Err(CliError::Usage)
  - double failure, --admin → 2 fetches + Ok(None) + exactly one warning
  - self path → 0 fetches

Add two parser tests pinning --admin on both archive and unarchive.

Fix --admin short help to accurately describe when the flag takes effect
(after extraction fails, not unconditionally).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 changed the title fix(cli): fail closed on archive/unarchive auth extraction failure refactor(cli): replace probe/decider/detail split with single typed extractor Aug 7, 2026
…_auth_profile

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 merged commit b2ac66c into main Aug 7, 2026
33 checks passed
@wpfleger96
wpfleger96 deleted the duncan/cli-archive-fail-closed branch August 7, 2026 17:12
atishpatel added a commit that referenced this pull request Aug 7, 2026
…-log-harness

* origin/main:
  feat(desktop): adding rich link previews to messages (#3818)
  fix(buzz-agent): Responses reasoning summary, Anthropic display:summarized, ACP v2 messageId (#5195)
  fix(desktop): retain distinct agent instances in autocomplete (#5202)
  fix(desktop): defer channel visibility change to Save (#5203)
  feat(desktop): Projects follow-ups — access restrictions, fast loading, activity feed polish (#5073)
  refactor(cli): replace probe/decider/detail split with single typed extractor (#5191)
  fix(desktop): drop unhandled rejection from throwing window.Notification (#5143)
  fix(desktop): fence localStorage SecurityError from killing the React tree (#5142)
  fix(desktop): make terminal output selectable (#4980)
  fix(desktop): use WEBKIT_DMABUF_RENDERER_FORCE_SHM for NVIDIA/AppImage (#3654) (#4505)
  Make public starter channels best effort (#5192)
  Mobile: add anchored reaction popover (#5025)
  feat(mobile): add bee pull-to-refresh (#5059)

Signed-off-by: Atish Patel <atish@squareup.com>
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.

1 participant