Skip to content

fix(hive): keep HITL notification classification type-aware - #677

Open
HsienW wants to merge 1 commit into
HarnessMD:mainfrom
HsienW:fix/worker-wake-hitl-notification-classification
Open

HsienW wants to merge 1 commit into
HarnessMD:mainfrom
HsienW:fix/worker-wake-hitl-notification-classification

Conversation

@HsienW

@HsienW HsienW commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

What & why

WorkerWake and the renderer currently classify Notification hooks independently, which allows mixed HITL and idle wording to fall through as idle when no authoritative notification type is available.

This change moves notification classification into a shared helper, treats known structured notification types as authoritative, and uses HITL first message heuristics only when the type is missing or unknown. It also passes notification_type through to WorkerWake and safely handles non string runtime values without changing the existing NDJSON framing or provider behavior.

Closes #676

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Docs
  • Build / CI

Evidence

Before

The same mixed Notification is classified as idle on main:

Event:    Notification
Message:  Permission required — waiting for your input
Actual:   idle
Expected: needsHuman
2026-10-03-141720

After

The same Notification is classified as needsHuman after the fix:

Event:    Notification
Message:  Permission required — waiting for your input
Actual:   needsHuman
Expected: needsHuman
2026-10-03-141838

The focused WorkerWake regression suite also passes with all 22 tests:

tests 22
pass 22
fail 0
2026-10-03-141903

How I tested it

  • OS: Windows 11
  • Steps:
    1. Reproduced the current main behavior with the mixed Notification and confirmed it returns idle.
    2. Ran the same reproduction on this branch and confirmed it returns needsHuman.
    3. Ran:
      node --test test/worker-wake.test.cjs
      Result: 22/22 passed.
    4. Ran:
      npm run typecheck
      Result: both node and web typechecks passed.
    5. Ran:
      git diff --check
      Result: passed.
    6. Ran:
      npm run test:focused
      Result: 967 tests total, 924 passed, 29 failed, and 14 skipped. The remaining failures are existing Windows specific symlink, Electron only, CRLF regex, external data, and concurrency failures. No WorkerWake tests failed, and this branch introduced no new failures relative to the Windows baseline.

No Electron launch was required for the reproduction or focused verification.

Checklist

  • Before and after evidence is attached above, under both headings.
  • npm run typecheck passes.
  • npm run test:focused passes.
  • npm run build succeeds.
  • This PR is one change. Unrelated fixes belong in their own PR.
  • I read the diff myself before opening this, and there is no debug output,
    commented-out code, or unrelated formatting churn in it.
  • Any new UI derives from DESIGN.md / tokens.ts — no ad-hoc colors,
    spacing, or fonts.
  • If I added art, it's my own or compatibly licensed, and listed in
    ATTRIBUTION.md.

- Share notification classification across WorkerWake and renderer paths.

- Prefer known structured notification types and fall back to HITL-first message heuristics.

- Pass notification types into WorkerWake and guard malformed runtime field values.
@HsienW
HsienW requested a review from chaitanyagiri as a code owner October 3, 2026 14:43

This branch has not been deployed

No deployments
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.

Worker wake can misclassify mixed HITL and idle notification wording

1 participant