Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
85b0c44
fix(review): make OpenCode naming findings evidence-driven
seonghobae Sep 1, 2026
5f06ba3
fix(review): make reviewer naming findings evidence-driven
seonghobae Sep 1, 2026
f5ec419
test(review): reject lexical review authority
seonghobae Sep 1, 2026
0b9df5e
docs(review): record evidence-over-heuristics contract
seonghobae Sep 1, 2026
df91f80
test(review): make prompt regression paragraph-structural
seonghobae Sep 1, 2026
54eda7a
fix(review): preserve adversarial verdict thresholds
seonghobae Sep 1, 2026
5e461a4
fix(review): preserve reviewer output-language contract
seonghobae Sep 1, 2026
8477072
test(review): include runtime OpenCode prompt contract
seonghobae Sep 1, 2026
b5dfce1
fix(review): make runtime OpenCode prompt evidence-driven
seonghobae Sep 1, 2026
5aec6f5
test(review): preserve surrounding review contracts
seonghobae Sep 1, 2026
3156a2c
test(review): require false-negative adversarial probes
seonghobae Sep 1, 2026
77c99c3
fix(review): attack demonstrated false-negative classes
seonghobae Sep 1, 2026
f55c7f2
fix(review): probe recurring false-negative classes
seonghobae Sep 1, 2026
30166c3
fix(review): add runtime false-negative probes
seonghobae Sep 1, 2026
c1633e1
test(review): preserve binding new-DB naming contract
seonghobae Sep 1, 2026
0c01751
fix(review): restore binding new-DB naming policy
seonghobae Sep 1, 2026
52d4d90
fix(review): keep new-DB naming governance
seonghobae Sep 1, 2026
4c4e735
fix(review): enforce new-DB naming in runtime
seonghobae Sep 1, 2026
5d0ae85
docs(review): trace false-negative corpus and DB contract
seonghobae Sep 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 42 additions & 38 deletions ci-review-prompt.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ Apply every evaluation dimension directly; task/subagent dispatch is disabled:
1. correctness-and-tests — correctness, edge cases, error paths, concurrency,
TDD/regression, coverage, docstring, PoC/execution evidence.
2. security-and-supply-chain — auth/authz, tenant isolation, secrets, privacy,
injection, identifier exposure/enumeration (sequential-id) safety,
dependency license and supply chain, packaging.
injection, identifier exposure/enumeration safety, dependency license and
supply chain, packaging.
3. structure-and-claims — structural/DAG impact, DDD/domain, CDD/context,
similar issues, claim/concept verification, standards search.
4. compatibility-and-naming — API compatibility, breaking-change/backcompat,
Expand Down Expand Up @@ -104,10 +104,15 @@ error/rollback behavior, numerical extremes, or mobile/accessibility behavior
as applicable. A green check or absence of a known bug is not a probe. Record
the exact changed path, positive line, counterexample, executed or source-backed
evidence, exactly one `source-line-sha256=<64 lowercase hex>` digest of the cited
current-head line bytes without its line ending, and whether the hypothesis was falsified or confirmed in the
`adversarial_validation` control field. APPROVE needs two falsified probes for
material code/workflow/config/package/test changes and one for non-code changes;
REQUEST_CHANGES needs a confirmed probe anchored to a published finding.
current-head line bytes without its line ending, and whether the hypothesis was
falsified or confirmed in the `adversarial_validation` control field. APPROVE
needs two falsified probes for material code/workflow/config/package/test
changes and one for non-code changes; REQUEST_CHANGES needs a confirmed probe
anchored to a published finding. For a heuristic review seed (for example
naming, identifier shape, or a peer-bot claim), actively try to falsify the seed
before blocking; the seed itself is never evidence of a defect.

Review-quality false-negative probes must actively attack mutable alias or post-validation mutation, changing getter/Proxy or other TOCTOU behavior, execution/tenant/request identity confusion, stale head/event evidence, substring-only, existence-only, or vacuous test oracles, cross-file or cross-document contract contradiction, internal/external authority boundary overreach, security/reliability state-machine race, and missing causal dependency context when the changed surface can exhibit them. For every candidate defect, record the exact changed source line and causal path, run or trace a disconfirming probe rather than accepting the seed, and classify the result as confirmed defect, falsified/false positive, or NEEDS_INFO. Do not relabel one observation as multiple classes, infer impact from taxonomy alone, or detach a blocker from the source/evidence that demonstrates its trigger and consequence.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📝 Info: Taxonomy cannot establish defects alone

The probe list applies only to relevant changed surfaces. Each blocker still needs a causal path, disconfirming probe, and demonstrated consequence.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


Execution provenance is mandatory. Never claim that React DevTools, Chrome
DevTools, browser DevTools, Playwright, Cypress, or Selenium ran, passed,
Expand Down Expand Up @@ -161,38 +166,37 @@ fallback/legacy or composite case when those paths exist.
Review object naming and reserved-word safety for changed database tables,
columns, primary keys, foreign keys, indexes, constraints, API fields, events,
configuration keys, routes, classes, functions, methods, generated models, and
serialized contracts. Follow local convention, but flag ambiguous single-word
names such as `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`,
or `key` when a two-word snake_case, camelCase, PascalCase, or local-equivalent
name would reduce ORM, SQL reserved-word, serialization, or portability risk.

Identifier exposure and enumeration safety is a security blocker, not a style
note. When a primary key or any identifier that appears in an API response, URL
path or query, redirect, filename, cache key, or other client-visible surface
is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT,
IDENTITY, or an ORM auto-increment `id`), return REQUEST_CHANGES: sequential
ids let attackers enumerate and reach other records (IDOR/enumeration — the
Coupang breach exploited guessable sequential ids). Require a non-sequential,
non-guessable identifier at every exposed boundary — a random UUIDv4 or random
token; treat time-ordered ULID/UUIDv7 as acceptable only when creation-order
leakage is harmless. An internal-only auto-increment key is acceptable solely
when it is never exposed and a separate opaque identifier is used at every
external boundary; when exposure is unclear, treat it as exposed.

Require every newly added or renamed identifier — tables, columns, keys,
indexes, constraints, API fields, event names, config keys, routes, classes,
functions, methods, variables, files, generated models, and serialized
contracts — to be composed of two or more meaningful words, never a bare single
word or reserved word, in the idiomatic case of that file's language:
snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for
JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes
and Go exported names, SCREAMING_SNAKE_CASE for constants; follow the
repository's existing convention where it differs and never force one language's
casing onto another. A single-word or reserved name such as `id`, `data`,
`user`, `type`, `value`, `run`, `handler`, or `temp` is a blocker when a
two-word equivalent such as `order_item_id`, `projectId`, `UserProfile`, or
`parseRequest` is clearer and safer. Short-lived loop indices and idiomatic
single-letter math variables are exempt.
serialized contracts. Follow repository and language conventions. New database
objects are the repository-specific exception: new table, column, primary-key,
foreign-key, index, and constraint names must use at least two words in
snake_case; existing CamelCase/PascalCase database objects are grandfathered and
must not be force-renamed. For every other naming surface, naming is a blocking
finding only when the changed name has a source-backed consequence — for example
a real reserved-word collision, ambiguous serialization or generated code,
incompatible public/API contract, portability break, or security/authority
confusion. Do not infer a defect from a name's word count outside that explicit
new-database-object contract.

Identifier exposure and enumeration deserve adversarial security review, but an
exposed sequential identifier is a signal, not automatic proof of IDOR. Trace
the actual authorization and lookup path. Block when source or execution
evidence shows that predictable identifiers enable unauthorized record access,
cross-tenant discovery, sensitive existence disclosure, or violate an explicit
opaque-identifier contract. Public or properly authorized sequential identifiers
can be acceptable. When exposure or authorization impact is unclear, return a
focused `NEEDS_INFO` item or non-blocking risk note rather than assuming the
identifier is exposed or exploitable. Recommend opaque identifiers only when
they address the demonstrated threat or an explicit product/privacy contract;
they do not substitute for authorization.

For newly added or renamed identifiers, enforce repository conventions,
language idioms, schema/API compatibility, and concrete ambiguity or collision
risks. Short or single-word names are acceptable when idiomatic and unambiguous
outside the explicit new-database-object naming contract; longer names are not
automatically safer. Never turn a lexical word-count rule into review authority.
Any blocking naming finding must cite the exact changed identifier and the
specific consumer, parser, database, serializer, generator, security boundary,
or compatibility behavior it can break.

Use these severity meanings in human-readable findings and in the control
block:
Expand Down
70 changes: 37 additions & 33 deletions code-reviewer-prompt.md
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ hypothesis, attack/counterexample, evidence with exactly one verified
`source-line-sha256=<64 lowercase hex>` digest of that cited current-head line,
and falsified/confirmed outcome in
the workflow's structured `adversarial_validation` control field. Green checks
alone and absence of a known failure are not adversarial evidence.
alone and absence of a known failure are not adversarial evidence. For a
heuristic review seed such as naming, identifier shape, or a peer-bot claim,
actively try to falsify the seed before blocking; the seed itself is never
evidence of a defect.

Review-quality false-negative probes must actively attack mutable alias or post-validation mutation, changing getter/Proxy or other TOCTOU behavior, execution/tenant/request identity confusion, stale head/event evidence, substring-only, existence-only, or vacuous test oracles, cross-file or cross-document contract contradiction, internal/external authority boundary overreach, security/reliability state-machine race, and missing causal dependency context when the changed surface can exhibit them. For every candidate defect, record the exact changed source line and causal path, run or trace a disconfirming probe rather than accepting the seed, and classify the result as confirmed defect, falsified/false positive, or NEEDS_INFO. Do not relabel one observation as multiple classes, infer impact from taxonomy alone, or detach a blocker from the source/evidence that demonstrates its trigger and consequence.

Implementation completeness is mandatory. Inspect changed runtime code and
connected call sites for placeholder bodies such as `pass`, `...`,
Expand All @@ -125,38 +130,37 @@ full-screen blocking layer.
Review object naming and reserved-word safety for changed database tables,
columns, primary keys, foreign keys, indexes, constraints, API fields, events,
configuration keys, routes, classes, functions, methods, generated models, and
serialized contracts. Follow local convention, but flag ambiguous single-word
names such as `id`, `name`, `type`, `value`, `data`, `user`, `order`, `group`,
or `key` when a two-word snake_case, camelCase, PascalCase, or local-equivalent
name would reduce ORM, SQL reserved-word, serialization, or portability risk.

Identifier exposure and enumeration safety is a security blocker, not a style
note. When a primary key or any identifier that appears in an API response, URL
path or query, redirect, filename, cache key, or other client-visible surface
is a sequential or auto-incrementing integer (SERIAL/BIGSERIAL, AUTO_INCREMENT,
IDENTITY, or an ORM auto-increment `id`), flag it as a blocker: sequential ids
let attackers enumerate and reach other records (IDOR/enumeration — the Coupang
breach exploited guessable sequential ids). Require a non-sequential,
non-guessable identifier at every exposed boundary — a random UUIDv4 or random
token; treat time-ordered ULID/UUIDv7 as acceptable only when creation-order
leakage is harmless. An internal-only auto-increment key is acceptable solely
when it is never exposed and a separate opaque identifier is used at every
external boundary; when exposure is unclear, treat it as exposed.

Require every newly added or renamed identifier — tables, columns, keys,
indexes, constraints, API fields, event names, config keys, routes, classes,
functions, methods, variables, files, generated models, and serialized
contracts — to be composed of two or more meaningful words, never a bare single
word or reserved word, in the idiomatic case of that file's language:
snake_case for Python/Ruby/Rust/SQL and DB columns, camelCase for
JavaScript/TypeScript/Java/Kotlin/Swift members, PascalCase for types/classes
and Go exported names, SCREAMING_SNAKE_CASE for constants; follow the
repository's existing convention where it differs and never force one language's
casing onto another. A single-word or reserved name such as `id`, `data`,
`user`, `type`, `value`, `run`, `handler`, or `temp` is a blocker when a
two-word equivalent such as `order_item_id`, `projectId`, `UserProfile`, or
`parseRequest` is clearer and safer. Short-lived loop indices and idiomatic
single-letter math variables are exempt.
serialized contracts. Follow repository and language conventions. New database
objects are the repository-specific exception: new table, column, primary-key,
foreign-key, index, and constraint names must use at least two words in
snake_case; existing CamelCase/PascalCase database objects are grandfathered and
must not be force-renamed. For every other naming surface, naming is a blocking
finding only when the changed name has a source-backed consequence — for example
a real reserved-word collision, ambiguous serialization or generated code,
incompatible public/API contract, portability break, or security/authority
confusion. Do not infer a defect from a name's word count outside that explicit
new-database-object contract.

Identifier exposure and enumeration deserve adversarial security review, but an
exposed sequential identifier is a signal, not automatic proof of IDOR. Trace
the actual authorization and lookup path. Block when source or execution
evidence shows that predictable identifiers enable unauthorized record access,
cross-tenant discovery, sensitive existence disclosure, or violate an explicit
opaque-identifier contract. Public or properly authorized sequential identifiers
can be acceptable. When exposure or authorization impact is unclear, return a
focused `NEEDS_INFO` item or non-blocking risk note rather than assuming the
identifier is exposed or exploitable. Recommend opaque identifiers only when
they address the demonstrated threat or an explicit product/privacy contract;
they do not substitute for authorization.

For newly added or renamed identifiers, enforce repository conventions,
language idioms, schema/API compatibility, and concrete ambiguity or collision
risks. Short or single-word names are acceptable when idiomatic and unambiguous
outside the explicit new-database-object naming contract; longer names are not
automatically safer. Never turn a lexical word-count rule into review authority.
Any blocking naming finding must cite the exact changed identifier and the
specific consumer, parser, database, serializer, generator, security boundary,
or compatibility behavior it can break.

Inspect repository-native execution contracts before choosing verification:
`pyproject`, `tox`/`nox`, GitHub Actions matrices, `package.json`/engines/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# OpenCode review false-positive and false-negative resistance — 2026-09-02

## Finding

The protected central OpenCode prompts had an internal authority contradiction. Their prime directive required source-backed material defects and prohibited style-only blocking findings, but later text made every new or renamed identifier a blocker unless it contained two or more meaningful words. The same section treated an exposed sequential identifier as automatic proof of an IDOR/enumeration defect and instructed the reviewer to assume exposure when that fact was unclear.

Those rules can generate false positives without tracing a consumer, authorization path, serializer, database, generated-code boundary, compatibility contract, or observable security impact. They also turn English lexical shape into review authority, which conflicts with the control plane's evidence-first and hallucination-resistance goals.

A second, opposite failure appeared during live peer review of the repair: after the blanket lexical rule was removed, the prompt said short or single-word names were acceptable without preserving the repository-specific contract for **new database objects**. `docs/product-goal-directive.md` §5 reconciles that rule against `docs/CWL-MASTER-CONTEXT.md` §7: new DB object names require 2+ word `snake_case`, while existing CamelCase/PascalCase DB objects are grandfathered. Devin correctly demonstrated that this cross-document contract could be lost by a locally reasonable prompt rewrite.

## Repair

`ci-review-prompt.md`, `code-reviewer-prompt.md`, and the executable `scripts/ci/opencode_review_prompt_template.md` now use general naming and identifier shape only as adversarial seeds. A reviewer must attempt to falsify a heuristic seed before blocking. Outside the explicit new-DB naming contract, naming becomes blocking only when the exact changed identifier has a source-backed consequence such as a real reserved-word collision, ambiguous serialization/generated code, public-contract incompatibility, portability break, or security/authority confusion.

The three prompt surfaces explicitly preserve the new-DB exception: new table, column, primary-key, foreign-key, index, and constraint names require at least two words in `snake_case`; existing CamelCase/PascalCase DB objects remain grandfathered and must not be force-renamed.

Sequential/exposed identifiers remain a security review signal, but no longer imply IDOR by themselves. The reviewer must trace the actual authorization and lookup path and block only when evidence shows unauthorized access, cross-tenant discovery, sensitive existence disclosure, or violation of an explicit opaque-identifier contract. Properly authorized or intentionally public sequential identifiers can be acceptable. When the exposure or authorization consequence is genuinely unavailable, the prompt requires focused `NEEDS_INFO` or a non-blocking risk note rather than fabricated exploitability.

## Durable false-negative corpus

The review contract now makes recurring externally demonstrated failure classes explicit adversarial targets rather than waiting for peer reviewers to rediscover them. Reviewers must actively probe mutable aliases/post-validation mutation, changing getter/Proxy or other TOCTOU behavior, execution/tenant/request identity confusion, stale head/event evidence, substring-only/existence-only/vacuous test oracles, cross-file or cross-document contract contradictions, internal/external authority overreach, security/reliability state-machine races, and missing causal dependency context.

Each candidate must stay tied to an exact changed source line and causal path, receive a disconfirming probe, and be classified as a confirmed defect, falsified/false positive, or `NEEDS_INFO`. A single observation may not be relabelled as multiple defect classes, and taxonomy alone is never impact evidence.

## Regression

`tests/test_opencode_review_prompt_false_positive_resistance.py` now covers all three prompt surfaces, including the live runtime template. It fails if the prompts restore the blanket lexical blocker, the assume-exposed IDOR rule, the unsupported incident anecdote, lose the evidence-driven authorization/consumer-path contract, erase the new-DB naming exception, or stop naming the durable false-negative probe classes above.

The regression is paragraph-scoped so scattered substrings cannot satisfy the contract. The runtime template also retains current-head and language-evidence authority, and the CI prompt retains its established adversarial probe-count thresholds.

## Review convergence and operating boundary

The external review finding that the runtime template escaped the first regression was repaired before resolution. The later live finding that single-word DB names could bypass organization governance was independently traced to `docs/product-goal-directive.md` §5 / `docs/CWL-MASTER-CONTEXT.md` §7, converted into a regression, repaired on all three prompt surfaces, and only then resolved. A subsequent peer observation that the new false-negative-prefix test had no matching prompt paragraph became obsolete after the GREEN prompt commits and was resolved from exact-head source evidence.

This hardening does not claim benchmark superiority over CodeRabbit or Devin and does not copy proprietary wording. It converts observable peer-review misses into executable local contracts while preserving authorization review, tenant isolation, exact changed-line evidence, adversarial validation, CodeGraph evidence, security checks, and the read-only reviewer sandbox.
Loading
Loading