refactor: re-architect around GitHub App with structured agent pipeline#100
Merged
Conversation
- Remove email webhook and legacy GitHub webhook handlers; GitHub App is now the sole webhook source and acting identity - Add installation token lifecycle: auto-discover default installation, set GH_TOKEN, refresh every 45 min, configure git identity from App - Migrate storage to Drizzle ORM with proper migration system (schema_version table, numbered migrations, auto-repair) - Add compound cursor pagination to prevent row skipping - Add delivery_id index for future persistent dedup - Enable PRAGMA foreign_keys for cascading deletes - Restructure resolve-issue into a phased pipeline: context gathering, bug verification, planning (Opus), implementation (Sonnet sub-agents), verification loop, cleanup and PR - Add model routing: Opus orchestrates, Sonnet sub-agents handle codebase analysis, bug reproduction, and code implementation - Add comment-and-wait flow for unreproducible bugs - Dynamic CI cron timing based on repo check count (1-20 min) - Add 10-minute quiet period to auto-merge preconditions - Adaptive batch window: start at 2s, extend to batchWindowMs - Worktree health check before reuse in repo-setup - Fix broken gh pr list search in resolve-issue - Add exact attempt-counting command in fix-ci - Add 3-round cap to deslop+review loop - Inject bot_login into prompt template context - Remove dead dependencies: @ai-sdk/anthropic, ai, zod
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
…encies Critical fixes: - bootstrap: resolve bot identity BEFORE creating handlers so ignore_authors self-loop guard actually works (was normalized with null botLogin, making $BOT_LOGIN substitution a no-op) - pipeline: remove leaked abort timer in stale-session retry that could fire prematurely before semaphore acquisition - migrations: use actual column type from EXPECTED_TABLES in repair instead of hardcoded TEXT (would corrupt INTEGER columns) - github-api: fix this-binding bug in fetchEntity that breaks when the fetcher is destructured; extract shared fetchByType helper Medium fixes: - bootstrap: add --global flag to git config calls (was writing to local repo config of whatever CWD happened to be) - auto-merge: use timeline API instead of issues events endpoint for ready_for_review (PR events don't appear in issue events) - fix-ci: filter attempt count by author to avoid miscounting human comments - pr: guard against empty checks array being misread as CI passed when CI hasn't started yet - respond-to-comment, review-pr: use $ME identity variable instead of redundant gh api user calls - mark-pr-ready: fix wording that contradicted auto-merge flow DRY consolidation: - github-api: extract shared fetchByType helper, eliminating duplicate try/catch/Sentry logging in fetchPR and fetchIssue - entity: consolidate 3 identical PR event handlers into one using a Set; consolidate check_suite/workflow_run into a shared lookup Documentation: - storage: document raw SQL vs Drizzle split and timestamp strategy - schema: document that Drizzle enum is TypeScript-only, SQL CHECK comes from migrations
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+82
to
+92
| // Normalize triggers with the resolved bot login so ignore_authors | ||
| // substitution works. This must happen after identity resolution. | ||
| const triggers = (cfg.triggers ?? []).map((t) => normalizeTrigger(t, botLogin)) | ||
| if (triggers.length === 0) { | ||
| logger.info("no triggers configured -- listener disabled", { path: configPath() }) | ||
| return null | ||
| } | ||
|
|
||
| const githubTriggerCount = triggers.filter((t) => t.source === "github_webhook").length | ||
| const emailTriggerCount = triggers.filter((t) => t.source === "email").length | ||
| const githubAppTriggerCount = triggers.filter((t) => t.source === "github_app").length | ||
|
|
||
| if (githubTriggerCount > 0 && !secret) { | ||
| logger.warn("no GitHub HMAC secret configured -- /webhooks/github will reject with 503") | ||
| } | ||
| if (emailTriggerCount > 0 && !emailSecret) { | ||
| logger.warn("no email HMAC secret configured -- /webhooks/email will reject with 503") | ||
| } | ||
|
|
||
| const githubApp = cfg.github_app ?? resolveGithubAppFromEnv() | ||
| if (githubAppTriggerCount > 0 && !githubApp) { | ||
| logger.warn( | ||
| "github_app triggers configured but no GitHub App credentials found -- /webhooks/github-app will not be registered", | ||
| ) | ||
| } | ||
| // Re-create handlers with properly normalized triggers (the initial | ||
| // createHandlers call above was only to get the auth object). | ||
| const { handlers: normalizedHandlers } = createHandlers({ triggers, githubApp }) |
There was a problem hiding this comment.
Bug: createHandlers is called twice, creating two GitHubAppAuth instances with separate token caches. This leads to redundant API calls and inefficient resource usage.
Severity: LOW
Suggested Fix
Refactor the bootstrap process to call createHandlers only once. The single GitHubAppAuth instance should be shared between the token refresh mechanism and the webhook handlers to unify token caching and eliminate redundant API calls. The unused auth property from the BootstrapResult should also be removed.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/opentower/src/bootstrap.ts#L62-L92
Potential issue: The `bootstrap.ts` file calls `createHandlers` twice, once on line 62
and again on line 92. Each call instantiates a new `GitHubAppAuth` object with its own
token cache. The first instance is used for a token refresh loop, while the second is
used by webhook handlers. This duplication leads to redundant API calls for the same
installation tokens, as each instance maintains a separate cache. Furthermore, the
`auth` object returned from the first call is never used, constituting dead code.
Did we get this right? 👍 / 👎 to inform future reviews.
Comment on lines
+54
to
+61
| async function fetchEntity(repo: string, number: number) { | ||
| // Try PR first — issues API also returns PRs but with less detail | ||
| const pr = await fetchPR(repo, number) | ||
| if (pr) return { kind: "pull_request" as const, body: pr.body } | ||
| const issue = await fetchIssue(repo, number) | ||
| if (issue) return { kind: "issue" as const, body: issue.body } | ||
| return null | ||
| } |
There was a problem hiding this comment.
Bug: The fetchEntity function, currently dead code, incorrectly handles API failures by falling back to fetchIssue, which could misclassify a pull request as an issue on transient errors.
Severity: LOW
Suggested Fix
Modify fetchEntity to differentiate between a '404 Not Found' error and other transient API failures. The fallback to fetchIssue should only occur on a 404 from the pull request endpoint. For other errors, the function should retry or propagate the error rather than misclassifying the entity. Alternatively, remove the dead code.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/opentower/src/github-api.ts#L54-L61
Potential issue: The `fetchEntity` function in `github-api.ts` contains a logical flaw,
although it is currently dead code. If called, it first attempts to fetch a pull
request. If this API call fails for any reason (including transient network errors, not
just a 404), it falls back to fetching an issue. Because GitHub's issue endpoint can
also return pull request data, a transient failure in the pull request API would cause a
pull request to be misclassified as an issue. This creates a latent hazard should this
function be used in the future.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-architects outpost around GitHub App as the sole authentication and webhook source, and restructures Jared's issue-resolution pipeline into a phased, model-routed workflow.
What changed
Architecture: GitHub App only
GH_TOKEN, refresh every 45 min, configure git identity from App bot accountStorage: Drizzle ORM + migration system
bun:sqliteto Drizzle ORM with typed schemaschema_versiontable, numbered migrations, schema verification, and auto-repairPRAGMA foreign_keys = ONfor cascading deletesdelivery_id, compound(created_at, id), compound(updated_at, entity_key)Agent pipeline: structured phases with model routing
resolve-issuerewritten as a 5-phase, 12-step pipeline:Pipeline optimizations
batchWindowMsas events arrivegh api usercall per session)Removed
@ai-sdk/anthropic,ai,zoddependencies (were only used by email entity resolver)handlers/email-webhook.ts,handlers/github-webhook.ts,entity-resolver.tsGH_TOKEN,GITHUB_WEBHOOK_SECRET,EMAIL_WEBHOOK_SECRETenv vars (replaced by GitHub App credentials)Testing
tsc --noEmitpassesbiome checkpasses