Harden code review workflow release

This commit is contained in:
Bryan Helmkamp 2026-08-28 14:32:33 -04:00
parent 81d714d8b7
commit 7416745cd7
No known key found for this signature in database
20 changed files with 269 additions and 626 deletions

View file

@ -1,47 +0,0 @@
# Repository review rules for the code-review workflow (xhigh/max tiers
# audit the full set; medium audits these plus the AGENTS.md pack).
#
# Rules are read from a review's base revision, so a change here takes
# effect after it lands. Validate before committing:
# python3 .fabro/workflows/code-review/scripts/code_review.py lint-rules
version: 1
rules:
- id: project.generated-docs
description: >
Generated reference regions are owned by `cargo dev docs refresh`;
hand edits are overwritten on the next refresh and fail the
staleness check.
match:
paths:
- "docs/public/reference/cli.mdx"
- "docs/public/reference/user-configuration.mdx"
checks:
- id: generated-region-integrity
category: conventions
guidance: |
Content between a `{/* generated:... */}` marker and its closing
`{/* /generated:... */}` marker is generator output. Flag any
hand-written change inside those markers; anchor at the edited
line. The fix is to change the generator's source (the CLI's
clap definitions or the options source) and run
`cargo dev docs refresh`. Edits outside the markers are ordinary
documentation and are fine.
- id: project.cli-reference-sync
description: >
The CLI reference is captured from the CLI's own help output.
match:
paths:
- "lib/apps/fabro-cli/src/args.rs"
checks:
- id: docs-refresh
category: conventions
guidance: |
A change that adds, removes, or renames a CLI argument or
subcommand, or changes its help text or default value, must
include the regenerated `docs/public/reference/cli.mdx` in the
same change (run `cargo dev docs refresh`). Anchor the finding
at the changed argument, not at the documentation file. Purely
internal changes that do not alter the CLI's help output need no
refresh.

View file

@ -33,7 +33,7 @@ digraph CodeReview {
timeout="300s",
output_schema="routing",
stdin_source="context.internal.run_id",
script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py 961fe81afa6a5c80432d8e8a05791d6c12ccc724e9f945e7190bbac77a6121d2 .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/publish_pr.py fe65b5c710e6f486bf0b4a0da4933b53b954c32481313efd8d60d34b3dcc3a44 .fabro/workflows/code-review/scripts/render_report.py 91ea86428ed4759fbb368b4fa90ca8f54f1ab05f09f18bc8a4e9ffa188ed2295 .fabro/workflows/code-review/scripts/rule_loader.py f885409c64c631075d7e31fcb6e7a100592430c5eba9a6e989222006061a9f52 .fabro/workflows/code-review/specs/report-spec.md 7a54f72ee46f09218d18854d184a1f36875f9011877e94779c6b1f0d5dd118a9 .fabro/workflows/code-review/templates/report.html 5def570da34ca186da31781378367d70fb9c58e82f7aeec4aaf420fd348a8e61 .fabro/workflows/code-review/schemas/findings.schema.json 2f4d0a9052d5af0dad92db12a1e9d49cc91a282c4dddda495791352bf1559ed8 .fabro/workflows/code-review/schemas/verdict.schema.json de13ce02c5fd0c088640542831cc732e35dee3ddb38f89d4412f6a46fea75567 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 cb3866240077d1bc8993b2f012a8d66a6ea61d4a9f2e88a1fefc6ef375f630e2 .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 a81ee5b0ac134eb121dbf503025387c64126d3276e4673ebc836cfb62a3689fb .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 47e3123fb25c560e36216dc9b8323c86e8301f1868a4fa3219bfd59bda3a533a && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}"
script="python3 -c \"import hashlib,sys; pairs=list(zip(sys.argv[1::2],sys.argv[2::2])); sys.exit(0 if pairs and all(hashlib.sha256(open(path,'rb').read()).hexdigest()==expected for path,expected in pairs) else 91)\" .fabro/workflows/code-review/scripts/code_review.py 6770c5ea6673063ea63a9bbef178c37b5427f24476122a4333eff7c4d390bbc2 .fabro/workflows/code-review/scripts/git_readonly.py bcd4364ba3aca2ee1e12d5909204f645c16bdf22e3753a39d74c79d8d37cf73e .fabro/workflows/code-review/scripts/publish_pr.py b69d8ed821293f05cb56e1719c460ac69d5fa08f9ea3123c3fca6811309087da .fabro/workflows/code-review/scripts/render_report.py 79def74b4415f1a6d1d58cab6480450fed2ee9de11105db95e0996ed08057085 .fabro/workflows/code-review/scripts/rule_loader.py 6cb9c665db54d4dd799acd0867c9a68ad0a26e19de804d63b7781abbd43d60e6 .fabro/workflows/code-review/specs/report-spec.md 7a54f72ee46f09218d18854d184a1f36875f9011877e94779c6b1f0d5dd118a9 .fabro/workflows/code-review/templates/report.html 5def570da34ca186da31781378367d70fb9c58e82f7aeec4aaf420fd348a8e61 .fabro/workflows/code-review/schemas/findings.schema.json 2f4d0a9052d5af0dad92db12a1e9d49cc91a282c4dddda495791352bf1559ed8 .fabro/workflows/code-review/schemas/verdict.schema.json de13ce02c5fd0c088640542831cc732e35dee3ddb38f89d4412f6a46fea75567 .fabro/workflows/code-review/schemas/file-groups.schema.json b53c4e1c0bbd07bbf70e83f4f3b35fd96cb880c621c7c424e95b9aea34e13d7c .fabro/workflows/code-review/prompts/finder.md.j2 86c2e6a032f7c54c1bbab1c12496a8f0d6bf48703abe6017eb175330608cf223 .fabro/workflows/code-review/prompts/verify.md.j2 cb3866240077d1bc8993b2f012a8d66a6ea61d4a9f2e88a1fefc6ef375f630e2 .fabro/workflows/code-review/prompts/sweep.md.j2 e6f89b47b11c57030a6ef7d5896ccb37dbd2a2982e9fa7eb7f2e3df73acab82c .fabro/workflows/code-review/prompts/group-files.md.j2 5b291313a1266d1d658f80ea7989cdefcd8609b6893b7197b17914d553dab041 .fabro/workflows/code-review/prompts/partials/finding-fields.md.j2 a81ee5b0ac134eb121dbf503025387c64126d3276e4673ebc836cfb62a3689fb .fabro/workflows/code-review/prompts/partials/guidance.md.j2 53bc0c40bb917288708bed1f9ba478fbd89b9790c92497762224cc752f40bef5 .fabro/workflows/code-review/prompts/partials/output-schema.md.j2 811994bb357739f2562d84f66dc05075ebe3c7f8d58034f8c25ee1c36bee996b .fabro/workflows/code-review/prompts/partials/read-only-explorer.md.j2 44a0244e7aa62fdb0dbbfdbadcffbfb640af249bae3e96895dedd5c7a33bad10 .fabro/workflows/code-review/prompts/partials/review-target.md.j2 abffeeff0e16b89a0754cd53f1833b3744494cd54ff761798b782a80467446ea .fabro/workflows/code-review/prompts/partials/safe-git-history.md.j2 4ddd8d36d5c51d7e166a6b7f1dff51b72cce0e64108cc7e892002ca909af8b3a .fabro/workflows/code-review/rules/builtin-manifest.json 68f853c1624e6a14596d0d5b79501b8b4ba182ce65a7670af413c667fbd84fc8 && python3 .fabro/workflows/code-review/scripts/code_review.py prepare --review-id-stdin --mode {{ inputs.mode }} --effort {{ inputs.effort }} --scope {{ inputs.scope }} --base {{ inputs.base }} --commit {{ inputs.commit }} --range {{ inputs.range }} --model {{ inputs.model }} --guidance {{ inputs.guidance }}"
]
grouping [
@ -43,7 +43,7 @@ digraph CodeReview {
output_schema="@schemas/file-groups.schema.json",
output_retries=2,
max_retries=2,
on_failure="succeed",
on_failure="route",
timeout="1800s",
project_memory=false
]
@ -140,7 +140,7 @@ digraph CodeReview {
output_schema="@schemas/findings.schema.json",
output_retries=2,
max_retries=2,
on_failure="succeed",
on_failure="route",
timeout="7200s",
project_memory=false
]

View file

@ -1,47 +0,0 @@
"""Inventory helpers for the demo storefront.
Deliberate review fixture: this module plants small correctness bugs for the
workflow's smoke run. Do not fix them; the smoke run expects to find them.
"""
from __future__ import annotations
import json
from pathlib import Path
from typing import Dict, List, Optional
def pick_discount(
user: Dict[str, object],
discounts: Dict[int, float],
) -> Optional[float]:
"""Return the user's discount rate, or None when they have none."""
discount_id = user.get("discount_id")
# Deliberate bug: discount id 0 is a valid catalog entry, but the falsy
# check treats it as "no discount configured".
if not discount_id:
return None
return discounts.get(int(discount_id)) # type: ignore[arg-type]
def total_in_stock(warehouse_counts: List[int]) -> int:
"""Sum the units available across every warehouse."""
total = 0
# Deliberate bug: the off-by-one range never counts the last warehouse.
for index in range(len(warehouse_counts) - 1):
total += warehouse_counts[index]
return total
def load_price_overrides(path: str) -> Dict[str, float]:
"""Read per-SKU price overrides, returning {} when the file is absent."""
overrides: Dict[str, float] = {}
try:
raw = Path(path).read_text(encoding="utf-8")
for sku, price in json.loads(raw).items():
overrides[str(sku)] = float(price)
except Exception:
# Deliberate bug: a corrupt overrides file is silently ignored, so
# every SKU quietly sells at the stale base price.
pass
return overrides

View file

@ -1,12 +0,0 @@
"""Fixture matched by the repository override rule.
The repository rule ``project.fixture-override`` uses ``mode: override``,
so the built-in Python checks are suppressed for this file and only the
``no-print`` check applies. The ``print`` call below is its planted
violation.
"""
def announce(message):
print("announce:", message)
return None

View file

@ -1,33 +0,0 @@
"""Deliberately flawed fixture for the PR publisher's live acceptance run.
Planted correctness bugs, so a refresh commit reviewed with post_pr
enabled is guaranteed inline-postable findings:
- ``percentile`` indexes past the end of the list when fraction is 1.0.
- ``moving_average`` divides every window by the full window size, so the
tail averages are too small.
- ``collect_values`` reuses its default list across calls, so results leak
between otherwise independent calls.
"""
def percentile(values, fraction):
"""Return the value at the given fraction of the sorted input."""
ordered = sorted(values)
index = int(len(ordered) * fraction)
return ordered[index]
def moving_average(values, window):
"""Average each window of the input, including the shorter tail."""
averages = []
for start in range(len(values)):
chunk = values[start:start + window]
averages.append(sum(chunk) / window)
return averages
def collect_values(values, collected=[]):
"""Collect values for one independent operation."""
collected.extend(values)
return collected

View file

@ -1,21 +0,0 @@
"""Deliberately flawed fixture for the xhigh rule verification run.
Two planted violations:
- ``record_event`` uses a mutable default argument, which the built-in
Python rule pack flags.
- ``clear_events`` is missing from the Functions list below, which the
repository rule ``project.fixture-inventory/function-inventory`` flags.
Functions:
- record_event
"""
def record_event(name, events=[]):
events.append(name)
return events
def clear_events(events):
events.clear()
return events

View file

@ -6,7 +6,7 @@
},
{
"path": "rules/builtin/format/bicep.yaml",
"sha256": "1b549eb4ea83a0fa23d0a1d139266dcc80cb1aeb514212602d5ad0fbd74473da"
"sha256": "0a4191dcbccec9a45c584d0796d00ffc9e857780b93d24f9edf3f060e80d77c2"
},
{
"path": "rules/builtin/format/build-gradle.yaml",
@ -58,7 +58,7 @@
},
{
"path": "rules/builtin/format/pot.yaml",
"sha256": "f0ed089cfa808c2407066ed84da55b9359bdbac07754a43d1bcaf730405130a1"
"sha256": "223422af99eecfb72463d7f8e1de9095130eacfa302e7b1a4d3ea4a009922410"
},
{
"path": "rules/builtin/format/prisma.yaml",
@ -74,7 +74,7 @@
},
{
"path": "rules/builtin/format/terraform.yaml",
"sha256": "bbfd40afa55f8131086c1a3d5f2a57dffd34879bfd17b12233f5cac9c61ffbd2"
"sha256": "81d83048def9ee11630242e3fb9129f27a3193bc469f62bcaf243c863e8992f0"
},
{
"path": "rules/builtin/format/thrift.yaml",
@ -106,7 +106,7 @@
},
{
"path": "rules/builtin/language/freemarker.yaml",
"sha256": "4b71fdd2dacacbb4969687bb227e449683b0486035d902c763784cee13086367"
"sha256": "a4b7be672e84d5cb0fd335a02279542e96ad0ae880327ccd59d2566fc410a235"
},
{
"path": "rules/builtin/language/go.yaml",
@ -122,7 +122,7 @@
},
{
"path": "rules/builtin/language/javascript-typescript.yaml",
"sha256": "a11a5ff6c37217ab7938cdf05b1dfbf71cfbd1ea3c7e78bb71adbd973f6aaed1"
"sha256": "ecdc07ad6a61db8f9d1ad8a80544831992c071a2c0eb54cf92ed391d6bf4eaab"
},
{
"path": "rules/builtin/language/jsonnet.yaml",
@ -134,7 +134,7 @@
},
{
"path": "rules/builtin/language/kotlin.yaml",
"sha256": "c5ed824fbace28b161d1d0ac6c6e9b554ffed22085b2dd4a5c4fe1a9e4672160"
"sha256": "14a7545bf6f2817051a1c67d2266d4d5513c8a306f203300fc363a6da88e61a5"
},
{
"path": "rules/builtin/language/matlab.yaml",

View file

@ -32,7 +32,7 @@ rules:
- id: insecure-resource-defaults
category: correctness
guidance: |
- A storage account without `minimumTlsVersion` set to a current version, or with `supportsHttpsTrafficOnly` explicitly set to `false`
- A storage account with `minimumTlsVersion` explicitly set to an outdated TLS version, or with `supportsHttpsTrafficOnly` explicitly set to `false`
- A resource property that disables encryption-at-rest or transparent data encryption where the resource type supports enabling it
- Do not flag a resource for merely omitting an optional hardening property when the diff gives no indication either way — only flag an explicit insecure value or an explicit disabling of a secure default
- id: versioning-and-reproducibility

View file

@ -25,7 +25,7 @@ rules:
- Unbalanced or unescaped quotes in `msgid`/`msgid_plural` strings, breaking the entry
- Multi-line continuation strings concatenated incorrectly (missing trailing space/newline between fragments that changes the resulting text)
- Orphaned `msgid_plural` or `msgstr` without a preceding `msgid`
- Duplicate `msgid` definitions within the file that conflict with each other (different `msgctxt`, comments, or placeholders)
- Duplicate entries with the same `msgctxt` and `msgid` that conflict in comments or placeholders; the same `msgid` under different `msgctxt` values is valid disambiguation
- A non-empty `msgstr` in a template entry, which usually means a translation was accidentally committed into the template
- id: placeholder-consistency
category: correctness

View file

@ -12,6 +12,8 @@ rules:
match:
paths:
- "**/*.{tf,hcl,tfvars}"
- "**/*.tfstate"
- "**/*.tfstate.backup"
checks:
- id: obvious-typos-or-spelling-errors
category: conventions

View file

@ -20,7 +20,7 @@ rules:
- id: output-escaping-and-xss
category: correctness
guidance: |
- Interpolations (`${...}`) that reach HTML without escaping: flag only when auto-escaping is not already active — i.e. the template lacks `<#ftl output_format="HTML">` (FreeMarker 2.3.24+) AND does not use the `.ftlh`/`.ftlx` extension (which auto-activate HTML/XML escaping via `recognize_standard_file_extensions`, on by default since 2.3.24) — and the value is not passed through `?html`/`?url`/`?js_string` appropriate to its sink (HTML body, attribute, URL, JS, CSS)
- Interpolations (`${...}`) that reach HTML without escaping: flag only when auto-escaping is not already active — for example through `<#ftl output_format="HTML">`, or through a `.ftlh`/`.ftlx` extension when `recognize_standard_file_extensions` is enabled (its default depends on `incompatible_improvements`) — and the value is not passed through `?html`/`?url`/`?js_string` appropriate to its sink (HTML body, attribute, URL, JS, CSS)
- Explicit `?no_esc` or `<#noautoesc>` on values that carry user-controlled data — treat as a high-risk escape hatch; flag unless the source is clearly trusted or already sanitized
- Escaping with the wrong context builtin (e.g. `?html` for a value placed inside a URL or inline `<script>`)
- Do not report missing `?html` when auto-escaping is active for the file's output format and no override disables it

View file

@ -10,7 +10,7 @@ rules:
- id: language.javascript-typescript
match:
paths:
- "**/*.{ts,js,tsx,jsx}"
- "**/*.{ts,js,tsx,jsx,mts,cts,mjs,cjs}"
checks:
- id: obvious-typos-or-spelling-errors
category: conventions

View file

@ -8,143 +8,65 @@ version: 1
rules:
- id: language.kotlin
description: |
### 1. Null Safety
- **Issue**: Nullable types not handled correctly, leading to potential `NullPointerException`.
- **Key checks**:
- Avoid overusing `!!` (non-null assertion); prefer safe calls `?.` or the Elvis operator `?:`.
- Ensure nullable properties from data classes or API responses are handled properly.
- **Bad example**:
```kotlin
val length: Int = text!!.length // Risk: text may be null
```
- **Improvement**:
```kotlin
val length: Int = text?.length ?: 0 // Safe handling
```
description: "> Favor precision over recall. Report a Kotlin issue only when the changed code provides a concrete failure or maintenance cost. Do not turn optional idioms into correctness requirements."
match:
paths:
- "**/*.kt"
- "**/*.{kt,kts}"
checks:
- id: null-safety
category: correctness
guidance: |
- A `!!` assertion on a value that can be null on a reachable path, causing `NullPointerException`; prefer a safe call, explicit guard, or Elvis default that preserves intended behavior
- A nullable API or data-class property dereferenced without a prior check
- Do not flag `!!` when a nearby invariant proves the value non-null
- id: dead-code
category: simplification
guidance: |
- Code blocks that can never be reached (e.g., branches where the condition is always false, code after a return statement)
- Variables that are declared but never read or referenced
- Large blocks of commented-out code (with no apparent intent to preserve)
### 2. Function and Expression Conciseness
- **Issue**: Redundant code undermines Kotlin's conciseness.
- **Key checks**:
- Use `=` to simplify single-expression functions (e.g., `fun sum(a: Int, b: Int) = a + b`).
- Replace complex `if-else` chains with `when`.
- Avoid unnecessary `return` (e.g., use expression results directly in lambdas).
- **Bad example**:
```kotlin
fun getGrade(score: Int): String {
if (score >= 90) return "A"
else if (score >= 80) return "B"
else return "C"
}
```
- **Improvement**:
```kotlin
fun getGrade(score: Int) = when {
score >= 90 -> "A"
score >= 80 -> "B"
else -> "C"
}
```
### 3. Collection Operation Optimization
- **Issue**: Inefficient collection operations causing performance problems.
- **Key checks**:
- Prefer `Sequence` for large collections (lazy evaluation reduces intermediate objects).
- Avoid redundant operations (e.g., merge multiple `filter` calls into one).
- Use `groupBy`, `associate`, etc. instead of manual iteration.
- **Bad example**:
```kotlin
val evenSquares = listOf(1, 2, 3).map { it * it }.filter { it % 2 == 0 } // creates intermediate collections
```
- **Improvement**:
```kotlin
val evenSquares = listOf(1, 2, 3).asSequence()
.map { it * it }
.filter { it % 2 == 0 }
.toList() // lazy evaluation
```
---
### 4. Proper Use of Coroutines
- **Issue**: Coroutine leaks or improper exception handling.
- **Key checks**:
- Use structured concurrency (`coroutineScope` or `supervisorScope` to manage lifecycle).
- Avoid `GlobalScope` (prone to resource leaks).
- Exception handling: wrap `withContext` or `async` in `try/catch`.
- **Bad example**:
```kotlin
GlobalScope.launch { // escapes scope, may leak
fetchData()
}
```
- **Improvement**:
```kotlin
viewModelScope.launch { // structured concurrency
try {
withContext(Dispatchers.IO) { fetchData() }
} catch (e: Exception) { /* handle exception */ }
}
```
### 5. Class and Object Design
- **Issue**: Not leveraging Kotlin features leads to redundancy.
- **Key checks**:
- **Data classes**: Use `data class` for pure data objects (auto-generates `equals`/`hashCode`).
- **Sealed classes/interfaces**: Use `sealed class` for restricted type hierarchies (enables exhaustive `when` branch checking).
- **Delegation**: Property delegation (e.g., `by lazy`) or class delegation (`by` for the decorator pattern).
- **Bad example**:
```kotlin
class User(val name: String) {
// manually implementing toString()/equals()...
}
```
- **Improvement**:
```kotlin
data class User(val name: String) // standard methods auto-generated
```
### 6. Resource Management and Scope Functions
- **Issue**: Resources not released or scope functions misused.
- **Key checks**:
- Use `use` to auto-close file/network resources (e.g., `FileInputStream().use { ... }`).
- Scope functions (`let`, `apply`, etc.) should maintain readability; avoid excessive nesting.
- **Bad example**:
```kotlin
val file = File("path")
val reader = BufferedReader(FileReader(file))
// forgot to call reader.close()
```
- **Improvement**:
```kotlin
File("path").inputStream().use { stream ->
// resource auto-closed
}
```
### 7. Performance Pitfalls
- **Issue**: Hidden performance overhead.
- **Key checks**:
- Inline functions: Use `inline` for higher-order functions to reduce lambda overhead (but avoid inlining large functions).
- Constants: Use `const val` for compile-time constants (instead of `val`).
- Avoid creating objects inside loops (e.g., `Regex` instances).
### 8. Interoperability (Java Interaction)
- **Issue**: Compatibility problems when Java code calls Kotlin code.
- **Key checks**:
- Use `@JvmStatic` and `@JvmOverloads` to optimize APIs exposed to Java callers.
- Null safety annotations: Use `@Nullable`/`@NonNull` to help Java recognize nullability.
### 9. Other Key Points
- **Immutability**: Prefer `val` over `var`.
- **String handling**: Use string templates (`"Value: $value"`) instead of concatenation.
- Unreachable branches or statements after an unconditional return or throw
- Variables, functions, or classes that are declared but never used
- Large blocks of commented-out code with no stated reason to retain them
- id: concise-expressions
category: simplification
guidance: |
- A verbose function or conditional whose direct expression form removes duplicated logic without hiding control flow
- Repeated `if`/`else if` equality checks that a `when` expression can express more safely
- Do not report optional expression-body or scope-function rewrites with no concrete benefit
- id: collection-performance
category: efficiency
guidance: |
- Multiple eager collection passes over a large or unbounded input that create avoidable intermediate collections; consider a sequence or one combined operation
- Manual iteration that repeats expensive lookups available from a standard collection operation
- Do not recommend sequences for small collections without a measurable allocation or latency concern
- id: coroutine-lifecycle
category: correctness
guidance: |
- `GlobalScope` or another detached coroutine whose work can outlive the owner, leak resources, or lose failures
- `async` work that is never awaited, or coroutine exceptions that escape the intended supervisor or error boundary
- Blocking I/O on a constrained dispatcher when the surrounding code establishes an appropriate I/O dispatcher
- id: type-design
category: altitude
guidance: |
- A closed state hierarchy modeled as an open type when exhaustive `when` handling is required and a sealed type would prevent missing cases
- A pure value holder with hand-written equality/hash behavior that can drift from its fields; a data class is the safer established abstraction
- Do not request a Kotlin feature only for terseness when the existing API has distinct behavior
- id: resource-management
category: correctness
guidance: |
- A closeable file, stream, cursor, or network resource that is not closed on every success and failure path; use `use` or an equivalent structured owner
- Nested scope functions that bind or return the wrong receiver and cause a concrete logic error
- id: runtime-performance
category: efficiency
guidance: |
- Recompiling regular expressions or allocating equivalent helper objects inside a hot loop
- A higher-order function on a verified hot path where an established inline helper avoids material lambda overhead
- Do not request `inline` or `const val` without a concrete runtime or initialization cost
- id: java-interop
category: correctness
guidance: |
- A Kotlin API called from Java whose generated shape makes the intended call impossible or ambiguous, and where `@JvmStatic`, `@JvmOverloads`, or an explicit overload fixes that contract
- Missing nullability information at a Java boundary that causes a reachable unsafe call
- id: kotlin-idioms
category: conventions
guidance: |
- Mutable `var` state where the value never changes and repository code consistently uses `val` for that invariant
- String concatenation that produces the wrong value or escaping, where a string template makes the intended interpolation explicit

View file

@ -28,6 +28,7 @@ PyYAML dependency; the low tier never imports it.
from __future__ import annotations
import argparse
import functools
import hashlib
import importlib.util
import json
@ -922,12 +923,26 @@ def worktree_dirty() -> Optional[bool]:
status = git("status", "--porcelain=v1", "-z", "--untracked-files=all")
if status.returncode != 0:
return None
for raw_entry in status.stdout.split(b"\0"):
entries = status.stdout.split(b"\0")
index = 0
while index < len(entries):
raw_entry = entries[index]
index += 1
if not raw_entry:
continue
entry = raw_entry.decode("utf-8", "surrogateescape")
path = entry[3:].split(" -> ")[-1] if len(entry) >= 4 else entry
if not is_generated_path(path):
if len(entry) < 4:
return True
status_code = entry[:2]
paths = [entry[3:]]
if status_code[0] in {"R", "C"} or status_code[1] in {"R", "C"}:
if index >= len(entries) or not entries[index]:
return True
paths.append(
entries[index].decode("utf-8", "surrogateescape")
)
index += 1
if any(not is_generated_path(path) for path in paths):
return True
return False
@ -950,7 +965,7 @@ def revision_record(
"commit": target_commit,
"parent": parent,
"branch": branch,
"dirty": False,
"dirty": worktree_dirty(),
"range": revision_range,
}
revision: Dict[str, Any] = {
@ -1531,6 +1546,7 @@ def prepare(args: argparse.Namespace) -> None:
# templates; the engine only records it so the report says what steering
# was applied.
guidance = clean_text(args.guidance, 2000).strip()
cell = EFFORT_CELLS[effort]
revision_range: Optional[str] = None
base: Optional[str] = None
@ -1540,6 +1556,7 @@ def prepare(args: argparse.Namespace) -> None:
changed_files: List[str] = []
diff_lines: Optional[int] = None
scope_file_count: Optional[int] = None
file_records: Optional[Dict[str, Dict[str, Any]]] = None
if mode == "changes":
if not inside_git_worktree():
@ -1579,7 +1596,20 @@ def prepare(args: argparse.Namespace) -> None:
f"base {base!r} and HEAD have no merge base"
)
revision_range = f"{merge_base}..HEAD"
changed_files, diff_lines = diff_stats(revision_range, scope)
if cell["rule_mapped"]:
file_records = diff_file_records(revision_range, scope)
changed_files = sorted(file_records)
churn = [
record.get("added", 0) + record.get("deleted", 0)
for record in file_records.values()
if isinstance(record.get("added"), int)
and isinstance(record.get("deleted"), int)
]
diff_lines = (
sum(churn) if len(churn) == len(file_records) else None
)
else:
changed_files, diff_lines = diff_stats(revision_range, scope)
elif mode == "commit":
if not inside_git_worktree():
raise WorkflowDataError("commit mode requires a Git worktree")
@ -1597,7 +1627,20 @@ def prepare(args: argparse.Namespace) -> None:
target_commit + "^",
)
revision_range = f"{parent or empty_tree_hash()}..{target_commit}"
changed_files, diff_lines = diff_stats(revision_range, scope)
if cell["rule_mapped"]:
file_records = diff_file_records(revision_range, scope)
changed_files = sorted(file_records)
churn = [
record.get("added", 0) + record.get("deleted", 0)
for record in file_records.values()
if isinstance(record.get("added"), int)
and isinstance(record.get("deleted"), int)
]
diff_lines = (
sum(churn) if len(churn) == len(file_records) else None
)
else:
changed_files, diff_lines = diff_stats(revision_range, scope)
else:
if base_input or commit_input or range_input:
raise WorkflowDataError(
@ -1628,7 +1671,6 @@ def prepare(args: argparse.Namespace) -> None:
empty_scope = mode == "files" and scope_file_count == 0
empty_target = empty_diff or empty_scope
cell = EFFORT_CELLS[effort]
state: Dict[str, Any] = {
"version": 1,
"root": str(root()),
@ -1744,7 +1786,10 @@ def prepare(args: argparse.Namespace) -> None:
# the grouping pass. Finder jobs are planned by plan-finders after
# grouping.
if mode in {"changes", "commit"}:
file_records = diff_file_records(revision_range, scope)
if file_records is None:
raise WorkflowDataError(
"rule-mapped diff metadata was not prepared"
)
else:
file_records = {path: {"status": "full"} for path in changed_files}
compile_rule_state(state, file_records)
@ -2337,6 +2382,13 @@ def merge(phase: str) -> None:
f"Merged {phase}: "
f"{updates[f'{phase}_results_merged']} result(s) recorded"
)
if phase in PHASE_OUTPUT_KEYS:
# The merge has copied every usable branch output into canonical
# state. Do not let the next fan-out inherit this fan-in payload.
# Fabro includes inherited context changes in each branch result, so
# retaining an earlier parallel.results array multiplies it by the
# next branch count and can exceed the checkpoint request limit.
updates["parallel.results"] = []
save_state(state)
emit(**updates)
@ -2417,7 +2469,7 @@ def verification_claim(
start_line = int(candidate.get("start_line") or candidate.get("line") or 0)
end_line = int(candidate.get("end_line") or candidate.get("line") or 0)
location = resolved_location(
str(candidate.get("file") or ""), start_line, end_line
str(candidate.get("file") or ""), start_line, end_line, state
)
claim: Dict[str, Any] = {
"file": candidate.get("file"),
@ -2816,17 +2868,38 @@ def safe_code_text(value: str) -> str:
return text
def reviewed_source_lines(file_path: str) -> Optional[List[str]]:
"""Read one UTF-8 source file from the unchanged reviewed tree."""
target = root() / file_path
try:
if target.is_symlink() or not target.is_file():
return None
if target.stat().st_size > CODE_FRAME_MAX_BYTES:
return None
raw = target.read_bytes()
except OSError:
def reviewed_revision(state: Optional[Mapping[str, Any]]) -> Optional[str]:
if not state or state.get("mode") not in {"changes", "commit"}:
return None
revision = state.get("commit")
return revision if isinstance(revision, str) and revision else None
@functools.lru_cache(maxsize=256)
def reviewed_source_lines(
file_path: str, revision: Optional[str] = None
) -> Optional[List[str]]:
"""Read one UTF-8 source file from the exact reviewed revision."""
if revision is not None:
tree_entry = git("ls-tree", "-z", revision, "--", file_path)
if tree_entry.returncode != 0 or not tree_entry.stdout:
return None
mode = tree_entry.stdout.split(None, 1)[0]
if mode == b"120000":
return None
raw = read_file_at_revision(revision, file_path)
if raw is None or len(raw) > CODE_FRAME_MAX_BYTES:
return None
else:
target = root() / file_path
try:
if target.is_symlink() or not target.is_file():
return None
if target.stat().st_size > CODE_FRAME_MAX_BYTES:
return None
raw = target.read_bytes()
except OSError:
return None
if b"\0" in raw:
return None
try:
@ -2836,11 +2909,14 @@ def reviewed_source_lines(file_path: str) -> Optional[List[str]]:
def resolved_location(
file_path: str, start_line: int, end_line: int
file_path: str,
start_line: int,
end_line: int,
state: Optional[Mapping[str, Any]] = None,
) -> Dict[str, Any]:
"""Build an engine-derived exact anchor for a finding."""
existing_code = ""
source_lines = reviewed_source_lines(file_path)
source_lines = reviewed_source_lines(file_path, reviewed_revision(state))
if (
source_lines is not None
and 1 <= start_line <= end_line <= len(source_lines)
@ -2862,7 +2938,10 @@ def resolved_location(
def code_frame(
file_path: str, start_line: int, end_line: Optional[int] = None
file_path: str,
start_line: int,
end_line: Optional[int] = None,
state: Optional[Mapping[str, Any]] = None,
) -> Dict[str, Any]:
"""Read the lines around a finding's anchor range from the reviewed tree.
@ -2877,7 +2956,7 @@ def code_frame(
"label": f"{file_path}:{start_line}-{end_line}",
"lines": [],
}
source_lines = reviewed_source_lines(file_path)
source_lines = reviewed_source_lines(file_path, reviewed_revision(state))
if (
source_lines is None
or start_line < 1
@ -2906,12 +2985,15 @@ def code_frame(
def reportable_finding(
record: Mapping[str, Any],
display_id: str,
state: Optional[Mapping[str, Any]] = None,
) -> Dict[str, Any]:
candidate = record["candidate"]
verdict = record.get("verdict")
start_line = int(candidate.get("start_line") or candidate["line"])
end_line = int(candidate.get("end_line") or candidate["line"])
location = resolved_location(candidate["file"], start_line, end_line)
location = resolved_location(
candidate["file"], start_line, end_line, state
)
finding = {
"id": display_id,
"file": candidate["file"],
@ -2932,7 +3014,9 @@ def reportable_finding(
"source": candidate.get("source", "finder"),
"verdict": verdict["verdict"] if verdict else "UNVERIFIED",
"verdict_reasoning": verdict["reasoning"] if verdict else "",
"code": code_frame(candidate["file"], start_line, end_line),
"code": code_frame(
candidate["file"], start_line, end_line, state
),
}
suggestion_code = str(candidate.get("suggestion_code") or "")
if (
@ -3242,7 +3326,7 @@ def final_tally() -> None:
deferred_by_report_cap = max(0, len(kept_records) - len(reported_records))
findings = [
reportable_finding(record, f"R{index}")
reportable_finding(record, f"R{index}", state)
for index, record in enumerate(reported_records, 1)
]
@ -3539,6 +3623,21 @@ def load_renderer() -> Any:
raise WorkflowDataError("could not load the report renderer")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
parity_checks = (
("category list", tuple(module.CATEGORIES), CATEGORIES),
("issue type list", tuple(module.ISSUE_TYPES), ISSUE_TYPES),
("effort tiers", tuple(module.EFFORT_TIERS), EFFORT_TIERS),
("review modes", tuple(module.REVIEW_MODES), REVIEW_MODES),
)
for label, renderer_value, engine_value in parity_checks:
if renderer_value != engine_value:
raise WorkflowDataError(
f"the renderer's {label} does not match this engine"
)
if module.COMPILED_RULE_ID_RE.pattern != COMPILED_RULE_ID_RE.pattern:
raise WorkflowDataError(
"the renderer's compiled-ID pattern does not match this engine"
)
return module

View file

@ -12,8 +12,11 @@ Posts a completed review's findings to the reviewed GitHub PR in two steps:
PR state) is confined here. The plan file is untrusted input: apply
re-validates it before any write.
Requirements register: .ai/plans/p1-pr-publisher-requirements.md (R-numbers
below reference it). Executable specification: tests/test_pr_publisher.py.
The canonical ``lithoscomputer/code-review`` source repository keeps the
requirements register at ``.ai/plans/p1-pr-publisher-requirements.md`` and
the executable specification at ``tests/test_pr_publisher.py``. Packaged
workflow installs do not need to copy those development files. R-numbers
below refer to that canonical requirements register.
Python 3.9-compatible. Standard library only.
"""
@ -65,18 +68,10 @@ REPO_RE = re.compile(
)
SHA_RE = re.compile(r"^[0-9a-f]{40}$")
HUNK_HEADER_RE = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@")
UNVERIFIED_NOTE = (
"_This finding comes from a low-effort single-pass review and was not "
"independently verified._"
)
PLAUSIBLE_WARNING = (
"> **Needs confirmation:** The verifier could not fully confirm this "
"finding from the available evidence."
)
UNVERIFIED_WARNING = (
"> **Not verified:** This finding comes from a low-effort single-pass "
"review and was not independently verified."
)
class PublishError(RuntimeError):
@ -167,7 +162,8 @@ def right_side_hunks(base: str, head: str) -> Dict[str, List[Tuple[int, int]]]:
boundary starts bare.
"""
result = run_git(
"diff", "--no-color", "--no-ext-diff", "--find-renames", "-U3",
"diff", "--no-color", "--no-ext-diff", "--no-textconv",
"--find-renames", "-U3",
base, head,
)
if result.returncode != 0:
@ -204,12 +200,6 @@ def right_side_hunks(base: str, head: str) -> Dict[str, List[Tuple[int, int]]]:
return ranges
def has_diff_position(
hunks: Mapping[str, Sequence[Tuple[int, int]]], path: str, line: int
) -> bool:
return any(start <= line <= end for start, end in hunks.get(path, ()))
def has_diff_range(
hunks: Mapping[str, Sequence[Tuple[int, int]]],
path: str,
@ -294,35 +284,6 @@ def routing_detail(
# --- Comment and summary rendering (R9, R11) ---------------------------------
def backtick_fence(texts: Sequence[str]) -> str:
longest = 0
for text in texts:
for run in re.findall(r"`+", text):
longest = max(longest, len(run))
return "`" * max(4, longest + 1)
def safe_code_block(code: Mapping[str, Any]) -> List[str]:
lines = code.get("lines") or []
if not lines:
return []
fence = backtick_fence([str(entry["text"]) for entry in lines])
width = max(len(str(entry["number"])) for entry in lines)
body = [fence + "text"]
for entry in lines:
marker = ">" if entry.get("highlight") else " "
body.append(
f"{marker} {str(entry['number']).rjust(width)} | {entry['text']}"
)
body.append(fence)
return body
def raw_code_block(text: str, language: str = "text") -> List[str]:
fence = backtick_fence([text])
return [fence + language, text, fence]
def finding_detail_lines(finding: Mapping[str, Any]) -> List[str]:
lines: List[str] = []
if finding["summary"].strip() != finding["short_summary"].strip():
@ -335,7 +296,7 @@ def finding_detail_lines(finding: Mapping[str, Any]) -> List[str]:
]
)
if finding["verdict"] == "UNVERIFIED":
lines.extend(["", UNVERIFIED_NOTE])
lines.extend(["", renderer.UNVERIFIED_FINDING_ITALIC])
else:
reasoning = str(finding.get("verdict_reasoning") or "").strip()
if reasoning:
@ -413,13 +374,15 @@ def inline_comment_body(finding: Mapping[str, Any], review_id: str) -> str:
if finding["verdict"] == "PLAUSIBLE":
lines.extend(["", PLAUSIBLE_WARNING])
elif finding["verdict"] == "UNVERIFIED":
lines.extend(["", UNVERIFIED_WARNING])
lines.extend(["", renderer.UNVERIFIED_FINDING_WARNING])
suggestion = finding.get("suggestion")
if isinstance(suggestion, dict):
lines.extend(
[
"",
*raw_code_block(suggestion["replacement_code"], "suggestion"),
*renderer.fenced_text(
suggestion["replacement_code"], "suggestion"
),
]
)
lines.extend(inline_more_lines(finding))
@ -457,7 +420,7 @@ def summary_section(
" · ".join(facts),
*finding_detail_lines(finding),
]
excerpt = safe_code_block(finding["code"])
excerpt = renderer.code_block(finding["code"])
if excerpt:
lines.extend(["", *excerpt])
suggestion = finding.get("suggestion")
@ -468,10 +431,10 @@ def summary_section(
"<details><summary>Suggested change</summary>",
"",
"**Before:**",
*raw_code_block(location_data["existing_code"]),
*renderer.fenced_text(location_data["existing_code"]),
"",
"**After:**",
*raw_code_block(suggestion["replacement_code"]),
*renderer.fenced_text(suggestion["replacement_code"]),
"",
"</details>",
]
@ -529,17 +492,9 @@ def summary_context_lines(
) -> List[str]:
lines: List[str] = []
if reasons:
lines.append(
"> **Partial review.** "
+ " ".join(
renderer.escape_markdown(reason) + "." for reason in reasons
)
)
lines.append(renderer.partial_review_warning(reasons))
if manifest["effort"] == "low":
lines.append(
"This was a low-effort single-pass review: findings were not "
"independently verified."
)
lines.append(renderer.LOW_EFFORT_REVIEW_NOTE)
rules_text = rules_coverage_line(coverage, findings)
if rules_text:
lines.append(rules_text)

View file

@ -17,7 +17,6 @@ import json
import hashlib
import os
import re
import sys
from pathlib import Path, PurePosixPath
from typing import Any, Dict, List, Mapping, NoReturn, Sequence, Tuple
from urllib.parse import quote
@ -69,6 +68,18 @@ COMPILED_RULE_ID_RE = re.compile(
MAX_TEXT = 8000
MAX_RULE_IDS_PER_FINDING = 50
MAX_LOCATION_LINES = 50
UNVERIFIED_FINDING_NOTE = (
"This finding comes from a low-effort single-pass review and was not "
"independently verified."
)
UNVERIFIED_FINDING_ITALIC = f"_{UNVERIFIED_FINDING_NOTE}_"
UNVERIFIED_FINDING_WARNING = (
"> **Not verified:** " + UNVERIFIED_FINDING_NOTE
)
LOW_EFFORT_REVIEW_NOTE = (
"This was a low-effort single-pass review: findings were not "
"independently verified."
)
class RenderError(RuntimeError):
@ -635,11 +646,25 @@ def code_span(value: object) -> str:
return f"`{text}`"
def partial_review_warning(reasons: Sequence[str]) -> str:
return "> **Partial review.** " + " ".join(
escape_markdown(reason) + "." for reason in reasons
)
def backtick_fence(texts: Sequence[str]) -> str:
longest = 0
for text in texts:
for run in re.findall(r"`+", text):
longest = max(longest, len(run))
return "`" * max(4, longest + 1)
def code_block(code: Mapping[str, Any]) -> List[str]:
lines = code.get("lines") or []
if not lines:
return []
fence = "````"
fence = backtick_fence([str(entry["text"]) for entry in lines])
body: List[str] = [fence + "text"]
width = max(len(str(entry["number"])) for entry in lines)
for entry in lines:
@ -652,8 +677,7 @@ def code_block(code: Mapping[str, Any]) -> List[str]:
def fenced_text(value: str, language: str = "text") -> List[str]:
longest = max((len(run) for run in re.findall(r"`+", value)), default=0)
fence = "`" * max(4, longest + 1)
fence = backtick_fence([value])
return [fence + language, value, fence]
@ -777,17 +801,9 @@ def render_markdown(
"",
]
if manifest["effort"] == "low":
lines.extend(
[
"This was a low-effort single-pass review: findings were not "
"independently verified.",
"",
]
)
lines.extend([LOW_EFFORT_REVIEW_NOTE, ""])
if reasons:
lines.append("> **Partial review.** " + " ".join(
escape_markdown(reason) + "." for reason in reasons
))
lines.append(partial_review_warning(reasons))
lines.append("")
if findings:
lines.append("## Findings")
@ -966,10 +982,6 @@ def jsonl_line(finding: Mapping[str, Any]) -> str:
SARIF_SCHEMA_URI = "https://json.schemastore.org/sarif-2.1.0.json"
SARIF_VERSION = "2.1.0"
SARIF_LEVELS = {"HIGH": "error", "MEDIUM": "warning", "LOW": "note"}
SARIF_UNVERIFIED_NOTE = (
"This finding comes from a low-effort single-pass review and was not "
"independently verified."
)
CATEGORY_DESCRIPTIONS = {
"correctness": (
"The change can produce wrong behavior: incorrect output, a crash, "
@ -1077,7 +1089,7 @@ def sarif_result(
f"Failure scenario: {finding['failure_scenario']}"
)
if finding["verdict"] == "UNVERIFIED":
message += "\n\n" + SARIF_UNVERIFIED_NOTE
message += "\n\n" + UNVERIFIED_FINDING_NOTE
location = finding["location"]
start_line = location["start_line"]
end_line = location["end_line"]
@ -1271,27 +1283,3 @@ def render(
+ "\n",
)
return findings, dict(manifest["verification"])
def main(argv: Sequence[str]) -> int:
if len(argv) != 3:
print(
"usage: render_report.py <evidence_dir> <products_dir> "
"<metadata_dir>",
file=sys.stderr,
)
return 2
try:
findings, verification = render(argv[0], argv[1], argv[2])
except RenderError as error:
print(f"render_report.py: {error}", file=sys.stderr)
return 2
print(
f"Rendered {len(findings)} finding(s); verification "
f"{verification.get('status')}"
)
return 0
if __name__ == "__main__":
sys.exit(main(sys.argv[1:]))

View file

@ -627,17 +627,28 @@ def discover_repo_rule_paths(paths: Iterable[str]) -> List[str]:
return entrypoint + sorted(extras)
def builtin_manifest_entries(workflow_root: Path) -> List[Dict[str, str]]:
"""Hash every built-in YAML file on disk, in lexical path order."""
def builtin_files_with_hashes(
workflow_root: Path,
) -> List[Tuple[str, str, bytes]]:
"""Read and hash every built-in YAML file in lexical path order."""
rules_dir = workflow_root / BUILTIN_RULES_DIR
entries: List[Dict[str, str]] = []
entries: List[Tuple[str, str, bytes]] = []
for path in sorted(rules_dir.rglob("*.yaml")):
relative = path.relative_to(workflow_root).as_posix()
digest = hashlib.sha256(path.read_bytes()).hexdigest()
entries.append({"path": relative, "sha256": digest})
contents = path.read_bytes()
digest = hashlib.sha256(contents).hexdigest()
entries.append((relative, digest, contents))
return entries
def builtin_manifest_entries(workflow_root: Path) -> List[Dict[str, str]]:
"""Hash every built-in YAML file on disk, in lexical path order."""
return [
{"path": path, "sha256": digest}
for path, digest, _contents in builtin_files_with_hashes(workflow_root)
]
def load_builtin_files(
workflow_root: Path,
manifest: Any,
@ -662,8 +673,8 @@ def load_builtin_files(
raise RuleLoaderError("built-in rule manifest entry is malformed")
expected[entry["path"]] = entry["sha256"]
actual = builtin_manifest_entries(workflow_root)
actual_paths = {entry["path"] for entry in actual}
actual = builtin_files_with_hashes(workflow_root)
actual_paths = {path for path, _digest, _contents in actual}
missing = sorted(set(expected) - actual_paths)
extra = sorted(actual_paths - set(expected))
if missing:
@ -675,15 +686,13 @@ def load_builtin_files(
"unexpected built-in rule files: " + ", ".join(extra)
)
files: List[Tuple[str, bytes]] = []
for entry in actual:
if expected[entry["path"]] != entry["sha256"]:
for path, digest, contents in actual:
if expected[path] != digest:
raise RuleLoaderError(
f"built-in rule file does not match its manifest hash: "
f"{entry['path']}"
f"{path}"
)
files.append(
(entry["path"], (workflow_root / entry["path"]).read_bytes())
)
files.append((path, contents))
return files

View file

@ -1,87 +0,0 @@
_version = 1
[workflow]
graph = "code-review.fabro"
[run]
goal = "Verify the rule-mapped xhigh review against its planted-violation fixtures."
# An xhigh files-mode run over the rule fixtures: exercises grouping, the
# local-correctness fan-out, built-in and repository rule audits (merge and
# override), rule-aware verification, and the coverage-aware sweep. Fails
# unless at least one rule-derived finding survives verification. Repository
# rules are read from the pushed HEAD (.fabro/rules.yaml), so commit and
# push rule changes before running.
[run.inputs]
mode = "files"
effort = "xhigh"
scope = ".fabro/workflows/code-review/fixtures/inventory_utils.py,.fabro/workflows/code-review/fixtures/rules_probe.py,.fabro/workflows/code-review/fixtures/override_probe.py"
base = ""
commit = ""
range = ""
model = "kimi-k3"
guidance = ""
expected_min_findings = "2"
expected_file = ".fabro/workflows/code-review/fixtures/rules_probe.py"
expected_min_rule_findings = "1"
# The shared graph's publish_pr node interpolates these inputs, so every
# workflow.toml that runs the graph must define them; smoke runs never
# post, so they stay empty.
post_pr = ""
pr_repo = ""
pr_number = ""
route_severity_below = ""
route_categories = ""
run_url = ""
[run.run_branch]
enabled = false
[run.pull_request]
enabled = false
[run.model.fallbacks]
"kimi-k3" = ["moonshot:kimi-k3", "modal:kimi-k3", "claude-opus-5"]
[run.environment]
id = "code-review"
[run.environment.env]
GITHUB_TOKEN = ""
GH_TOKEN = ""
[run.artifacts]
include = [
"CODE-REVIEW-*/.gitignore",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.md",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.html",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.jsonl",
"CODE-REVIEW-*/evidence/review-manifest.json",
"CODE-REVIEW-*/evidence/candidate-ledger.jsonl",
"CODE-REVIEW-*/evidence/findings.json",
"CODE-REVIEW-*/evidence/coverage.json",
"CODE-REVIEW-*/evidence/votes.jsonl",
"CODE-REVIEW-*/metadata/revision.json",
"CODE-REVIEW-*/metadata/state.json",
"CODE-REVIEW-*/metadata/review-meta.json",
]
[environments.code-review]
provider = "daytona"
# The review's agents search the tree constantly. The mirrored buildpack-deps
# noble image is the Daytona default base. It ships grep but not ripgrep,
# which respects .gitignore and is far faster on a large repository.
# The xhigh/max rule loader needs PyYAML; the pin and hashes below must stay
# in lockstep with requirements-rules.txt (cp312 manylinux wheels + sdist).
[environments.code-review.image]
dockerfile = """
FROM ghcr.io/lithoscomputer/docker-mirror/buildpack-deps:noble@sha256:1fdce57bbb1105e0e515f6523bd0c3eb1df8b601847cfea140483672f6484afa
RUN apt-get update && apt-get install -y --no-install-recommends ripgrep python3-pip && rm -rf /var/lib/apt/lists/*
RUN echo 'PyYAML==6.0.3 --hash=sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f --hash=sha256:ba1cc08a7ccde2d2ec775841541641e4548226580ab850948cbfda66a1befcdc --hash=sha256:9149cad251584d5fb4981be1ecde53a1ca46c891a79788c0df828d2f166bda28' > /tmp/requirements-rules.txt && python3 -m pip install --break-system-packages --no-cache-dir --require-hashes -r /tmp/requirements-rules.txt && rm /tmp/requirements-rules.txt
"""
[environments.code-review.resources]
cpu = 2
memory = "4GB"

View file

@ -1,86 +0,0 @@
_version = 1
[workflow]
graph = "code-review.fabro"
[run]
goal = "Verify the code-review workflow against its deliberately buggy fixture."
# A medium files-mode run over the planted-bug fixtures: the cheapest shape
# that still exercises the rule-mapped planner (collapsed to local passes
# and rule audits for this small scope), the deterministic merge, repository
# rules, and the verification pass. rules_probe.py plants a violation of the
# repository rule project.fixture-inventory/function-inventory.
[run.inputs]
mode = "files"
effort = "medium"
scope = ".fabro/workflows/code-review/fixtures/inventory_utils.py,.fabro/workflows/code-review/fixtures/rules_probe.py"
base = ""
commit = ""
range = ""
model = "kimi-k3"
guidance = ""
expected_min_findings = "1"
expected_file = ".fabro/workflows/code-review/fixtures/inventory_utils.py"
expected_min_rule_findings = "1"
# The shared graph's publish_pr node interpolates these inputs, so every
# workflow.toml that runs the graph must define them; smoke runs never
# post, so they stay empty.
post_pr = ""
pr_repo = ""
pr_number = ""
route_severity_below = ""
route_categories = ""
run_url = ""
[run.run_branch]
enabled = false
[run.pull_request]
enabled = false
[run.model.fallbacks]
"kimi-k3" = ["moonshot:kimi-k3", "modal:kimi-k3", "claude-opus-5"]
[run.environment]
id = "code-review"
[run.environment.env]
GITHUB_TOKEN = ""
GH_TOKEN = ""
[run.artifacts]
include = [
"CODE-REVIEW-*/.gitignore",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.md",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.html",
"CODE-REVIEW-*/CODE-REVIEW-RESULTS.jsonl",
"CODE-REVIEW-*/evidence/review-manifest.json",
"CODE-REVIEW-*/evidence/candidate-ledger.jsonl",
"CODE-REVIEW-*/evidence/findings.json",
"CODE-REVIEW-*/evidence/coverage.json",
"CODE-REVIEW-*/evidence/votes.jsonl",
"CODE-REVIEW-*/metadata/revision.json",
"CODE-REVIEW-*/metadata/state.json",
"CODE-REVIEW-*/metadata/review-meta.json",
]
[environments.code-review]
provider = "daytona"
# The review's agents search the tree constantly. The mirrored buildpack-deps
# noble image is the Daytona default base. It ships grep but not ripgrep,
# which respects .gitignore and is far faster on a large repository.
# The xhigh/max rule loader needs PyYAML; the pin and hashes below must stay
# in lockstep with requirements-rules.txt (cp312 manylinux wheels + sdist).
[environments.code-review.image]
dockerfile = """
FROM ghcr.io/lithoscomputer/docker-mirror/buildpack-deps:noble@sha256:1fdce57bbb1105e0e515f6523bd0c3eb1df8b601847cfea140483672f6484afa
RUN apt-get update && apt-get install -y --no-install-recommends ripgrep python3-pip && rm -rf /var/lib/apt/lists/*
RUN echo 'PyYAML==6.0.3 --hash=sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f --hash=sha256:ba1cc08a7ccde2d2ec775841541641e4548226580ab850948cbfda66a1befcdc --hash=sha256:9149cad251584d5fb4981be1ecde53a1ca46c891a79788c0df828d2f166bda28' > /tmp/requirements-rules.txt && python3 -m pip install --break-system-packages --no-cache-dir --require-hashes -r /tmp/requirements-rules.txt && rm /tmp/requirements-rules.txt
"""
[environments.code-review.resources]
cpu = 2
memory = "4GB"

View file

@ -94,8 +94,9 @@ provider = "daytona"
# The review's agents search the tree constantly. The mirrored buildpack-deps
# noble image is the Daytona default base. It ships grep but not ripgrep,
# which respects .gitignore and is far faster on a large repository.
# The xhigh/max rule loader needs PyYAML; the pin and hashes below must stay
# in lockstep with requirements-rules.txt (cp312 manylinux wheels + sdist).
# Every tier above low uses the rule loader and needs PyYAML; the pin and
# hashes below must stay in lockstep with requirements-rules.txt (cp312
# manylinux wheels + sdist).
[environments.code-review.image]
dockerfile = """
FROM ghcr.io/lithoscomputer/docker-mirror/buildpack-deps:noble@sha256:1fdce57bbb1105e0e515f6523bd0c3eb1df8b601847cfea140483672f6484afa