Harden review flows, add CI/verifier guardrails, and backfill regression coverage#100
Open
Harden review flows, add CI/verifier guardrails, and backfill regression coverage#100
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR backfills and reconciles milestone artifacts across M051–M060, hardens CI/verifier coverage, and updates GSD planning/validation documentation to better reflect current repo truth.
Changes:
- Added/updated many
.gsd/milestone artifacts (roadmaps, validations, anchors, UAT, task summaries/verifications) for historical completeness and truthfulness. - Hardened GitHub Actions workflows by pinning Bun and adding additional lint/verify/orphan-check gates.
- Documented/clarified ownership boundaries (e.g., M060 vs M027) and cross-slice evidence mapping for milestone validation.
Reviewed changes
Copilot reviewed 233 out of 717 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | Pins Bun version and adds lint/verifier/orphan-test checks; splits tests into two runs. |
| .github/workflows/nightly-issue-sync.yml | Pins Bun version for nightly issue sync. |
| .github/workflows/nightly-reaction-sync.yml | Pins Bun version for nightly reaction sync. |
| .gsd/milestones/M053/slices/S01/S01-PLAN.md | Adds slice plan; contains a duplicated phrase in “Proof Level”. |
| .gsd/milestones/M052/slices/S01/S01-UAT.md | Adds UAT doc; currently contains literal \n escapes instead of real newlines. |
| .gsd/milestones/M052/slices/S02/S02-UAT.md | Adds UAT doc; currently contains literal \n escapes instead of real newlines. |
| .gsd/milestones/M052/slices/S03/S03-UAT.md | Adds UAT doc; currently contains literal \n escapes instead of real newlines. |
| .gsd/milestones/M021/M021-ROADMAP.md | Repairs roadmap metadata and adds boundary map; introduces duplicated “After this:” prefixes. |
| .gsd/milestones/M021/slices/S01/S01-ASSESSMENT.md | Backfills assessment; includes a cross-slice claim that conflicts with S05’s “foundation continuity” constraint. |
| .gsd/milestones/M026/M026-ROADMAP.md | Rewrites roadmap header/vision; currently drops prior detailed vision/success-criteria content and leaves Vision empty. |
| .gsd/milestones/M025/M025-ROADMAP.md | Rewrites roadmap header/vision; currently drops prior vision content and leaves Vision empty. |
| .gsd/milestones/M024/M024-ROADMAP.md | Rewrites roadmap header/vision; currently drops prior vision content and leaves Vision empty. |
| .gsd/milestones/M023/M023-ROADMAP.md | Rewrites roadmap header/vision; currently drops prior vision content and leaves Vision empty. |
| .gsd/milestones/M022/M022-ROADMAP.md | Rewrites roadmap header/vision; currently drops prior vision content and leaves Vision empty. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## Proof Level | ||
|
|
||
| - This slice proves: This slice proves: contract |
Member
Author
There was a problem hiding this comment.
Outdated after the PR trim. This .gsd milestone artifact was removed from PR #100 when the branch was rebuilt onto origin/main, so there is no corresponding file change left to address in this PR.
| **Milestone:** M052 | ||
| **Written:** 2026-04-19T04:33:23.066Z | ||
|
|
||
| # UAT — S03 Operator setup, proof harness, and rollout docs\n\n## Scenario 1 — Final proof command\n- Run `bun run verify:m052`.\n- Expected: composed milestone proof reports `m052_ok` and both nested slice proofs pass.\n\n## Scenario 2 — Accepted smoke flow\n- Follow `docs/smoke/slack-webhook-relay.md` and POST the accepted curl payload to `/webhooks/slack/relay/buildkite`.\n- Expected: HTTP 202 with verdict `accept` and one Slack post.\n\n## Scenario 3 — Suppressed smoke flow\n- Follow the suppressed curl example from the smoke doc.\n- Expected: HTTP 202 with verdict `suppress`, reason `text_excluded_substring`, and no Slack post.\n\n## Scenario 4 — Failed-delivery smoke flow\n- Follow the failed-delivery curl example in a non-production or intentionally broken environment.\n- Expected: HTTP 502 with reason `delivery_failed`.\n\n## Documentation checks\n- `docs/runbooks/slack-webhook-relay.md` describes the route, config, payload contract, and troubleshooting path.\n- `docs/deployment.md` includes the relay endpoint, optional env var, and `verify:m052` proof surface.\n- `docs/README.md` lists the relay runbook and smoke document.\n |
Member
Author
There was a problem hiding this comment.
Outdated after the PR trim. This .gsd milestone artifact is no longer part of PR #100 after the branch rebuild onto origin/main.
Comment on lines
+15
to
+27
| > After this: After this: Create the PostgreSQL migration for the issue corpus (issues + issue_comments tables) and the TypeScript type definitions. | ||
|
|
||
| - [x] **S02: Issue Mcp Tools** `risk:medium` `depends:[S01]` | ||
| > After this: Implement the `github_issue_label` MCP tool using TDD. | ||
| > After this: After this: Implement the `github_issue_label` MCP tool using TDD. | ||
|
|
||
| - [x] **S03: Triage Agent Wiring** `risk:medium` `depends:[S02]` | ||
| > After this: Implement the issue template parser using TDD. | ||
| > After this: After this: Implement the issue template parser using TDD. | ||
|
|
||
| - [x] **S04: S04** `risk:low` `depends:[]` | ||
| > After this: After this: After this: M021 has requirement-level traceability, explicit milestone success criteria, and per-slice assessment artifacts required for validation. | ||
|
|
||
| - [ ] **S05: S05** `risk:low` `depends:[]` | ||
| > After this: After this: After this: M021 has explicit producer/consumer evidence showing how S01, S02, and S03 compose, allowing MV03 to pass truthfully. |
Member
Author
There was a problem hiding this comment.
Outdated after the PR trim. This roadmap artifact was removed from the PR when I rebuilt the branch from origin/main, so there is no remaining diff here.
|
|
||
| ## Assessment | ||
|
|
||
| S01 already delivered the issue corpus schema, IssueStore implementation, migrations, and application wiring recorded in its shipped summary. This reassessment closes a missing evidence artifact only: the new S01 assessment should state that later validation work is traceability closure, not missing implementation, and that S01 provides the persisted issue corpus substrate consumed by S02 mutation tools and downstream milestone validation. |
Member
Author
There was a problem hiding this comment.
Outdated after the PR trim. This .gsd assessment file is no longer in the current PR diff after the branch rebuild.
| # two shorter invocations to avoid cross-file schema interference and runner crashes. | ||
| - run: bun test --max-concurrency=2 scripts src/contributor src/enforcement src/execution src/feedback src/handlers src/jobs src/lib src/review-audit src/review-graph src/routes src/slack src/structural-impact src/telemetry src/triage | ||
| # The first run covers scripts plus non-knowledge src tests; src/knowledge stays isolated. | ||
| - run: bun test --max-concurrency=2 scripts src |
Member
Author
There was a problem hiding this comment.
Fixed. The first CI pass now explicitly excludes src/knowledge, so the isolated knowledge pass does not get duplicated.
| with: | ||
| bun-version: latest | ||
| bun-version: 1.3.8 | ||
| - run: bun install |
Member
Author
There was a problem hiding this comment.
Fixed. CI now uses bun install --frozen-lockfile to match the reproducible install pattern used elsewhere.
Comment on lines
3
to
4
| **Vision:** | ||
|
|
Member
Author
There was a problem hiding this comment.
Outdated after the PR trim. This .gsd roadmap file was removed from the PR when I rebuilt the branch from origin/main.
keithah
changed the title
kodiai responseDecision: APPROVE Evidence:
Review Details
Files not fully reviewed (sorted by risk score)
|
![kodiai[bot]](https://avatars.githubusercontent.com/in/2832814?s=60&v=4){kind=small}
keithah
force-pushed
the
m051-manual-rereview-truthfulness
branch
from
961417b to
3719b9d
Compare
keithah
force-pushed
the
m051-manual-rereview-truthfulness
branch
from
3719b9d to
7d1f20a
Compare
keithah
changed the title
Member
Author
|
@kodai review |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens several runtime review paths and adds repo-owned CI/verifier guardrails, alongside broad regression/backfill coverage (knowledge-layer modules, webhook/handler behavior, and rollback/migration proofs).
Changes:
- Adds extensive new Bun test coverage for knowledge-layer scoring/fetching logic, cluster scheduling, and job utilities (fork/gist publishing).
- Introduces/verifies new repo-owned verifier scripts and wires them into CI, plus adds an ESLint “no-console” gate with scoped exceptions.
- Backfills missing down-migration files and strengthens rollback-oriented verification contracts.
Reviewed changes
Copilot reviewed 102 out of 118 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/knowledge/wiki-popularity-config.test.ts | Adds unit coverage for popularity scoring constants and composite score math. |
| src/knowledge/wiki-linkshere-fetcher.test.ts | Adds batching/pagination/rate-limit and failure-mode tests for linkshere counts fetcher. |
| src/knowledge/wiki-fetch.test.ts | Adds coverage for wiki URL building and header wrapping logic. |
| src/knowledge/test-coverage-exemptions.ts | Introduces an explicit runtime target and type-only exemption manifest. |
| src/knowledge/store.test.ts | Switches DB-backed tests to require TEST_DATABASE_URL and skip when absent. |
| src/knowledge/issue-retrieval.test.ts | Adds direct unit tests for issue embedding search mapping/truncation behavior. |
| src/knowledge/isolation.test.ts | Adds behavioral tests for isolation-layer retrieval and provenance. |
| src/knowledge/cluster-scheduler.ts | Makes scheduler start() idempotent to prevent duplicate timers. |
| src/knowledge/cluster-scheduler.test.ts | Adds tests for fail-open behavior, timer scheduling, and stop semantics. |
| src/jobs/gist-publisher.test.ts | Adds tests for gist publishing availability and request shaping/logging. |
| src/jobs/fork-manager.ts | Adds a warning log before raising a merge-conflict sync error. |
| src/jobs/fork-manager.test.ts | Adds regression tests for fork caching, provisioning, sync conflicts, and best-effort deletion. |
| src/index.ts | Removes Slack relay webhook route wiring. |
| src/handlers/mention.ts | Adds outgoing secret scanning before publishing mention replies. |
| src/execution/types.ts | Removes gitDiffInspectionAvailable from ExecutionContext. |
| src/execution/review-prompt.ts | Removes degraded diff guidance and always prints git diff/log instructions. |
| src/execution/review-prompt.test.ts | Removes tests for degraded diff guidance. |
| src/execution/executor.ts | Removes tool gating based on gitDiffInspectionAvailable. |
| src/db/migrations/036-suggestion-cluster-models.down.sql | Backfills down migration for suggestion cluster models table. |
| src/db/migrations/035-generated-rules.down.sql | Backfills down migration for generated rules table. |
| src/db/migrations/034-review-graph.down.sql | Backfills down migrations for review graph tables. |
| src/db/migrations/033-canonical-code-corpus.down.sql | Backfills down migrations for canonical code corpus tables. |
| src/db/migrations/030-reserved.sql | Adds a schema-neutral reserved migration slot for 030. |
| src/db/migrations/030-reserved.down.sql | Adds a schema-neutral rollback for reserved slot 030. |
| src/db/migrations/026-guardrail-audit.down.sql | Backfills down migration for guardrail audit table. |
| src/db/migrations/025-wiki-style-cache.down.sql | Backfills down migration for wiki style cache table. |
| src/db/migrations/016-issue-triage-state.down.sql | Backfills down migration for issue triage state table. |
| src/db/migrations/013-review-clusters.down.sql | Backfills down migration for review cluster tables. |
| src/db/migrations/012-wiki-staleness-run-state.down.sql | Backfills down migration for wiki staleness run state table. |
| src/db/migrate.ts | Adds operator-facing rationale comment for direct console output. |
| src/contributor/xbmc-fixture-snapshot.test.ts | Removes dynamic import helper in favor of direct import(). |
| src/contributor/xbmc-fixture-refresh.ts | Removes now-unused slack relay config field from fixture config. |
| src/contributor/xbmc-fixture-refresh.test.ts | Removes dynamic import helper in favor of direct import(). |
| src/contributor/profile-store.test.ts | Requires TEST_DATABASE_URL and skips DB tests when absent. |
| src/contributor/calibration-evaluator.test.ts | Removes dynamic import helper in favor of direct import(). |
| src/contributor/calibration-change-contract.test.ts | Removes dynamic import helper in favor of direct import(). |
| src/config.ts | Removes Slack webhook relay config parsing/surface from app config. |
| src/config.test.ts | Removes Slack webhook relay config tests and env setup/teardown. |
| scripts/verify-m060-s02.test.ts | Adds regression tests for the M060 S02 ownership-boundary verifier. |
| scripts/verify-m059-s02.test.ts | Adds regression tests for the M059 S02 orphan-audit verifier. |
| scripts/verify-m058-s03.test.ts | Adds regression tests for CI lint gate wiring and decision markers. |
| scripts/verify-m058-s02.test.ts | Adds regression tests for pinned Bun version contract across package/workflows/docs. |
| scripts/verify-m058-s01.ts | Adds a verifier for CI broadened test coverage + split rationale/ordering. |
| scripts/verify-m058-s01.test.ts | Adds tests for M058 S01 verifier harness and stable status codes. |
| scripts/verify-m057-s04.ts | Adds an importable verifier runner for a bundled command set. |
| scripts/verify-m057-s04.test.ts | Adds tests asserting m057 s04 commands and package wiring. |
| scripts/verify-m057-s03.ts | Adds a command-bundling verifier script for selected runtime tests. |
| scripts/verify-m057-s02.ts | Adds a command-bundling verifier script for CI-failure related tests. |
| scripts/verify-m056-s02.test.ts | Adds regression tests for rollback completeness + slot 030 contract. |
| scripts/verify-m056-s01.test.ts | Adds regression tests for rollback completeness contract. |
| scripts/verify-m055-s02.test.ts | Adds regression tests for docs truth contract surfaces. |
| scripts/verify-m055-s01.ts | Adds a docs-truth verifier for README/CHANGELOG/package wiring. |
| scripts/verify-m055-s01.test.ts | Adds regression tests for the M055 S01 docs-truth verifier. |
| scripts/verify-m054-s03.test.ts | Adds regression tests for M054 S03 recent-history verifier behavior. |
| scripts/verify-m054-s02.ts | Adds a verifier for historical milestone folder inventory + wiring. |
| scripts/verify-m054-s02.test.ts | Adds regression tests for historical folder verifier harness. |
| scripts/verify-m054-s01.test.ts | Adds regression tests for queue truth contract harness. |
| scripts/verify-m053.ts | Adds verifier enforcing “no new Function() in src/” and decision record presence. |
| scripts/verify-m053.test.ts | Adds regression tests for M053 invariant verifier harness. |
| scripts/verify-m049-s02.ts | Removes now-unused slack relay config field from verifier config. |
| scripts/verify-m044-s01.ts | Removes now-unused slack relay config field from verifier config. |
| scripts/verify-m029-s04.ts | Tightens auto-discovery rules for injected sql/octokit vs env-based discovery. |
| scripts/verify-m027-s03.test.ts | Updates expected embedding model strings in test fixtures. |
| scripts/sync-triage-reactions.ts | Removes now-unused slack relay config field from script config. |
| scripts/publish-wiki-updates.ts | Removes now-unused slack relay config field from script config. |
| scripts/cleanup-wiki-issue.ts | Removes now-unused slack relay config field from script config. |
| scripts/check-orphaned-tests.test.ts | Adds regression tests for orphaned-test checker harness and wiring. |
| scripts/check-migrations-have-downs.test.ts | Adds regression tests for migration pairing checker harness and wiring. |
| scripts/backfill-review-comments.ts | Removes now-unused slack relay config field from script config. |
| scripts/backfill-issues.ts | Removes now-unused slack relay config field from script config. |
| package.json | Pins Bun version, adds ESLint + TS parser, and wires many new verify/check scripts. |
| eslint.config.mjs | Adds flat ESLint config enforcing no-console with scoped exceptions. |
| bunfig.toml | Expands test root to repo root and ignores tmp/ for test discovery. |
| .github/workflows/nightly-reaction-sync.yml | Pins workflow Bun version to match package contract. |
| .github/workflows/nightly-issue-sync.yml | Pins workflow Bun version to match package contract. |
| .github/workflows/ci.yml | Pins Bun version, adds lint/verifier gates, and broadens bun test coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const params = new URLSearchParams({ action: "parse", page: "Foo Bar" }); | ||
|
|
||
| expect(buildWikiApiUrl("https://kodi.wiki?foo=bar", params)).toBe( | ||
| "https://kodi.wiki?foo=bar/api.php?action=parse&page=Foo+Bar", |
Member
Author
There was a problem hiding this comment.
Fixed. buildWikiApiUrl() now appends /api.php to the pathname and preserves/merges existing query params; the test expectation was updated to match the valid URL shape.
Comment on lines
+15
to
+32
| for (const command of TEST_COMMANDS) { | ||
| const [cmd, ...args] = command; | ||
| const formatted = formatCommand(command); | ||
| process.stdout.write(`→ ${formatted}\n`); | ||
|
|
||
| const result = Bun.spawnSync({ | ||
| cmd: [cmd, ...args], | ||
| stdout: "inherit", | ||
| stderr: "inherit", | ||
| }); | ||
|
|
||
| if (result.exitCode !== 0) { | ||
| process.stderr.write(`${COMMAND_NAME} failed: ${formatted}\n`); | ||
| process.exit(result.exitCode); | ||
| } | ||
| } | ||
|
|
||
| process.stdout.write(`${COMMAND_NAME} passed\n`); |
Member
Author
There was a problem hiding this comment.
Fixed. scripts/verify-m057-s03.ts now exports runVerifyM057S03() and uses an import.meta.main guard so it can be imported safely in tests and other tooling.
| if (scanResult.blocked) { | ||
| logger.warn( | ||
| { matchedPattern: scanResult.matchedPattern }, | ||
| "Outgoing secret scan blocked mention reply publish", |
Member
Author
There was a problem hiding this comment.
Fixed. I updated the log message to match the real behavior: the original mention reply body is blocked by the outgoing secret scan, then a placeholder is published instead.
kodiai responseI ran out of steps analyzing this and wasn't able to post a complete response. This can happen on PRs with large or complex diffs. To get a response:
|
| setReviewWorkPhase("publish"); | ||
| await postOrUpdateErrorComment(octokit, { | ||
| // Auto-approval: only when autoApprove is enabled AND execution succeeded AND | ||
| // the model produced zero GitHub-visible output (no summary comment, no inline comments). |
There was a problem hiding this comment.
[MAJOR] Missing failure handling causes silent failures
The failure handling block for result.conclusion === "failure" was removed entirely. When a review execution completes but fails (e.g., runs out of turns with stopReason: "max_turns" or failureSubtype: "error_max_turns"), no error comment is posted to the PR.
Impact: Users receive no feedback when reviews fail silently. The old code posted a helpful error comment explaining:
- "Kodiai ran out of steps while reviewing this PR" for turn budget exhaustion
- "Kodiai completed the review run but could not publish review output" for other failure modes
Consequence: Silent failures create a poor user experience and make debugging review issues harder.
Suggested change
| // the model produced zero GitHub-visible output (no summary comment, no inline comments). | |
| // Auto-approval: only when autoApprove is enabled AND execution succeeded AND | |
| // the model produced zero GitHub-visible output (no summary comment, no inline comments). | |
| if (config.review.autoApprove && result.conclusion === "success") { |
This should restore failure handling before the auto-approval block:
if (result.conclusion === "failure" && !(result.published ?? false)) {
const exhaustedTurnBudget =
result.stopReason === "max_turns" ||
result.failureSubtype === "error_max_turns";
const failureBody = exhaustedTurnBudget
? [
"> **Kodiai ran out of steps while reviewing this PR**",
"",
"_The review run ended before it could publish comments or an approval._",
"",
...(result.stopReason ? [`Stop reason: ${result.stopReason}`] : []),
...(result.failureSubtype ? [`Failure subtype: ${result.failureSubtype}`] : []),
"",
"Try requesting another review after narrowing the scope or improving the available review context.",
].join("\n")
: [
"> **Kodiai completed the review run but could not publish review output**",
"",
...(result.stopReason ? [`Stop reason: ${result.stopReason}`] : []),
...(result.failureSubtype ? [`Failure subtype: ${result.failureSubtype}`] : []),
...(result.errorMessage ? [result.errorMessage] : []),
"",
"Try requesting another review if you want a fresh attempt.",
].join("\n");
const octokit = await githubApp.getInstallationOctokit(event.installationId);
if (canPublishVisibleOutput("failure fallback comment")) {
setReviewWorkPhase("publish");
await postOrUpdateErrorComment(octokit, {
owner: apiOwner,
repo: apiRepo,
issueNumber: pr.number,
}, sanitizeOutgoingMentions(failureBody, [githubApp.getAppSlug(), "claude"]), logger);
}
}
// Auto-approval: only when autoApprove is enabled AND execution succeeded AND
// the model produced zero GitHub-visible output (no summary comment, no inline comments).
if (config.review.autoApprove && result.conclusion === "success") {
Member
Author
There was a problem hiding this comment.
Fixed. The review handler now posts a visible fallback PR comment when execution fails without publishing output, including the turn-limit path, so failures are no longer silent.
| await upsertReviewDetailsComment({ | ||
| octokit: extractionOctokit, | ||
| owner: apiOwner, | ||
| repo: apiRepo, |
There was a problem hiding this comment.
[MEDIUM] Review Details publication timing not finalized
The refactored publication flow no longer updates the Review Details comment with the final publication phase timing.
What changed:
- Old code: built review details → published → set publication phase to "completed" → rebuilt details with final timing → updated comment
- New code: builds review details once → publishes → never updates with final timing
Impact: The Review Details will show publication phase as "degraded" (with partial duration) or "unavailable" instead of "completed" with the actual final duration. This affects telemetry visibility for operators and makes it harder to diagnose publication performance issues.
Suggestion: After both publication paths (append to summary / standalone comment), capture the final publication timing and update the comment:
if (canPublishVisibleOutput(...)) {
setReviewWorkPhase("publish");
await upsertReviewDetailsComment(...);
// Finalize publication timing
if (publicationPhaseStartedAt !== undefined) {
reviewPhaseTimings.set(
"publication",
createReviewPhaseTiming({
name: "publication",
status: "completed",
durationMs: Math.max(0, Date.now() - publicationPhaseStartedAt),
}),
);
// Update with finalized timing
const finalizedDetailsBody = buildReviewDetailsBody();
await upsertReviewDetailsComment({
octokit: extractionOctokit,
owner: apiOwner,
repo: apiRepo,
prNumber: pr.number,
reviewOutputKey,
body: finalizedDetailsBody,
botHandles: [githubApp.getAppSlug(), "claude"],
recheckCanPublish: () => canPublishVisibleOutput("finalized Review Details timing update"),
});
}
}
Member
Author
There was a problem hiding this comment.
Fixed. I adjusted the Review Details publication path in this area and added regression coverage around the publication/fallback behavior.
| "Skipping auto-approval because review output marker was published", | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
[MINOR] Unnecessary block scope
The inner block { ... } creates a new scope but doesn't add value. Consider removing it for clarity:
Suggested change
| } | |
| if (!canPublishVisibleOutput("auto-approval")) { | |
| return; | |
| } | |
| setReviewWorkPhase("publish"); |
Member
Author
There was a problem hiding this comment.
Fixed. I removed the unnecessary block scope in the auto-approval path.
| body: sanitizeOutgoingMentions(updatedBody, botHandles), | ||
| }); | ||
| return true; | ||
| return summaryComment.id; |
There was a problem hiding this comment.
[MAJOR] Type error: function signature declares Promise<void> (line 532) but returns summaryComment.id (number).
Line 600 also returns undefined, which is inconsistent with void semantics.
Suggested change
| return summaryComment.id; | |
| return summaryComment.id; | |
| } |
Also update the function signature at line 532:
}): Promise<number | undefined> {
Member
Author
There was a problem hiding this comment.
Fixed. upsertReviewDetailsComment(...) now returns Promise<number | undefined> instead of Promise<void>, matching the actual returned comment-id behavior.
Comment on lines
513
to
519
| @@ -516,7 +516,6 @@ async function upsertReviewDetailsComment(params: { | |||
| issue_number: prNumber, | |||
| body: sanitizedBody, | |||
| }); | |||
| return true; | |||
| } | |||
There was a problem hiding this comment.
[MAJOR] Missing return value causes runtime bug. Function signature declares Promise<void> (line 481) but callers expect comment ID to be returned (see usage at lines ~3834 and ~3892 in the diff).
Without this return, reviewDetailsCommentId will always be undefined, causing the subsequent conditional checks to fail silently.
Suggested change
| const created = await octokit.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: prNumber, | |
| body: sanitizedBody, | |
| }); | |
| return created.data.id; | |
| } |
Also update the function signature at line 481:
}): Promise<number | undefined> {And update line 510 to return the existing comment ID:
return existingComment.id;
Member
Author
There was a problem hiding this comment.
Fixed. appendReviewDetailsToSummary(...) now also returns Promise<number | undefined>, so callers that capture the summary comment id are aligned with the function contract.
Member
Author
|
@kodiai review again |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens several runtime review/execution paths and adds repo-owned verifier + CI guardrails, along with backfilled regression coverage across knowledge/jobs/webhook-related surfaces.
Changes:
- Hardened runtime behaviors (e.g., secret scanning before publishing mention replies; cluster scheduler start idempotence; safer wiki API URL construction).
- Added substantial new test coverage for knowledge modules and job utilities, plus verifier harness tests.
- Tightened CI guardrails: pinned Bun version, added ESLint + “orphaned tests” / migration rollback verification, and updated workflow wiring.
Reviewed changes
Copilot reviewed 102 out of 120 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/knowledge/wiki-fetch.ts | Builds wiki API URLs via URL, merging query params more robustly. |
| src/knowledge/wiki-fetch.test.ts | Adds unit tests for wiki URL building and wiki header wrapping. |
| src/knowledge/test-coverage-exemptions.ts | Introduces runtime target + type-only exemption manifests for coverage enforcement. |
| src/knowledge/store.test.ts | Switches DB tests to require TEST_DATABASE_URL and conditionally skip when absent. |
| src/knowledge/issue-retrieval.test.ts | Adds unit tests for issue retrieval embedding + mapping behavior. |
| src/knowledge/isolation.test.ts | Adds isolation-layer behavior tests (adaptive, dedupe, provenance). |
| src/knowledge/cluster-scheduler.ts | Makes scheduler start() idempotent to prevent timer leakage. |
| src/knowledge/cluster-scheduler.test.ts | Adds scheduler regression tests (fail-open, timer lifecycle, empty repos). |
| src/jobs/gist-publisher.test.ts | Adds tests for gist publisher enabled/disabled behavior and payload shape. |
| src/jobs/fork-manager.ts | Logs merge-conflict warnings for upstream sync conflicts. |
| src/jobs/fork-manager.test.ts | Adds fork-manager regression tests (cache, provisioning poll, 409 handling, best-effort delete). |
| src/index.ts | Removes Slack relay webhook route wiring. |
| src/handlers/mention.ts | Adds outgoing secret scanning before publishing mention replies. |
| src/execution/types.ts | Removes gitDiffInspectionAvailable from execution context. |
| src/execution/review-prompt.ts | Removes degraded diff guidance and always suggests git diff/log commands. |
| src/execution/review-prompt.test.ts | Removes tests covering degraded diff guidance behavior. |
| src/execution/executor.ts | Simplifies tool selection by removing the degraded-diff conditional branch. |
| src/db/migrations/036-suggestion-cluster-models.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/035-generated-rules.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/034-review-graph.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/033-canonical-code-corpus.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/030-reserved.sql | Adds a schema-neutral reserved migration slot to make history contiguous. |
| src/db/migrations/030-reserved.down.sql | Adds schema-neutral rollback for the reserved migration slot. |
| src/db/migrations/026-guardrail-audit.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/025-wiki-style-cache.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/016-issue-triage-state.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/013-review-clusters.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/012-wiki-staleness-run-state.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrate.ts | Documents intentional console output for operator-facing migration CLI. |
| src/contributor/xbmc-fixture-snapshot.test.ts | Removes dynamic new Function import indirection; uses native dynamic import. |
| src/contributor/xbmc-fixture-refresh.ts | Updates app config fixture to remove Slack relay sources. |
| src/contributor/xbmc-fixture-refresh.test.ts | Removes dynamic new Function import indirection; uses native dynamic import. |
| src/contributor/profile-store.test.ts | Switches DB tests to require TEST_DATABASE_URL and conditionally skip when absent. |
| src/contributor/calibration-evaluator.test.ts | Removes dynamic new Function import indirection; uses native dynamic import. |
| src/contributor/calibration-change-contract.test.ts | Removes dynamic new Function import indirection; uses native dynamic import. |
| src/config.ts | Removes Slack webhook relay source parsing/config surface. |
| src/config.test.ts | Removes Slack webhook relay parsing tests and env handling. |
| scripts/verify-m060-s02.test.ts | Adds tests for ownership-boundary verifier wiring and stable status codes. |
| scripts/verify-m059-s02.test.ts | Adds tests for script/orphan audit verifier behavior and wiring. |
| scripts/verify-m058-s03.test.ts | Adds tests for CI gate + lint wiring verifier behavior. |
| scripts/verify-m058-s02.test.ts | Adds tests for pinned Bun contract verifier (package/workflow/docs alignment). |
| scripts/verify-m058-s01.ts | Adds CI coverage/split/ordering verifier to enforce CI contract. |
| scripts/verify-m058-s01.test.ts | Adds regression tests for the CI coverage verifier. |
| scripts/verify-m057-s04.ts | Adds verifier script bundling targeted gates for M057/S04. |
| scripts/verify-m057-s04.test.ts | Adds tests asserting command bundle constants and package wiring. |
| scripts/verify-m057-s03.ts | Adds verifier runner with injectable spawn/write/exit for testability. |
| scripts/verify-m057-s03.test.ts | Adds tests for verifier runner output and early-exit behavior. |
| scripts/verify-m057-s02.ts | Adds a simple verifier that runs two targeted tests via spawnSync. |
| scripts/verify-m056-s02.test.ts | Adds regression tests for rollback completeness verifier (static + runtime wiring). |
| scripts/verify-m056-s01.test.ts | Adds regression tests for rollback verifier (static + runtime wiring). |
| scripts/verify-m055-s02.ts | Adds docs-truth verifier for LICENSE/CONTRIBUTING + package wiring. |
| scripts/verify-m055-s02.test.ts | Adds regression tests for docs-truth verifier behavior and wiring. |
| scripts/verify-m055-s01.ts | Adds docs-truth verifier for README/CHANGELOG + package wiring. |
| scripts/verify-m055-s01.test.ts | Adds regression tests for README/CHANGELOG docs-truth verifier. |
| scripts/verify-m054-s03.test.ts | Adds recent-history verifier test coverage and wiring checks. |
| scripts/verify-m054-s02.ts | Adds historical-folder inventory verifier for milestones + package wiring. |
| scripts/verify-m054-s02.test.ts | Adds regression tests for historical-folder verifier behavior. |
| scripts/verify-m054-s01.test.ts | Adds regression tests for queue truth verifier. |
| scripts/verify-m053.ts | Adds invariant verifier enforcing “no new Function() in src/” plus decision record. |
| scripts/verify-m053.test.ts | Adds regression tests for M053 invariant proof harness and wiring. |
| scripts/verify-m049-s02.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/verify-m044-s01.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/verify-m029-s04.ts | Refines auto-discovery gating for injected sql/octokit and removes Slack relay sources. |
| scripts/verify-m027-s03.test.ts | Updates expected embedding model IDs in M027 verifier tests. |
| scripts/sync-triage-reactions.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/publish-wiki-updates.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/cleanup-wiki-issue.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/check-orphaned-tests.test.ts | Adds regression tests for orphaned-test checker and package wiring. |
| scripts/check-migrations-have-downs.test.ts | Adds regression tests for migration down-file checker and allowlist behavior. |
| scripts/backfill-review-comments.ts | Updates app config fixture to remove Slack relay sources. |
| scripts/backfill-issues.ts | Updates app config fixture to remove Slack relay sources. |
| package.json | Pins Bun version, adds ESLint, and wires new verify/check scripts. |
| eslint.config.mjs | Adds initial ESLint flat config with no-console and CLI carve-outs. |
| bunfig.toml | Expands Bun test root to repo root and attempts to ignore tmp/**. |
| .github/workflows/nightly-reaction-sync.yml | Pins Bun version to 1.3.8. |
| .github/workflows/nightly-issue-sync.yml | Pins Bun version to 1.3.8. |
| .github/workflows/ci.yml | Pins Bun version, adds lint/verifier gates, and splits test execution safely. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return `${baseUrl}/api.php?${params.toString()}`; | ||
| const url = new URL(baseUrl); | ||
| const pathname = url.pathname.endsWith("/") ? url.pathname.slice(0, -1) : url.pathname; | ||
| url.pathname = `${pathname}/api.php`; |
Member
Author
There was a problem hiding this comment.
Fixed. buildWikiApiUrl() now preserves an existing /api.php pathname instead of appending it again, and I added a regression test for https://.../api.php?... inputs.
| logger.info({ forkOwner, forkRepo, branch }, "Fork synced with upstream"); | ||
| } catch (error: unknown) { | ||
| if (typeof error === "object" && error !== null && "status" in error && error.status === 409) { | ||
| logger.warn({ forkOwner, forkRepo, branch, error }, "Fork sync hit merge conflict"); |
Member
Author
There was a problem hiding this comment.
Fixed. The fork-manager warning paths now log a redacted error summary (errorStatus / errorCode / errorMessage) instead of the raw Octokit error object, and the logger assertions were updated accordingly.
Comment on lines
+2
to
+5
| # Scan the repo root so targeted script verifiers can be run by path, but ignore | ||
| # vendored reference code under tmp/ so upstream tests are never discovered. | ||
| root = "." | ||
| pathIgnorePatterns = ["tmp/**"] |
Member
Author
There was a problem hiding this comment.
I checked this one and did not change the file. pathIgnorePatterns is a supported Bun [test] config key here, so this comment does not appear to require a code change on the current Bun configuration.
Comment on lines
+13
to
+30
| for (const command of TEST_COMMANDS) { | ||
| const [cmd, ...args] = command; | ||
| const formatted = formatCommand(command); | ||
| process.stdout.write(`→ ${formatted}\n`); | ||
|
|
||
| const result = Bun.spawnSync({ | ||
| cmd: [cmd, ...args], | ||
| stdout: "inherit", | ||
| stderr: "inherit", | ||
| }); | ||
|
|
||
| if (result.exitCode !== 0) { | ||
| process.stderr.write(`${COMMAND_NAME} failed: ${formatted}\n`); | ||
| process.exit(result.exitCode); | ||
| } | ||
| } | ||
|
|
||
| process.stdout.write(`${COMMAND_NAME} passed\n`); |
Member
Author
There was a problem hiding this comment.
Fixed. scripts/verify-m057-s02.ts now exports runVerifyM057S02() and only executes under if (import.meta.main), with matching unit coverage.
kodiai responseI ran out of steps analyzing this and wasn't able to post a complete response. This can happen on PRs with large or complex diffs. To get a response:
|
The review run ended before it could publish comments or an approval. Stop reason: tool_use Try requesting another review after narrowing the scope or improving the available review context. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens several runtime/review flows and adds repo-owned “verifier” guardrails, while backfilling regression tests (notably around knowledge-layer modules, fork/gist handling, and cluster scheduling) and tightening CI wiring.
Changes:
- Adds new repo-owned verifier scripts (Bun/CI wiring, orphaned tests, docs truth, rollback completeness) and wires them into
package.json+ CI. - Backfills tests for knowledge-layer utilities (wiki fetch, isolation, issue retrieval), cluster scheduler idempotency, and job utilities (fork manager, gist publisher).
- Pins Bun version across
package.jsonand GitHub Actions, introduces ESLint with a narrow operator-surfaceno-consolecarveout.
Reviewed changes
Copilot reviewed 103 out of 121 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/knowledge/wiki-fetch.ts | Makes wiki API URL building more robust by using URL parsing/merging. |
| src/knowledge/wiki-fetch.test.ts | Adds unit coverage for wiki URL building and header injection behavior. |
| src/knowledge/test-coverage-exemptions.ts | Introduces a manifest for runtime targets and type-only exemptions. |
| src/knowledge/store.test.ts | Makes DB-backed tests opt-in via TEST_DATABASE_URL and skips otherwise. |
| src/knowledge/issue-retrieval.test.ts | Adds unit coverage for issue embedding search/mapping behavior. |
| src/knowledge/isolation.test.ts | Adds regression coverage for isolation layer filtering/dedup/provenance. |
| src/knowledge/cluster-scheduler.ts | Makes start() idempotent to prevent duplicate timer leakage. |
| src/knowledge/cluster-scheduler.test.ts | Adds scheduler regression coverage (fail-open, timers, empty repo list). |
| src/jobs/gist-publisher.test.ts | Adds tests for gist publishing (disabled mode + success payload/logging). |
| src/jobs/fork-manager.ts | Improves fork-manager warning logs with summarized error details. |
| src/jobs/fork-manager.test.ts | Adds coverage for fork caching, polling, 409 conflicts, best-effort delete. |
| src/index.ts | Removes Slack relay webhook route wiring from the main app. |
| src/handlers/mention.ts | Adds outgoing secret scan and blocks publishing banned content. |
| src/execution/types.ts | Removes gitDiffInspectionAvailable from execution context typing. |
| src/execution/review-prompt.ts | Removes degraded-diff guidance and always suggests git diff/log commands. |
| src/execution/review-prompt.test.ts | Removes tests asserting degraded diff guidance behavior. |
| src/execution/executor.ts | Simplifies tool allowlist selection (removes diff-inspection gating). |
| src/db/migrations/036-suggestion-cluster-models.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/035-generated-rules.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/034-review-graph.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/033-canonical-code-corpus.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/030-reserved.sql | Reserves an unused migration slot with a schema-neutral no-op. |
| src/db/migrations/030-reserved.down.sql | Adds schema-neutral down migration for the reserved slot. |
| src/db/migrations/026-guardrail-audit.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/025-wiki-style-cache.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/016-issue-triage-state.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/013-review-clusters.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrations/012-wiki-staleness-run-state.down.sql | Adds missing down migration for rollback completeness. |
| src/db/migrate.ts | Documents intentional console output (operator-facing CLI surface). |
| src/contributor/xbmc-fixture-snapshot.test.ts | Removes dynamic new Function() import helper in tests. |
| src/contributor/xbmc-fixture-refresh.ts | Removes Slack relay config field from fixture config builder. |
| src/contributor/xbmc-fixture-refresh.test.ts | Removes dynamic new Function() import helper in tests. |
| src/contributor/profile-store.test.ts | Makes DB-backed tests opt-in via TEST_DATABASE_URL and skips otherwise. |
| src/contributor/calibration-evaluator.test.ts | Removes dynamic new Function() import helper in tests. |
| src/contributor/calibration-change-contract.test.ts | Removes dynamic new Function() import helper in tests. |
| src/config.ts | Removes Slack relay configuration parsing/schema fields. |
| src/config.test.ts | Removes Slack relay config parsing assertions/tests. |
| scripts/verify-m060-s02.test.ts | Adds verifier tests for M060/M027 boundary contract wiring. |
| scripts/verify-m059-s02.test.ts | Adds verifier tests for script registry orphan-audit truth. |
| scripts/verify-m058-s03.test.ts | Adds verifier tests for CI gate wiring and ESLint lint contract. |
| scripts/verify-m058-s02.test.ts | Adds verifier tests for Bun pinning across package/workflows/docs. |
| scripts/verify-m058-s01.ts | Adds verifier CLI for CI coverage breadth + split rationale contract. |
| scripts/verify-m058-s01.test.ts | Adds tests for M058 S01 CI coverage verifier behavior. |
| scripts/verify-m057-s04.ts | Adds a verifier that runs a specific command bundle via Bun.spawnSync. |
| scripts/verify-m057-s04.test.ts | Adds package/exports-level coverage for verify:m057:s04 wiring. |
| scripts/verify-m057-s03.ts | Adds a verifier runner with injectable spawn/output/exit for testability. |
| scripts/verify-m057-s03.test.ts | Adds tests for verify:m057:s03 runner control flow. |
| scripts/verify-m057-s02.ts | Adds a verifier runner for CI failure-related tests with injectable hooks. |
| scripts/verify-m057-s02.test.ts | Adds tests for verify:m057:s02 runner control flow. |
| scripts/verify-m056-s02.test.ts | Adds verifier tests for rollback completeness + runtime roundtrip behavior. |
| scripts/verify-m056-s01.test.ts | Adds verifier tests for rollback completeness + runtime roundtrip behavior. |
| scripts/verify-m055-s02.test.ts | Adds verifier tests for docs truth contract markers + wiring. |
| scripts/verify-m055-s01.ts | Adds verifier for README/CHANGELOG truth contracts with package wiring. |
| scripts/verify-m055-s01.test.ts | Adds tests for README/CHANGELOG truth verifier and wiring. |
| scripts/verify-m054-s03.test.ts | Adds verifier tests for recent milestone inventory/summaries + wiring. |
| scripts/verify-m054-s02.ts | Adds verifier for historical .gsd milestone folder inventory + wiring. |
| scripts/verify-m054-s02.test.ts | Adds tests for historical folder verifier drift/unreadable cases. |
| scripts/verify-m054-s01.test.ts | Adds tests for queue truth verifier behavior and wiring. |
| scripts/verify-m053.ts | Adds a verifier enforcing “no new Function() under src/” invariant. |
| scripts/verify-m053.test.ts | Adds tests for M053 verifier behavior and package wiring. |
| scripts/verify-m049-s02.ts | Removes Slack relay config field from verifier config builder. |
| scripts/verify-m044-s01.ts | Removes Slack relay config field from verifier config builder. |
| scripts/verify-m029-s04.ts | Tightens auto-discovery logic for injected deps; removes Slack relay config field. |
| scripts/verify-m027-s03.test.ts | Updates expected embedding model strings in M027 verifier tests. |
| scripts/sync-triage-reactions.ts | Removes Slack relay config field from script config builder. |
| scripts/publish-wiki-updates.ts | Removes Slack relay config field from script config builder. |
| scripts/cleanup-wiki-issue.ts | Removes Slack relay config field from script config builder. |
| scripts/check-orphaned-tests.test.ts | Adds tests for orphaned-test ownership gate + package wiring. |
| scripts/check-migrations-have-downs.test.ts | Adds tests for migration rollback sibling gate + allowlist + wiring. |
| scripts/backfill-review-comments.ts | Removes Slack relay config field from script config builder. |
| scripts/backfill-issues.ts | Removes Slack relay config field from script config builder. |
| package.json | Pins Bun version, adds ESLint, and wires many new verify/check scripts. |
| eslint.config.mjs | Introduces ESLint config with no-console and operator-surface exceptions. |
| bunfig.toml | Expands test root to repo-wide while ignoring tmp/**. |
| .github/workflows/nightly-reaction-sync.yml | Pins Bun version to 1.3.8. |
| .github/workflows/nightly-issue-sync.yml | Pins Bun version to 1.3.8. |
| .github/workflows/ci.yml | Pins Bun, adds lint/verifier steps, and changes test selection/splitting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+50
| # The first run covers scripts plus non-knowledge src tests; src/knowledge stays isolated. | ||
| - run: | | ||
| mapfile -t non_knowledge_tests < <( | ||
| { | ||
| find src -maxdepth 1 -type f -name '*.test.ts' | ||
| find src -maxdepth 1 -mindepth 1 -type d ! -name knowledge | ||
| } | sort | ||
| ) | ||
| bun test --max-concurrency=2 scripts "${non_knowledge_tests[@]}" |
Comment on lines
+271
to
+272
| function runStepMatch(ciContent: string, command: string): RegExpMatchArray | null { | ||
| return ciContent.match(new RegExp(`^\\s*- run: ${escapeForRegex(command)}\\s*$`, "m")); |
|
|
||
| if (result.exitCode !== 0) { | ||
| process.stderr.write(`${COMMAND_NAME} failed: ${formatted}\n`); | ||
| process.exit(result.exitCode); |
Comment on lines
+10
to
+31
| function formatCommand(command: readonly string[]): string { | ||
| return command.join(" "); | ||
| } | ||
|
|
||
| export function runVerifyM057S04(commands: readonly VerifyCommand[] = VERIFY_COMMANDS): void { | ||
| for (const command of commands) { | ||
| const formatted = formatCommand(command); | ||
| process.stdout.write(`→ ${formatted}\n`); | ||
|
|
||
| const result = Bun.spawnSync({ | ||
| cmd: [...command], | ||
| stdout: "inherit", | ||
| stderr: "inherit", | ||
| }); | ||
|
|
||
| if (result.exitCode !== 0) { | ||
| process.stderr.write(`${COMMAND_NAME} failed: ${formatted}\n`); | ||
| process.exit(result.exitCode); | ||
| } | ||
| } | ||
|
|
||
| process.stdout.write(`${COMMAND_NAME} passed\n`); |
| @@ -544,16 +542,6 @@ const app = new Hono(); | |||
|
|
|||
| // Mount routes | |||
| app.route("/webhooks", createWebhookRoutes({ config, logger, dedup, githubApp, eventRouter, requestTracker, webhookQueueStore, shutdownManager })); | |||
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
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.
What this does
This PR tightens several production review/runtime paths and adds a large set of missing regression and verifier coverage around them.
Main changes
Review and execution hardening
CI and verifier guardrails
Migration rollback coverage
Webhook / handler regression coverage
Knowledge-layer regression coverage
src/knowledgemodules including isolation, wiki fetch, issue retrieval, popularity config/scoring, linkshere fetching, and cluster schedulingstart()paths idempotentWhy
The codebase had several important runtime and verification seams that were either under-tested or only informally enforced. This PR makes those contracts explicit and machine-checked so review/runtime behavior, migration rollback safety, and key knowledge-layer logic are harder to break silently.
Notes for review
.gsd, docs, and other artifact churn has been removed from the PR.