Skip to content

[quality] run()'s top-level setFailed catch (src/run.ts:40) is never reached by the bundle e2e suite — a body-less issue_comment payload drives it through dist/index.js #360

Closed

Description

@hivecommons-hive

Finding

src/run.ts wraps event dispatch in one try/catch whose catch is the action's last line of defence: core.setFailed(error instanceof Error ? error.message : String(error)) (line 40). Every registered handler swallows its own rejections (runEventHandlers collects them, handleCronJobs and handleIssueComment .catch per job/command), so nothing in the bundle suite ever reaches that catch.

Evidence, main @ 3fc21f2:

  • Unit: npx vitest run --coverage → run.ts 100 % lines / 100 % branches; __tests__/run.test.ts:108-125 spy on core.setFailed for both the Error and the non-Error arm.
  • E2E: npm run test:coverage:e2e → run.ts 88.88 % lines, line 40: 0 hits. __tests__/bundle/bundle.test.ts:63 drives the sibling not yet supported arm (line 34) but nothing throws out of a handler.

The catch is reachable from dist/index.js. handleIssueComment tests the comment body against every configured command (commandForms(command).some(form => hasCommand(form, commentBody))) outside the per-command .catch(normalizeError), and hasCommand → findCommandArgs → commandLines → splitLines calls body.replace(...) unguarded. An issue_comment payload with no comment object, or with comment.body: null, therefore throws a TypeError straight up to run(). Reproduced against the fake GitHub server with prow-commands: /assign /lgtm:

exit 1   ::error::Cannot read properties of undefined (reading 'replace')   (no API calls)
exit 1   ::error::Cannot read properties of null (reading 'replace')        (no API calls)

That is the behaviour a consumer sees today for a malformed/truncated event payload: the run fails with an opaque property-access message rather than naming the missing comment body. The test pins the current contract (fails once, zero writes); whether the message should be friendlier is a product question outside this lane.

Of the 41 open hold-gated test(bundle) PRs, none drives a body-less comment: #343 covers the missing issue/pull number guards (body present), #324/#357 drive commandLines fences and alias canonicalization with a body. This cluster — src/run.ts:40 via handleIssueComment's unguarded body match — is disjoint.

Recommendation

  • Add __tests__/bundle/runTopLevelCatch.test.ts: run dist/index.js on issue_comment with (a) no comment object and (b) comment.body: null, assert exit 1, exactly one ::error:: line, and no API requests. Brings run.ts e2e lines to 100 %; the remaining String(error) branch is unreachable from the bundle (every throw here is an Error) and stays unit-only.

Priority

  • Impact: medium (covered by unit tests, not by e2e; the one arm that turns an unexpected throw into a red run)
  • Effort: low

Filed by quality agent (hold-gated mode)


🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: 3fc21f2

— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88

Activity

  1. github-actions commented on Oct 7, 2026

    @github-actions
    Contributor

    Please add a kind label with /kind failing-test or /kind cleanup.

  2. added
    🐝 covered by PRHive verified that an open PR references or claims this issue; still actionable until confirmed
    and removed
    🐝 covered by PRHive verified that an open PR references or claims this issue; still actionable until confirmed
    on Oct 7, 2026
  3. hivecommons-hive commented on Oct 8, 2026

    @hivecommons-hive
    ContributorAuthor

    Closing: the premise no longer holds on main.

    #374 (dec749d) guards the comment body in handleIssueComment (context.payload.comment?.body ?? ''), which was the only reachable route from the bundle to run()'s top-level catch. Re-ran the reproduction against the current dist/index.js (main @ aa6a0f8, prow-commands: /assign /lgtm):

    • no comment object → exit 0, no ::error::, 0 API requests
    • comment.body: null → exit 0, no ::error::, 0 API requests

    The no-body case itself is unit-covered by __tests__/issueCommentTest/commentWithoutBody.test.ts (from #374). Every other path before the catch is guarded: skipReadOnlyForkRun uses optional chains and typeof checks, runEventHandlers catches per handler, handleCronJobs/handleIssueComment catch per job/command, and sweep() catches per step. run.ts:40 is therefore a defensive catch with no realistic input reaching it from the bundle (npm run test:coverage:e2e on aa6a0f8: run.ts line 40, 0 hits; all other lines hit); it stays unit-only (__tests__/run.test.ts). PR #361 was closed for the same reason.


    🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: aa6a0f8

    — hive: agent=quality backend=copilot model=claude-fable-5.1

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

    agent/qualityCreated by Hive for agent-filed issue provenanceneeds-kindqualityCreated by Hive for agent-filed issue provenancetestingCreated by Hive for agent-filed issue provenance

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions