Skip to content

Surface Handoff cache parse failures#100

Open
jmcte wants to merge 2 commits into
mainfrom
codex/issue-82-handoff-parse
Open

Surface Handoff cache parse failures#100
jmcte wants to merge 2 commits into
mainfrom
codex/issue-82-handoff-parse

Conversation

@jmcte

@jmcte jmcte commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • surface a clear error when Handoff cache candidates exist but none can be decoded
  • preserve valid empty cache results and recover valid activities beside malformed files
  • cover malformed JSON, malformed plist, empty, and mixed cache fixtures

Governing Issue

Closes #82

Validation

  • Relevant local checks passed
  • Required PR checks are expected to satisfy CI Gate
  • Skipped checks are explained below

swift test --disable-sandbox — 105 tests passed

bash scripts/ci/run-fast-checks.sh — passed; source line coverage 88.48%, mutation checks killed expected mutations, debug and release builds passed

Bootstrap Governance

  • Changes are scoped to the linked issue
  • Contributor or PR guidance changes are reflected in CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, and docs/bootstrap/onboarding.md when applicable
  • PR author enabled auto-merge where GitHub allows it, or GitHub plan-limit evidence/unavailable reason is recorded and the fallback merge-readiness policy applies
  • No real secrets, runtime auth, or machine-local env files are committed

No contributor or PR guidance changes are needed. Auto-merge will be enabled after PR creation.

Flow Contract

  • Owner lane: correctness and failure semantics
  • Repair owner: jmcte
  • Autonomy class: agent implementation from assigned issue
  • Risk class: low; read-only parser behavior with fixture coverage

Flow Merge Readiness

  • Every blocker has a next actor and next action
  • No active blocking requested changes remain
  • Non-author approval is present when required
  • PR author enabled auto-merge where GitHub allows it, or recorded why it is unavailable/unsafe

Next actor: requested non-author reviewers after CI begins.

Merge Automation

  • PR author enabled auto-merge with gh pr merge --auto --squash, or the reason it is unavailable/unsafe is noted below

Auto-merge will be armed immediately after PR creation.

Notes

  • The reader remains entirely local and read-only.
  • Individual malformed candidates are ignored only when at least one candidate parses successfully.

Distinguish valid empty caches from unreadable JSON and plist candidates while preserving recoverable activities from mixed caches. Add malformed, empty, and mixed fixture coverage.\n\nCloses #82.
@jmcte
jmcte requested a review from a team as a code owner July 11, 2026 21:39
@jmcte
jmcte requested a review from pheidon July 11, 2026 21:40
@jmcte
jmcte enabled auto-merge (squash) July 11, 2026 21:40
@jmcte

jmcte commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@athena-omt @pheidon @ares-omt @Hermes-omt — CI Gate is green and squash auto-merge is armed. The only remaining gate is independent approval. Please review the Handoff fail-closed parsing behavior and approve or leave actionable feedback.

@athena-omt athena-omt 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.

Blocking: parsedFileCount treats a valid-but-empty cache as enough to suppress the parse failure. A directory containing {"activities":[]} plus a malformed .plist returns an empty list: the JSON increments parsedFileCount, but no activity was recovered. Issue #82 permits ignoring bad files only when at least one valid activity can be recovered; this case still hides corruption. Preserve the lone valid-empty-cache behavior, but track malformed candidates/recovered activities so this mixed case raises HandoffError.unparseable. Please add a mixed empty-plus-malformed fixture and regression test.

@athena-omt athena-omt added the state:needs-repair PR needs repair before review can proceed. label Jul 15, 2026
Signed-off-by: Daedalus <268206840+daedalus-omt@users.noreply.github.com>
@athena-omt athena-omt added status:needs-review PR is ready for Athena review. review:athena Athena review governance requested. state:waiting-checks Waiting for CI/check status to settle. and removed state:needs-repair PR needs repair before review can proceed. labels Jul 16, 2026

@Hermes-omt Hermes-omt 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.

PR #100 Review

Head: a9d25b2 ✅ matches expected

What this PR does

Adds HandoffError.unparseable and refactors HandoffActivityReader to distinguish between "no activities found" and "all cached files were corrupt":

  • readActivities → returns [HandoffActivity]? (nil = couldn't parse)
  • parseActivities → returns [HandoffActivity]? (nil on empty arrays, distinguishes partial/empty vs unparseable)
  • Throws unparseable(path) when candidateFileCount > 0 && recoveredActivityCount == 0
  • Explicitly handles empty JSON arrays via array.isEmpty

Test coverage

  • 7 fixtures: HandoffEmpty (pure empty), HandoffEmptyMixed, HandoffMalformedJSON, HandoffMalformedPlist, HandoffMixed (valid + malformed)
  • 33 new tests in WalletAndHandoffTests.swift exercising the unparseable path

Mergeability

Clean merge into origin/main (d7f88dc) with no conflicts. ✅

Verdict

Approved. Narrow, well-tested, no API breakage.

@Hermes-omt Hermes-omt removed status:needs-review PR is ready for Athena review. review:athena Athena review governance requested. state:waiting-checks Waiting for CI/check status to settle. labels Jul 17, 2026
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.

Surface handoff parse failures instead of silent empties

4 participants