feat(janitor): periodic reconciler — detect stuck issues/PRs/tasks and self-heal (#269) #273
No reviewers
Labels
No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
charles/claude-hooks!273
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/269-janitor"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #269
What
New
apps/server/src/janitor.tsmodule with 7 rules that run every 10 min (configurable). Default mode isdry-run— it logs what it would do without touching anything. Flip toauto-healonce you've reviewed a week of dry-run logs.Rules
closes_keyword_driftCloses #Nbut issue N still opendesign_approved_not_closed**Verdict**: APPROVEDbut issue still openlabel_dispatch_missarea:design/area:design-review/area:dashboardlabel with no dispatch in 2hdependent_unblockedzero_output_successstatus=successwith 0 turns or null coststale_idle_assignedready_to_mergeSafety rails
(rule, repo, target)triple acted on at most once per 6hjanitor.enabled: falseinconfig/agents.jsonmode: "auto-heal"is setjanitor.rules: ["only_this_rule"]disables all othersConfig (add to
config/agents.json)New endpoint
GET /janitor/history— last 100 actions from the ring buffer.SSE
Each action emits
{ type: "janitor_action", rule, action, repo, target, details, dry_run, ts }on the/eventsstream.Supporting changes
forgejo-api:patchIssue,listRepoLabels,addIssueLabels,removeIssueLabel,listClosedPullRequests; extendedPullRequestDetailwithmerged/merged_at; madeIssueListEntry.labels[].idoptionaltask-store:hasRecentDispatch,listZeroOutputSuccesseswebhook-config: parsejanitor:block intoWebhookConfig.janitor@claude-hooks/shared:JanitorAction,JanitorReport,JanitorHistoryResponsetypesTests
42 unit tests in
janitor.test.ts— one "fires" + one "no-op" + dry-run guard per rule, plus idempotency, ring buffer, and broadcast cross-cutting tests. All green, zero Forgejo HTTP calls.New module apps/server/src/janitor.ts with 7 rules: - closes_keyword_drift: close issues referenced by Closes/Fixes/Resolves in a recently merged PR that Forgejo didn't auto-close - design_approved_not_closed: close issues where design-reviewer posted '**Verdict**: APPROVED' but the skill missed the close step - label_dispatch_miss: bounce area:design / area:design-review / area:dashboard routing labels that have no dispatch in the last 2h - dependent_unblocked: re-fire assignee bounce for issues whose blockers are all closed but the propagator never ran - zero_output_success: flag success tasks with 0 turns / null cost - stale_idle_assigned: comment + bounce assignee on issues idle >30 min - ready_to_merge: flag PRs that are mergeable + CI green + APPROVED Config block janitor: { enabled, interval_ms, mode, rules } in config/agents.json. Default mode is dry-run. GET /janitor/history returns last 100 actions. Each action emits a janitor_action SSE envelope. Per-rule per-target 6h idempotency guard. Supporting changes: - forgejo-api: patchIssue, listRepoLabels, addIssueLabels, removeIssueLabel, listClosedPullRequests, extend PullRequestDetail with merged/merged_at, make IssueListEntry labels id optional - task-store: hasRecentDispatch, listZeroOutputSuccesses - webhook-config: parse janitor: block - shared: JanitorAction, JanitorReport, JanitorHistoryResponse types 42 unit tests, all green.The s11 refactor moved dispatchByType / getWorker / broadcastSSE out of main.ts into registry.ts / sse.ts. Several test files updated the module specifier in their mock.module() calls, but three related leaks made 20 sibling-file tests fail in the full bun test run: 1. janitor.test.ts used mock.module("./forgejo-api") and ("./task-analytics") to stub its deps. Bun's mock.module is process-global — those stubs persisted through every later test file and broke deps.test.ts, webhook-handlers.test.ts pivot tests, propagateDependencyClosure, etc. Fix: add `impl` test seam to janitor.ts (same pattern as cleanup.ts) so the test mutates impl.* instead of touching module registry. 2. webhook-post-merge.test.ts was still mocking "./main" for dispatchByType (refactor missed this one) and also mocked ./main's broadcastSSE, both of which now live in ./registry / ./sse. Fix: point mock.module at the correct modules; drop the unused getWorker override (not exercised by handlePostMergeRebase). 3. webhook-assign-dedup / webhook-ci / force-merge.integration all mocked ./registry with both dispatchByType AND getWorker. getWorker is only actually exercised by webhook-assign-dedup's assign-dedup tests; the other two carried stale overrides that corrupted getWorker for every later test file (main.test.ts POST /cancel + POST /reset). Fix: drop the getWorker override where unused; in webhook-assign-dedup, keep the stub but fall back to the real function for names the test doesn't track, so main.test.ts's registered workers still resolve. Plus: fix the stale mock.module("./main") → ("./registry") in webhook-handlers.test.ts and refresh an outdated comment in main.test.ts. Server test suite now: 990 pass / 4 fail — and the 4 remaining failures (session JSONL pruning + foreman session CRUD) are pre-existing on main, out of scope for this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Port additions: - ForgeDirEntry shape for listDir (name, path, type, size, sha) - htmlUrl field on ForgeIssue (needed by createIssue → breakdown summary link) - readFile returns { content, sha } so callers can chain writeFile - writeFile takes optional sha for update-vs-create discrimination foreman.ts migration: - /foreman/repos, /foreman/files, /foreman/specs/:name save, /foreman/breakdown-preview remote fetch, /foreman/create-issues all go through ForgejoAdapter - spec save now surfaces a 502 on adapter failure instead of 500ing out of createOrUpdateRepoFile's throw (adapter swallows to bool) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>e31100072697e9db3e8a