Commit Graph
4 Commits
Author SHA1 Message Date
akushonkamen 3cf0e709d5 fix(github-coordination): reject malformed issue numbers instead of truncating them
normalizeIssueNumber parsed with Number.parseInt, which stops at the
first non-digit ("42oops" -> 42, "42.9" -> 42, "4e2" -> 4) and loses
precision past 2^53 (9007199254740993 became 9007199254740992). The CLI
shares this helper for --issue, --limit, and the positional issue number,
so malformed values silently targeted the wrong issue before any integer
guard downstream could notice.

Validate the entire trimmed decimal token and require
Number.isSafeInteger so malformed or out-of-range values throw
"Invalid issue number" during argument parsing, before the state store
is opened or GitHub is contacted. Whitespace, leading plus, and leading
zero tolerance is preserved. Regression coverage added at both the
library level (tests/lib/github-coordination-gh-api.test.js) and the CLI
argument parser (tests/scripts/github-coordination.test.js).

Fixes #3514
2026-10-12 01:25:09 +08:00
Wu ShuwenandGitHub 3033436dcc fix: filter epic sync issues by label (#3089)
github-coordination sync listed every repo issue and pushed the epic label onto all of them (#3084). Scope the listing to issues carrying the policy's epic label plus issues whose body still holds the coordination marker (label-drift recovery), deduped by number, and reject an empty labels.epic in loadPolicy. Tests cover the filtered path and the recovery path with exact gh argv. Independent exact-head review passed with no P0/P1; CI 44/44 at the head.
2026-09-12 01:46:04 +01:00
Victor CasadoandClaude Sonnet 4.6 33f2219307 fix: address second round of code-review findings
actions.js:
- Add assertValidRepo/assertValidIssueNumber guards at the top of all
  action handlers (applyClaim, applySync, applyValidate, applyPublish,
  applyReview, applyDecompose, applyUnblock) for fast-fail validation
- applyValidate: fix status transition — set 'validated' unconditionally
  when ok=true instead of preserving 'blocked' (was inconsistent with
  projectState becoming 'ready')

gh-api.js:
- runGh: preserve GITHUB_TOKEN by default; only delete when caller
  explicitly sets options.stripGithubToken=true (was deleting by
  default, breaking CI)

parsing.js:
- extractCoordinationState: throw SyntaxError on malformed JSON instead
  of silently returning null — lets callers distinguish bad JSON from
  absent marker
- normalizeBodyForComparison: fix regex to match JSON-quoted form
  "lastSyncAt": ... instead of bare lastSyncAt: ...

policy.js:
- loadPolicy: validate that parsed JSON is a plain object before
  spreading; coerce nested fields (labels, review, validation,
  branchModel, project, fieldNames) to objects before merging

state.js:
- assertIssueClaimable: block re-claim on status alone (not status AND
  owner) to prevent {status:'claimed', owner:null} bypass; use
  state.owner || 'unknown' in error message
- getCoordinationState: catch SyntaxError from extractCoordinationState,
  log warning to stderr, fall back to default state

tests/lib:
- Update malformed-JSON test to expect SyntaxError throw instead of null

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-11 14:25:58 -04:00
Victor CasadoandClaude Sonnet 4.6 d4486a7a29 refactor: apply code-review findings to github-native coordination
scripts/github-coordination.js:
- parseArgs: replace 13-entry if/else chain with BOOL_FLAGS/VALUE_FLAGS
  lookup maps; shrinks from 119 to ~45 lines
- Extract dispatchCommand(options, ctx) and formatOutput(payload, options)
  from main(); main() shrinks to ~20 lines

scripts/lib/github-coordination.js:
- Split 1041-line monolith into 6 focused sub-modules under
  scripts/lib/github-coordination/ (policy, parsing, gh-api, state,
  actions, store); index becomes a thin re-export (~55 lines)
- Document ECC_GH_SHIM trust boundary in runGh() (gh-api.js)
- Document applyClaim() read→check→write race condition (actions.js)

tests/lib/github-coordination.test.js:
- Refactor runTests() to data-driven DESCRIPTORS array + runGroup()
  helper; runTests() shrinks to ~10 lines
- Add 5 new edge-case tests: normalizeRepo('') and normalizeRepo('   ')
  throw, desiredLabelsForState for blocked/ready statuses, and
  buildIssueStateFromAction for validate action (15 → 20 tests)

tests/scripts/github-coordination.test.js:
- Replace console.log in test runner with process.stdout.write

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-11 14:05:42 -04:00