Skip to content

[AIG-637] Issue-to-PR GitHub/GitLab integration (M1-9)#26

Merged
blackms merged 6 commits into
mainfrom
pm-agent/aig-637-issue-to-pr-github
May 29, 2026
Merged

[AIG-637] Issue-to-PR GitHub/GitLab integration (M1-9)#26
blackms merged 6 commits into
mainfrom
pm-agent/aig-637-issue-to-pr-github

Conversation

@blackms
Copy link
Copy Markdown
Owner

@blackms blackms commented May 28, 2026

Implements AIG-637.

Milestone: M1 - Production
Estimate: 8 pts

Summary

See commit message for full implementation details (schema, API, CLI, tests, docs).

Notes from PM agent

DEPENDS ON AIG-636 webhook framework. Branch ships a local stub at src/transport/webhook.ts that should resolve via 'git checkout --theirs' when AIG-636 lands first.


Auto-opened by aistack PM agent on 2026-05-28 10:22. Review with /review or human dispatch.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added aistack ingest issue <url> CLI command for GitHub/GitLab issue ingestion
    • Added GitHub and GitLab webhook endpoints for issue automation
    • Automated draft PR/MR creation with iterative review and lifecycle labels
    • Duplicate dispatch protection and rate-limit handling
  • Documentation

    • Added GitHub/GitLab integration guide covering CLI, webhooks, and configuration

Review Change Stack

…bhook + CLI ingest

End-to-end "issue assigned to bot -> spawn coordinator + coder + reviewer
agents -> open draft PR" workflow reachable from a CLI command
(`aistack ingest issue <url>`) or via webhook (POST /v1/github/webhook,
POST /v1/gitlab/webhook).

New modules under src/github/: parses GitHub + GitLab issue URLs, fetches
the issue via the provider REST API (raw fetch, zero new deps), drives the
existing ReviewLoopCoordinator on the generated plan, renders a PR
description with plan + review iterations + audit-trail link, and posts a
draft PR/MR. Lifecycle labels (aistack-claimed/in-progress/
blocked-needs-human/done) are written back to the source issue on each
phase transition.

New transport routes register on the AIG-636 WebhookServer. To unblock
this branch ahead of AIG-636 landing on main, a minimal stub
src/transport/webhook.ts ships here with an interface compatible with
AIG-636 (WebhookServer + verifyHmacSignature). When AIG-636 merges first
the stub should be REPLACED (not deleted); both the GitHub and GitLab
route handlers import the stub via its public surface and need no changes.

GitHubConfig grows webhookSecret/gitlabWebhookSecret/gitlabToken/labels/
auditUrlTemplate fields (all optional, defaults preserved). CLI receives
a new `ingest` Commander chain registered in src/cli/index.ts. No
modifications to existing webhook/daemon/review-loop code.

Tests: tests/unit/github/{ingest,coordinator,webhook}.test.ts cover URL
parsing, provider routing, label transitions, PR body rendering,
HMAC verification, and a live WebhookServer integration test (ping +
401 invalid signature + 202 non-actionable event).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 28, 2026

Warning

Review limit reached

@blackms, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 2 minutes and 59 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a779f00c-353d-4874-97de-af62c59931d1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e27fb8 and 773fbeb.

📒 Files selected for processing (13)
  • src/cli/commands/index.ts
  • src/cli/commands/ingest.ts
  • src/cli/index.ts
  • src/github/coordinator.ts
  • src/github/providers.ts
  • src/transport/github-webhook.ts
  • src/types.ts
  • src/utils/config.ts
  • tests/unit/cli-ingest.test.ts
  • tests/unit/config.test.ts
  • tests/unit/github/coordinator.test.ts
  • tests/unit/github/ingest.test.ts
  • tests/unit/github/webhook.test.ts
📝 Walkthrough

Walkthrough

This PR adds a complete end-to-end GitHub/GitLab issue-to-draft-PR integration. It introduces provider clients for fetching issues, a workflow coordinator that orchestrates planning and review loops, webhook handlers for automated dispatch, and a CLI command for manual triggering. Issues progress through lifecycle labels (claimed → in-progress → blocked/done), and PR bodies include generated plans, adversarial review results, and audit trails. Dispatch deduplication prevents concurrent duplicate workflows.

Changes

Issue-to-PR Workflow Integration

Layer / File(s) Summary
Provider infrastructure and issue ingestion
src/github/providers.ts, src/github/ingest.ts, src/github/index.ts, src/types.ts, src/utils/config.ts, tests/unit/github/ingest.test.ts
Unified GitHub/GitLab provider client using raw fetch with URL parsing, issue detail fetching (labels, assignees, state normalization), label updates, and draft PR/MR creation; rate-limit retry with exponential backoff; ingestIssue entry point that returns normalized issue plus reusable provider client; configuration schema support for webhook secrets and GitLab auth.
Workflow orchestration, labels, and PR body rendering
src/github/coordinator.ts, src/github/labels.ts, src/github/pr-body.ts, src/github/dispatch-dedupe.ts, src/github/index.ts, tests/unit/github/coordinator.test.ts
runIssueToPRWorkflow coordinates issue→PR conversion via idempotent lifecycle label transitions (claimed → in-progress → blocked/done), plan generation, review loop execution, and draft PR opening; applyLifecycleLabel atomically replaces lifecycle labels while preserving user labels; buildPrBody renders Markdown sections for summary, plan, review iterations, and audit trail; beginIssueDispatch/finishIssueDispatch suppress concurrent and recently-completed workflow triggers.
Integration router and webhook handlers
src/transport/integration-router.ts, src/transport/github-webhook.ts, src/transport/gitlab-webhook.ts, tests/unit/github/webhook.test.ts
IntegrationRouter HTTP server handles POST requests, verifies HMAC signatures (GitHub sha256=, GitLab constant-time token), and dispatches to handlers; GitHub webhook filters actionable events (issue assignment by bot, labeled with aistack-claimed); GitLab webhook filters actionable events (open/update with bot in assignees); both handlers trigger async workflows via dedupe guards and return 202 Accepted immediately.
CLI ingest command and command wiring
src/cli/commands/ingest.ts, src/cli/commands/index.ts, src/cli/index.ts
New aistack ingest issue <url> command supports options for --dry-run, --no-labels, --base/--head branches, --max-iterations, and --watch progress streaming; loads config, calls ingestIssue and runIssueToPRWorkflow with mapped options, prints structured result (status, branch, PR URL), and exits with code 1 on failure.
Documentation, configuration, and test updates
docs/GITHUB_INTEGRATION.md, src/types.ts, src/utils/config.ts, tests/unit/consensus-service.test.ts, tests/unit/daemon/webhook.test.ts, tests/unit/monitoring/health.test.ts
Integration guide describing CLI/webhook entry points, configuration (webhook secrets, labels, audit URL template), label lifecycle, and limitations; configuration schema extensions for webhook/GitLab fields and label overrides; minor test refactors for resilience (consensus-service: direct DB update for expiry; webhook: file rewrite loop for debounce tolerance; health: extracted fakeMemoryUsage/withMemoryUsage helpers).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

The PR introduces significant new infrastructure across multiple layers (providers, orchestration, webhooks, CLI) with heterogeneous concerns: URL parsing, rate-limit handling, provider-specific API differences, label idempotency, HMAC signature verification, HTTP request handling, and workflow state management. While individual files are well-structured with clear responsibilities, reviewers must understand the cross-module data flow, the webhook dispatch lifecycle, lifecycle label semantics, and test mocking patterns. The change is substantial in scope (80 code ranges across 24 files) and introduces multiple new public contracts that affect downstream callers.

Possibly related PRs

  • blackms/aistack#1: Implements the ReviewLoopCoordinator and adversarial review iteration results that runIssueToPRWorkflow depends on for the review loop execution phase.

Poem

🐰 From issue's claim to PR's glory,
A rabbit-hopped workflow takes flight—
Labels bloom, reviews agree,
Webhooks ping and dedupe fights,
Draft PRs birth'd with delight! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.51% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: implementing issue-to-PR integration for GitHub and GitLab, with ticket reference (AIG-637) and scope notation (M1-9).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pm-agent/aig-637-issue-to-pr-github

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Owner Author

@blackms blackms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Request changes — solid scaffold, but stub interface is NOT compatible with AIG-636 and idempotency is missing

Nice clean module structure: the provider abstraction (src/github/providers.ts), the lifecycle label state machine (src/github/labels.ts), and the PR-body renderer (src/github/pr-body.ts) are well-separated and tested. The HMAC implementation in the stub itself is correct (createHmac + timingSafeEqual with length guard, body buffered before verification so the digest covers the exact bytes signed). Auth handling is also fine: tokens are read from env into private fields, never logged, never echoed into error messages — only HTTP status / statusText / response text are surfaced.

That said, there are blocking issues before this can merge after AIG-636.

🚨 Blocking

1. The "forward-compatible stub" claim is false — interfaces diverge from AIG-636.
The PR description and the comment block at the top of src/transport/webhook.ts both promise that WebhookServer, verifyHmacSignature and WebhookHandler will remain backward-compatible with AIG-636's contract. They are not:

Symbol AIG-637 stub (this PR) AIG-636 (#25)
WebhookServer ctor new WebhookServer(options) new WebhookServer(runtime, config) — requires a DaemonRuntime
Route registration addRoute({ path, handler, secret, signatureFormat, signatureHeader }) none — hard-coded POST /v1/tasks + GET /health
verifyHmacSignature signature (payload, signature, secret, format) (body, secret, providedSignature) — arg order swapped, no format
Signature comparison hex-string compare (sha256= prefixed) hex-decoded buffer compare
WebhookHandler type exported here does not exist in #25

Once AIG-636 lands on main, this stub cannot be "REPLACED" by it without rewriting github-webhook.ts and gitlab-webhook.ts, which both consume server.addRoute(...) and signatureFormat: 'github'|'gitlab'. Either AIG-636 needs to grow the routed surface this PR assumes (preferred — open an issue / coordinate with #25), or this PR needs to introduce an adapter layer (WebhookRouter) on top of AIG-636's DaemonRuntime-bound server.

2. No idempotency on issue → PR dispatch.
dispatchWorkflow in github-webhook.ts and gitlab-webhook.ts fires a brand-new workflow on every actionable event. Real-world consequences:

  • GitHub frequently re-delivers webhooks (manual redelivery, network retry); two issues.assigned events for the same issue produce two coordinator runs, two plan-agent spawns, two review loops, and two draft PRs against two random aistack/github-<n>-<uuid> branches.
  • The aistack-claimed label is checked on the labeled path but isn't used as a guard against re-entry — a webhook can race with the label write done inside runIssueToPRWorkflow step 1.
  • lastInvocation is a single mutable slot, so concurrent workflows for distinct issues clobber each other's promise reference (test-only helper, but the racing dispatches themselves are real).

Minimum mitigation: dedupe by (provider, owner, repo, issueNumber) with an in-flight set + completed cache (TTL on the order of hours), and short-circuit to 202 {dispatched:false,reason:'already in flight'} on collision. GitHub also sends X-GitHub-Delivery — log/store it for traceability.

3. No rate-limit / retry handling against GitHub or GitLab.
GitHubProvider.getIssue/setLabels/createPullRequest and the GitLab equivalents throw on any non-2xx, with zero awareness of 429, 403 rate limited, Retry-After, or X-RateLimit-Remaining: 0. On a busy bot user this will surface as GitHub setLabels failed: 403 Forbidden mid-workflow, leaving the issue in aistack-in-progress with no PR ever created. At minimum add:

  • A fetch helper that inspects 429/403 + x-ratelimit-remaining: 0 and waits the Retry-After / x-ratelimit-reset window once before re-throwing.
  • Differentiate auth failure (401) from rate-limit so the coordinator can mark blocked vs retry.

⚠️ Should fix before merge

4. coordinator.ts claim step happens after the URL parse but before any dedup, and the label is never set back to "claimed" on the labeled-event path — meaning the webhook flow that listens for aistack-claimed will trigger itself in a loop the first time it runs (it will re-apply the label, GitHub fires labeled again, etc.). Either suppress dispatch when the sender is the bot itself, or make label re-application not trigger.

**5. applyLifecycleLabel uses label.toLowerCase().startsWith('aistack-') to identify our labels — this will eat any user label starting with aistack- (e.g. aistack-discussion, aistack-roadmap). Restrict to the known set in DEFAULT_LABELS + overrides.

6. GitLab createPullRequest builds the body twice when draft: true:

body: JSON.stringify({
  title: params.title,           // <-- first title
  ...
  ...(params.draft ? { title: `Draft: ${params.title}` } : {}),  // <-- overwrites
}),

This works (object spread overrides) but is brittle and confusing — set const title = params.draft ? \Draft: ${params.title}` : params.title;` once and reuse.

**7. GitLab webhook signature mode treats X-Gitlab-Token as a shared-secret echo. This is correct for the legacy token mode, but GitLab 16+ supports an actual HMAC-signed X-Gitlab-Token when secret is enabled at a group level via Webhook secret token plus optional Verify SSL. Document the limitation in docs/GITHUB_INTEGRATION.md (currently silent on it).

8. webhook.ts stub defaults host to 0.0.0.0 — exposed to all interfaces with no TLS, no IP allowlist. AIG-636's webhook defaults to 127.0.0.1. Match that default to avoid accidentally publishing the unauthenticated bind in dev.

📝 Nits

  • dispatchWorkflow swallows errors with .catch((err) => log.error(...)); the rejection is logged but the underlying lastInvocation promise stays pending-then-fulfilled-with-undefined. Tests that await registered.lastInvocation() will see undefined even on failure — fine for now but worth a comment.
  • pr-body.ts uses _No plan produced_ and similar italics-as-placeholder; for GitLab MR descriptions this renders fine, just confirm with the team that the #/! issue-ref split (issueRef = issue.provider === 'github' ? \#${issue.number}` : `!${issue.number}`) is intended (GitLab uses #for issues,!for MRs — this PR labels the issue as!` which is wrong).
  • getIssue for GitHub silently treats a webhook-style issue (where pull_request field is present) the same as a real issue; consider rejecting PRs masquerading as issues to avoid weird workflows on repos/.../issues/<n> when n is actually a PR.
  • randomUUID().slice(0, 8) for branch suffixes is fine, but consider using the issue's updated_at or a hash of (provider, owner, repo, n) so a retried run resumes the same branch instead of creating an orphan.

🧪 Tests

The unit coverage on verifyHmacSignature, shouldDispatch, parseIssueUrl, applyLifecycleLabel, and the coordinator happy/sad paths is genuinely good. Missing: an idempotency test (fire two identical webhooks back-to-back, assert only one workflow runs), a 429/Retry-After test for the providers, and a self-hosted GHE base-URL test (the host.includes('api.github.com') check will misroute github.example.com).


Verdict: hold for changes. Items 1, 2, 3 are blockers; the rest are quick fixes. Happy to re-review once the stub is reconciled with #25's actual WebhookServer shape (or a coordination plan is documented).

aistack PM agent and others added 3 commits May 28, 2026 11:18
… framework

Resolves the merge conflict between AIG-637's local stub
src/transport/webhook.ts and the real WebhookServer landed by AIG-636
on main. The two had incompatible public APIs (addRoute vs. fixed
/v1/tasks; 4-arg vs 3-arg verifyHmacSignature).

Resolution
- src/transport/webhook.ts: taken from main verbatim (--theirs); now
  owns the daemon task-ingestion endpoint exclusively.
- src/transport/integration-router.ts (new): the multi-route HTTP
  router previously hiding inside the stub, renamed to IntegrationRouter
  with verifyIntegrationSignature. Same multi-format HMAC (github
  sha256 / gitlab raw token) that the SCM handlers need.
- src/transport/github-webhook.ts, src/transport/gitlab-webhook.ts:
  registerGitHubWebhook / registerGitLabWebhook now take an
  IntegrationRouter instead of WebhookServer. addRoute call site
  unchanged.
- src/cli/commands/index.ts, src/cli/index.ts: trivial merge — kept
  both createIngestCommand and main's createDaemonCommand /
  createWatchCommand / createRunCommand.
- tests/unit/github/webhook.test.ts: spins up IntegrationRouter
  instead of the (now incompatible) WebhookServer; imports
  verifyIntegrationSignature under the existing alias.
- docs/GITHUB_INTEGRATION.md: doc-only — mention IntegrationRouter in
  the E2E fixture section.

The GitHub/GitLab webhooks now listen on their own port (default 9091)
alongside the daemon's task-ingestion webhook (default 8787). No
behavior change to the issue->PR workflow, label transitions, or
HMAC verification semantics.
…e-to-pr-github

# Conflicts:
#	tests/unit/monitoring/health.test.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/commands/ingest.ts`:
- Line 23: When parsing the --max-iterations CLI option, validate that the value
is a positive integer before using it (don't rely on Number(...) blindly).
Replace the current conversion at the site where argv.max-iterations is read
(the code that calls Number(...) around line 57) with a parseInt-based parse and
then check Number.isInteger(value) && value > 0; if the check fails, print a
clear error and exit (or throw) so invalid inputs like "abc", "0", or "-1" are
rejected. Ensure the option declaration (--max-iterations) still has a sensible
default and propagate the validated integer (e.g., maxIterations) downstream.
- Around line 41-43: When calling ingestIssue, don't always pass
config.github.token; determine the provider from the URL first (use the existing
url value or helper that parses provider) and select the appropriate credentials
object (e.g., use config.github.token for GitHub URLs and config.gitlab.token
for GitLab URLs) before invoking ingestIssue; update the call site that
currently does ingestIssue(url, { credentials: { token: config.github.token } })
to compute the correct token based on provider and pass that token in the
credentials parameter.

In `@src/github/coordinator.ts`:
- Around line 216-219: The ReviewLoopCoordinator instance (loop) may not be
cleaned up if loop.start() throws; wrap the start/usage in a try/finally so
loop.cleanup() always runs: construct the ReviewLoopCoordinator
(ReviewLoopCoordinator, variable loop), then await loop.start() inside a try
block and call loop.cleanup() in the finally block, rethrow or return
appropriately so the original behavior (returning { reviews, approved } when
successful) is preserved while guaranteeing cleanup on errors.
- Around line 141-144: The dry-run branch (checking opts.dryRun) returns early
and skips the lifecycle completion that transitions issues out of
claimed/inProgress when skipLabels is false; move or modify the dry-run handling
so that before returning you invoke the same done-label/transition logic used in
the normal flow (the code that handles label transitions for issue and
skipLabels) — i.e., ensure the transition function that updates issue
labels/state (the same routine used later in this coordinator) is called for the
current issue when opts.dryRun is true, then log and return { status: 'success',
issue, branch, plan, reviews, prBody } as before.
- Around line 232-246: finishFailure currently hardcodes the lifecycle label
'blocked' when calling applyLifecycleLabel, which ignores any configured label
overrides; change the call in finishFailure to use the configured failure
lifecycle label from the issue/opts/config (e.g. read from IssueToPrOptions or
config.github.labels — locate where lifecycle labels are stored in your
codebase) and pass that value into applyLifecycleLabel instead of the literal
'blocked' so configured overrides are respected.

In `@src/github/providers.ts`:
- Around line 323-335: The two raw fetch calls inside the provider request flow
(the initial fetch and the retry) lack timeouts and can hang; wrap both calls
with an AbortController-based timeout: create an AbortController, set a timeout
(e.g., providerRequestTimeoutMs) that calls controller.abort(), attach
controller.signal to the fetch init (ensure you clone/merge the existing init
into a new object with signal), clear the timer after fetch resolves, and reuse
the same timeout logic for the retry fetch as well; update the code paths around
isRateLimited, retryDelayMs, and delay so both fetch(input, init) invocations
use the abort-capable init.

In `@src/transport/github-webhook.ts`:
- Around line 152-159: The current handler for the GitHub "issues.labeled" event
(involving GitHubPayload and the issues.labeled branch) triggers dispatch when
the issue contains "aistack-claimed" rather than when that label was the one
added by the event; change the logic to inspect the event payload's label
(payload.label.name or similar) and only call dispatch when the added label
equals "aistack-claimed" (keep existing checks for action === "labeled" and use
the same dispatch call/site), so unrelated label changes do not trigger a rerun.

In `@src/utils/config.ts`:
- Around line 51-56: The IssueLabelSetSchema currently allows empty strings
which can propagate to applyLifecycleLabel and cause attempts to write empty
labels; update IssueLabelSetSchema so each label field uses a non-empty string
validator (e.g., z.string().min(1) or z.string().nonempty()) combined with
.optional() to reject "" while still allowing undefined, then run/type-check to
ensure callers (like applyLifecycleLabel) handle missing labels instead of empty
values.

In `@tests/unit/github/coordinator.test.ts`:
- Around line 52-66: The fakeIssue helper currently returns a fixed IssueDetails
object and ignores any caller overrides; update the fakeIssue function signature
to accept an optional overrides parameter (e.g., overrides?:
Partial<IssueDetails>) and spread it into the returned object so callers like
fakeIssue({ provider: 'gitlab', ... }) actually override fields; ensure default
fields remain and typing uses Partial<IssueDetails> for the parameter to keep
TypeScript happy.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e270d35-a6e5-4abb-9d00-661492fd356b

📥 Commits

Reviewing files that changed from the base of the PR and between 99adfcd and 4e27fb8.

📒 Files selected for processing (22)
  • docs/GITHUB_INTEGRATION.md
  • src/cli/commands/index.ts
  • src/cli/commands/ingest.ts
  • src/cli/index.ts
  • src/github/coordinator.ts
  • src/github/dispatch-dedupe.ts
  • src/github/index.ts
  • src/github/ingest.ts
  • src/github/labels.ts
  • src/github/pr-body.ts
  • src/github/providers.ts
  • src/transport/github-webhook.ts
  • src/transport/gitlab-webhook.ts
  • src/transport/integration-router.ts
  • src/types.ts
  • src/utils/config.ts
  • tests/unit/consensus-service.test.ts
  • tests/unit/daemon/webhook.test.ts
  • tests/unit/github/coordinator.test.ts
  • tests/unit/github/ingest.test.ts
  • tests/unit/github/webhook.test.ts
  • tests/unit/monitoring/health.test.ts

Comment thread src/cli/commands/ingest.ts
Comment thread src/cli/commands/ingest.ts
Comment thread src/github/coordinator.ts
Comment thread src/github/coordinator.ts Outdated
Comment thread src/github/coordinator.ts
Comment thread src/github/providers.ts Outdated
Comment on lines +152 to +159
interface GitHubPayload {
action?: string;
assignee?: { login?: string };
issue?: {
html_url?: string;
number?: number;
labels?: Array<{ name?: string } | string>;
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Narrow issues.labeled dispatch to the label actually added.

At Line [188], dispatch is based on whether the issue currently contains aistack-claimed, not whether that was the label added by this event. This can trigger unnecessary reruns on unrelated label changes.

💡 Proposed fix
 interface GitHubPayload {
   action?: string;
   assignee?: { login?: string };
+  label?: { name?: string };
   issue?: {
     html_url?: string;
     number?: number;
     labels?: Array<{ name?: string } | string>;
   };
@@
   if (event === 'issues' && payload.action === 'labeled') {
-    const labels = payload.issue?.labels ?? [];
-    const claimed = labels.some((l) =>
-      typeof l === 'string' ? l === 'aistack-claimed' : l.name === 'aistack-claimed'
-    );
-    if (claimed) return { dispatch: true, reason: 'aistack-claimed label added' };
+    if (payload.label?.name === 'aistack-claimed') {
+      return { dispatch: true, reason: 'aistack-claimed label added' };
+    }
   }

Also applies to: 188-193

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/transport/github-webhook.ts` around lines 152 - 159, The current handler
for the GitHub "issues.labeled" event (involving GitHubPayload and the
issues.labeled branch) triggers dispatch when the issue contains
"aistack-claimed" rather than when that label was the one added by the event;
change the logic to inspect the event payload's label (payload.label.name or
similar) and only call dispatch when the added label equals "aistack-claimed"
(keep existing checks for action === "labeled" and use the same dispatch
call/site), so unrelated label changes do not trigger a rerun.

Comment thread src/utils/config.ts
Comment on lines +51 to +56
const IssueLabelSetSchema = z.object({
inProgress: z.string().optional(),
blocked: z.string().optional(),
done: z.string().optional(),
claimed: z.string().optional(),
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reject empty lifecycle label names in config.

At Line 51–56, empty strings are currently valid. That can propagate to applyLifecycleLabel and attempt to write an empty label value to GitHub/GitLab APIs.

💡 Proposed fix
-const IssueLabelSetSchema = z.object({
-  inProgress: z.string().optional(),
-  blocked: z.string().optional(),
-  done: z.string().optional(),
-  claimed: z.string().optional(),
-});
+const NonEmptyLabelSchema = z.string().trim().min(1);
+
+const IssueLabelSetSchema = z.object({
+  inProgress: NonEmptyLabelSchema.optional(),
+  blocked: NonEmptyLabelSchema.optional(),
+  done: NonEmptyLabelSchema.optional(),
+  claimed: NonEmptyLabelSchema.optional(),
+});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const IssueLabelSetSchema = z.object({
inProgress: z.string().optional(),
blocked: z.string().optional(),
done: z.string().optional(),
claimed: z.string().optional(),
});
const NonEmptyLabelSchema = z.string().trim().min(1);
const IssueLabelSetSchema = z.object({
inProgress: NonEmptyLabelSchema.optional(),
blocked: NonEmptyLabelSchema.optional(),
done: NonEmptyLabelSchema.optional(),
claimed: NonEmptyLabelSchema.optional(),
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/config.ts` around lines 51 - 56, The IssueLabelSetSchema currently
allows empty strings which can propagate to applyLifecycleLabel and cause
attempts to write empty labels; update IssueLabelSetSchema so each label field
uses a non-empty string validator (e.g., z.string().min(1) or
z.string().nonempty()) combined with .optional() to reject "" while still
allowing undefined, then run/type-check to ensure callers (like
applyLifecycleLabel) handle missing labels instead of empty values.

Comment thread tests/unit/github/coordinator.test.ts Outdated
Comment on lines +52 to +66
function fakeIssue(): IssueDetails {
return {
provider: 'github',
host: 'github.com',
owner: 'octocat',
repo: 'hello',
number: 42,
title: 'Add new feature',
body: 'Please add X',
labels: ['enhancement'],
assignees: [],
htmlUrl: 'https://github.com/octocat/hello/issues/42',
state: 'open',
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify mismatch: definition takes 0 args while a call passes 1 arg.
rg -n "function fakeIssue\\(" tests/unit/github/coordinator.test.ts -A6 -B2
rg -n "fakeIssue\\(" tests/unit/github/coordinator.test.ts -A2 -B2

Repository: blackms/aistack

Length of output: 1946


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="tests/unit/github/coordinator.test.ts"

echo "---- fakeIssue definition (lines ~45-110) ----"
nl -ba "$FILE" | sed -n '40,110p'

echo "---- call site with overrides (lines ~210-270) ----"
nl -ba "$FILE" | sed -n '210,270p'

Repository: blackms/aistack

Length of output: 167


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="tests/unit/github/coordinator.test.ts"

echo "---- fakeIssue definition (approx lines 40-110) ----"
cat -n "$FILE" | sed -n '40,110p'

echo "---- call site with overrides (approx lines 210-270) ----"
cat -n "$FILE" | sed -n '210,270p'

Repository: blackms/aistack

Length of output: 4658


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find test runner / TS transpilation config
rg -n "ts-jest|swc|babel|transform|isolatedModules|transpileOnly|jest\\.config|ts-node" package.json jest.config.* . 2>/dev/null || true

Repository: blackms/aistack

Length of output: 16027


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show relevant package.json scripts
cat -n package.json | sed -n '1,160p'

# Look for vitest/jest config
fd -t f "vitest.config.*|jest.config.*|tsconfig.*" -d 4 . || true
rg -n "vitest|vitest\\.config|jest|ts-jest|typecheck|isolatedModules|transpileOnly" package.json **/*vitest*.ts **/*vitest*.js **/*jest*.ts **/*jest*.js **/*tsconfig*.json 2>/dev/null || true

Repository: blackms/aistack

Length of output: 4279


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- tsconfig.json ----"
cat -n tsconfig.json | sed -n '1,220p'

echo "---- vitest.config.ts ----"
cat -n vitest.config.ts | sed -n '1,220p'

echo "---- vitest.integration.config.ts (if present) ----"
if [ -f vitest.integration.config.ts ]; then cat -n vitest.integration.config.ts | sed -n '1,220p'; fi

echo "---- vitest.e2e.config.ts (if present) ----"
if [ -f vitest.e2e.config.ts ]; then cat -n vitest.e2e.config.ts | sed -n '1,220p'; fi

Repository: blackms/aistack

Length of output: 3357


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="tests/unit/github/coordinator.test.ts"

rg -n "interface IssueDetails|type IssueDetails|IssueDetails" "$FILE" -S

Repository: blackms/aistack

Length of output: 117


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="tests/unit/github/coordinator.test.ts"

cat -n "$FILE" | sed -n '1,90p'

Repository: blackms/aistack

Length of output: 3777


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for test-specific tsconfig or typecheck hooks
fd -t f "tsconfig.*" . -d 4 || true
rg -n "typecheck|tsc --noEmit|noEmitOnError|tsconfig\\.test|vitest.*tsconfig|esbuild.*tsconfig|tsconfig" . || true

Repository: blackms/aistack

Length of output: 1175


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "vite-plugin-checker|typecheck.*vitest|vitest.*typecheck|ts-jest|tsc --noEmit|esbuild.*ts|transpileOnly" . || true

Repository: blackms/aistack

Length of output: 110


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# show the type/definition area for IssueDetails
FILE="src/github/providers.js"
if [ -f "$FILE" ]; then
  sed -n '1,220p' "$FILE" | cat -n
  echo "---- search IssueDetails in file ----"
  rg -n "IssueDetails" "$FILE" -n
else
  echo "File not found: $FILE"
  ls -la src/github
fi

Repository: blackms/aistack

Length of output: 720


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE_TS="src/github/providers.ts"
FILE_JS="src/github/providers.js"

ls -la src/github
echo "---- searching for IssueDetails ----"
rg -n "export (type|interface) IssueDetails|interface IssueDetails|type IssueDetails" src/github src -S

Repository: blackms/aistack

Length of output: 1013


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/github/providers.ts"
# print around IssueDetails
sed -n '1,120p' "$FILE" | cat -n
echo "---- around export interface IssueDetails ----"
rg -n "export interface IssueDetails" "$FILE"
START=1
END=120
sed -n "${START},${END}p" "$FILE" | cat -n

Repository: blackms/aistack

Length of output: 9333


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="src/github/pr-body.ts"
sed -n '1,240p' "$FILE" | cat -n
rg -n "htmlUrl|provider|issue|merge-request|!42|issue #" "$FILE"

Repository: blackms/aistack

Length of output: 3135


🏁 Script executed:

#!/bin/bash
set -euo pipefail

cat -n eslint.config.js | sed -n '1,220p'

Repository: blackms/aistack

Length of output: 876


Fix fakeIssue override handling to match its calls

tests/unit/github/coordinator.test.ts defines fakeIssue() with no parameters, but the GitLab-specific test calls fakeIssue({ provider: 'gitlab', ... }). Update fakeIssue to accept overrides and spread them so the overridden values are actually used.

💡 Proposed fix
-function fakeIssue(): IssueDetails {
+function fakeIssue(overrides: Partial<IssueDetails> = {}): IssueDetails {
   return {
     provider: 'github',
     host: 'github.com',
     owner: 'octocat',
     repo: 'hello',
     number: 42,
     title: 'Add new feature',
     body: 'Please add X',
     labels: ['enhancement'],
     assignees: [],
     htmlUrl: 'https://github.com/octocat/hello/issues/42',
     state: 'open',
+    ...overrides,
   };
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/github/coordinator.test.ts` around lines 52 - 66, The fakeIssue
helper currently returns a fixed IssueDetails object and ignores any caller
overrides; update the fakeIssue function signature to accept an optional
overrides parameter (e.g., overrides?: Partial<IssueDetails>) and spread it into
the returned object so callers like fakeIssue({ provider: 'gitlab', ... })
actually override fields; ensure default fields remain and typing uses
Partial<IssueDetails> for the parameter to keep TypeScript happy.

Copy link
Copy Markdown
Owner Author

@blackms blackms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head after the issue-to-PR fixes and main merge. CI is green (build, lint, typecheck, tests) and CodeRabbit is green. The two remaining GitHub review-thread objects are stale against current code: github-webhook now dispatches from payload.label, and config now rejects empty lifecycle labels via a trimmed non-empty schema with test coverage.

@blackms blackms merged commit f096996 into main May 29, 2026
6 checks passed
@blackms blackms deleted the pm-agent/aig-637-issue-to-pr-github branch May 29, 2026 12:49
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.

1 participant