Skip to content

ConfigurationCommandRequest.reason is loosely typed string, unlike its sibling PrCommandRequest #6616

Description

@JSONbored

Context

src/github/configuration-command.ts and src/github/pr-command-request.ts implement the same documented
pattern: a PURE, exhaustively-tested "guard preamble" classifier for an @loopover PR/issue-comment command,
returning a discriminated { ok: true; ... } | { ok: false; reason: ...; ... } union so a webhook handler
carries a single ok branch instead of re-deriving the same guards. configuration-command.ts's own doc
comment (src/github/configuration-command.ts:4-7) explicitly frames itself this way and cross-references
issue #2168; pr-command-request.ts's doc comment (src/github/pr-command-request.ts:1-8) describes the
identical pattern and explicitly says it mirrors classifyPlanCommandRequest.

pr-command-request.ts types its failure reason as a proper string-literal union
(src/github/pr-command-request.ts:15-23):

export type PrCommandRequest =
  | { ok: true; ... }
  | { ok: false; reason: "unsupported_comment_action" | "bot_author" | "missing_repo_pr_installation_or_actor"; ... };

configuration-command.ts types the same conceptual field as a bare string
(src/github/configuration-command.ts:8-10):

export type ConfigurationCommandRequest =
  | { ok: true; repoFullName: string; installationId: number; actor: string; issueNumber: number }
  | { ok: false; reason: string; repoFullName: string | null; actor: string | null; targetKey: string | null };

In practice classifyConfigurationCommandRequest (src/github/configuration-command.ts:12-30) only ever
returns two literal reason strings, "unsupported_comment_action_or_bot" (line 24) and
"missing_repo_issue_installation_or_actor" (line 27) — both already pinned by exact-string assertions in
test/unit/configuration-command.test.ts:42-58 — but the loose string type means a future typo in either
literal (or a third ad hoc reason string added later) would type-check silently instead of being caught at
compile time, unlike pr-command-request.ts's stricter sibling.

Requirements

  • Change ConfigurationCommandRequest's reason field in src/github/configuration-command.ts:10 from
    string to the exact literal union of the two values classifyConfigurationCommandRequest actually
    returns: "unsupported_comment_action_or_bot" | "missing_repo_issue_installation_or_actor", matching the
    literal-union style already used by PrCommandRequest["reason"] in src/github/pr-command-request.ts:23.
  • This is a type-only change — classifyConfigurationCommandRequest's runtime behavior, return values, and
    every existing passing assertion in test/unit/configuration-command.test.ts must be unaffected. No new
    reason values may be introduced as part of this fix.
  • Any other file that imports ConfigurationCommandRequest or destructures a reason field from its return
    value must continue to typecheck; if the stricter type surfaces a real type error elsewhere, fix the call
    site to use the correct literal rather than widening the type back to string.

Deliverables

  • ConfigurationCommandRequest's reason field in src/github/configuration-command.ts is a
    "unsupported_comment_action_or_bot" | "missing_repo_issue_installation_or_actor" literal union
    instead of string.
  • npm run typecheck (or the repo's equivalent TypeScript check script) passes with no new errors.
  • test/unit/configuration-command.test.ts continues to pass unmodified, confirming no behavior changed.

Test Coverage Requirements

This is a type-annotation-only change with no new runtime branch, so no new test assertions are strictly
required by the Codecov patch gate — the existing test/unit/configuration-command.test.ts suite (already
asserting both literal reason values via toMatchObject, e.g. lines 42 and 52) continues to provide full
coverage of classifyConfigurationCommandRequest. Do not reduce or remove any existing test coverage while
making this change.

Expected Outcome

ConfigurationCommandRequest carries the same compile-time guarantee PrCommandRequest already does: a
typo or unintended new value assigned to reason anywhere in src/github/configuration-command.ts (or by
any future caller constructing this type) is caught by tsc, instead of silently type-checking as a bare
string.

Links & Resources

  • src/github/configuration-command.ts:8-30 (the type and its single producer, classifyConfigurationCommandRequest)
  • src/github/pr-command-request.ts:15-23 (PrCommandRequest, the literal-union pattern to mirror)
  • test/unit/configuration-command.test.ts:42-58 (existing tests pinning the two literal reason values)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions