WIP: Test inline OpenCode review flow #2

lambadalambda · 2026-08-04T11:56:13Z

lambadalambda commented

2026-08-04 11:56:13 +00:00

Purpose

Production smoke test for line-anchored OpenCode review comments. The changed fixture intentionally omits index validation.

Expected

The bot should publish a compact canonical comment and attach the actionable finding to the changed return line.

## Purpose Production smoke test for line-anchored OpenCode review comments. The changed fixture intentionally omits index validation. ## Expected The bot should publish a compact canonical comment and attach the actionable finding to the changed return line.

lambadalambda added 1 commit

2026-08-04 11:56:13 +00:00

test: add inline review fixture

opencode/review OpenCode execution completed

Details

0bee668084

lambadalambda commented

2026-08-04 11:56:23 +00:00

/oc review

opencode-bot commented

2026-08-04 11:59:02 +00:00

OpenCode Review

Execution completed for lambadalambda/opencode-forgejo-bot at 0bee668084ae (job #18).

WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc.

View the pull review with 3 finding(s).

## OpenCode Review _Execution completed for `lambadalambda/opencode-forgejo-bot` at `0bee668084ae` (job #18)._ WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc. [View the pull review with 3 finding(s)](https://git.pleroma.social/lambadalambda/opencode-forgejo-bot/pulls/2#issuecomment-117490).

lambadalambda commented

2026-08-04 12:00:29 +00:00

/oc rerun

opencode-bot reviewed

2026-08-04 12:04:11 +00:00

opencode-bot left a comment

OpenCode Review

WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc.

1 finding(s) are attached to changed lines.

[LOW] New testdata/inline-review-fixture/ directory is referenced by no test or script

A repo-wide search for "inline-review-fixture", "inlinereviewfixture", and "Lookup" finds only the fixture file itself; no Go test, scripts/*.sh, or README references it. Unlike testdata/adversarial-repo (exercised by TestAdversarialFixtureTarContainsOnlySafeFile and scripts/adversarial-check.sh), this fixture currently ships unreferenced. Either wire it into a test (e.g., a snapshot/inline-flow test) or hold the commit until the inline-review-flow harness that consumes it lands.

[LOW] Fixture has no README/comment documenting its purpose and lifecycle

The sibling testdata/adversarial-repo has implicit documentation through its test name and the README's "Adversarial Verification" section. The new inline-review-fixture directory has no README and the .go file has no package/function comment explaining what scenario it exists to exercise or whether the unchecked index is intentional. Add a short README.go-style note (or a package comment) describing the fixture's role so reviewers and future maintainers understand why it is here and whether its contents are expected to be "buggy".

Overall

The change is low-risk in isolation: it adds one fixture file under testdata/, which Go's tooling ignores for build/test, and the existing adversarial-fixture tests confirm the sanitization path correctly drops instruction/config files from similar fixtures. However, as committed the artifact has two real problems worth flagging before merge. (1) Lookup performs values[index] with no bounds check, so any negative, out-of-range, or empty/nil input triggers a runtime panic instead of a safe return; this is a genuine correctness/robustness defect unless it is an intentional bug target for the inline reviewer, in which case the intent should be documented with a comment so future readers and reviewers do not 'fix' or inherit it. (2) The new testdata/inline-review-fixture/ directory is not exercised by any Go test, shell script, or README; nothing in the repo references it, so it currently ships as unreferenced dead weight with no stated purpose or lifecycle. Given the PR title ("Test inline OpenCode review flow") and WIP status, these are likely placeholders, but the review should still surface them. No security, secret, or sanitization-boundary impact was found: the file is a plain regular .go file, contains no credentials, no instruction/config filenames that bypass sanitization, and no symlinks or executable content.

## OpenCode Review WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc. _1 finding(s) are attached to changed lines._ ### [LOW] New testdata/inline-review-fixture/ directory is referenced by no test or script A repo-wide search for "inline-review-fixture", "inlinereviewfixture", and "Lookup" finds only the fixture file itself; no Go test, scripts/*.sh, or README references it. Unlike testdata/adversarial-repo (exercised by TestAdversarialFixtureTarContainsOnlySafeFile and scripts/adversarial-check.sh), this fixture currently ships unreferenced. Either wire it into a test (e.g., a snapshot/inline-flow test) or hold the commit until the inline-review-flow harness that consumes it lands. ### [LOW] Fixture has no README/comment documenting its purpose and lifecycle The sibling testdata/adversarial-repo has implicit documentation through its test name and the README's "Adversarial Verification" section. The new inline-review-fixture directory has no README and the .go file has no package/function comment explaining what scenario it exists to exercise or whether the unchecked index is intentional. Add a short README.go-style note (or a package comment) describing the fixture's role so reviewers and future maintainers understand why it is here and whether its contents are expected to be "buggy". ## Overall The change is low-risk in isolation: it adds one fixture file under testdata/, which Go's tooling ignores for build/test, and the existing adversarial-fixture tests confirm the sanitization path correctly drops instruction/config files from similar fixtures. However, as committed the artifact has two real problems worth flagging before merge. (1) Lookup performs `values[index]` with no bounds check, so any negative, out-of-range, or empty/nil input triggers a runtime panic instead of a safe return; this is a genuine correctness/robustness defect unless it is an intentional bug target for the inline reviewer, in which case the intent should be documented with a comment so future readers and reviewers do not 'fix' or inherit it. (2) The new testdata/inline-review-fixture/ directory is not exercised by any Go test, shell script, or README; nothing in the repo references it, so it currently ships as unreferenced dead weight with no stated purpose or lifecycle. Given the PR title ("Test inline OpenCode review flow") and WIP status, these are likely placeholders, but the review should still surface them. No security, secret, or sanitization-boundary impact was found: the file is a plain regular .go file, contains no credentials, no instruction/config filenames that bypass sanitization, and no symlinks or executable content.

testdata/inline-review-fixture/lookup.go

					
				@ -0,0 +1,5 @@

				package inlinereviewfixture

				func Lookup(values []string, index int) string {

					return values[index]

[MEDIUM] Lookup indexes the slice with no bounds check (panic on empty/nil/out-of-range)

return values[index] panics whenever index < 0, index >= len(values), or values is nil/empty. There is no guard, no error return, and no default value. If this is a deliberate defect for the inline reviewer to flag, add a comment stating that intent so it is not treated as production-safe or silently inherited; otherwise it is a real robustness bug in code now committed to the repository.

Suggested replacement (manual):

func Lookup(values []string, index int) string {
	if index < 0 || index >= len(values) {
		return ""
	}
	return values[index]
}

**[MEDIUM] Lookup indexes the slice with no bounds check (panic on empty/nil/out-of-range)** `return values[index]` panics whenever `index < 0`, `index >= len(values)`, or `values` is nil/empty. There is no guard, no error return, and no default value. If this is a deliberate defect for the inline reviewer to flag, add a comment stating that intent so it is not treated as production-safe or silently inherited; otherwise it is a real robustness bug in code now committed to the repository. **Suggested replacement (manual):** func Lookup(values []string, index int) string { if index < 0 || index >= len(values) { return "" } return values[index] }

opencode-bot reviewed

2026-08-04 12:04:12 +00:00

opencode-bot left a comment

OpenCode Review

WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc.

1 finding(s) are attached to changed lines.

[LOW] New testdata/inline-review-fixture/ directory is referenced by no test or script

A repo-wide search for "inline-review-fixture", "inlinereviewfixture", and "Lookup" finds only the fixture file itself; no Go test, scripts/*.sh, or README references it. Unlike testdata/adversarial-repo (exercised by TestAdversarialFixtureTarContainsOnlySafeFile and scripts/adversarial-check.sh), this fixture currently ships unreferenced. Either wire it into a test (e.g., a snapshot/inline-flow test) or hold the commit until the inline-review-flow harness that consumes it lands.

[LOW] Fixture has no README/comment documenting its purpose and lifecycle

The sibling testdata/adversarial-repo has implicit documentation through its test name and the README's "Adversarial Verification" section. The new inline-review-fixture directory has no README and the .go file has no package/function comment explaining what scenario it exists to exercise or whether the unchecked index is intentional. Add a short README.go-style note (or a package comment) describing the fixture's role so reviewers and future maintainers understand why it is here and whether its contents are expected to be "buggy".

Overall

The change is low-risk in isolation: it adds one fixture file under testdata/, which Go's tooling ignores for build/test, and the existing adversarial-fixture tests confirm the sanitization path correctly drops instruction/config files from similar fixtures. However, as committed the artifact has two real problems worth flagging before merge. (1) Lookup performs values[index] with no bounds check, so any negative, out-of-range, or empty/nil input triggers a runtime panic instead of a safe return; this is a genuine correctness/robustness defect unless it is an intentional bug target for the inline reviewer, in which case the intent should be documented with a comment so future readers and reviewers do not 'fix' or inherit it. (2) The new testdata/inline-review-fixture/ directory is not exercised by any Go test, shell script, or README; nothing in the repo references it, so it currently ships as unreferenced dead weight with no stated purpose or lifecycle. Given the PR title ("Test inline OpenCode review flow") and WIP status, these are likely placeholders, but the review should still surface them. No security, secret, or sanitization-boundary impact was found: the file is a plain regular .go file, contains no credentials, no instruction/config filenames that bypass sanitization, and no symlinks or executable content.

## OpenCode Review WIP PR adds a single 5-line Go fixture at testdata/inline-review-fixture/lookup.go. It lives under testdata/ (excluded from go build) and is gofmt-clean, but its Lookup() indexes a slice with no bounds check and the new directory is referenced by no test, script, or doc. _1 finding(s) are attached to changed lines._ ### [LOW] New testdata/inline-review-fixture/ directory is referenced by no test or script A repo-wide search for "inline-review-fixture", "inlinereviewfixture", and "Lookup" finds only the fixture file itself; no Go test, scripts/*.sh, or README references it. Unlike testdata/adversarial-repo (exercised by TestAdversarialFixtureTarContainsOnlySafeFile and scripts/adversarial-check.sh), this fixture currently ships unreferenced. Either wire it into a test (e.g., a snapshot/inline-flow test) or hold the commit until the inline-review-flow harness that consumes it lands. ### [LOW] Fixture has no README/comment documenting its purpose and lifecycle The sibling testdata/adversarial-repo has implicit documentation through its test name and the README's "Adversarial Verification" section. The new inline-review-fixture directory has no README and the .go file has no package/function comment explaining what scenario it exists to exercise or whether the unchecked index is intentional. Add a short README.go-style note (or a package comment) describing the fixture's role so reviewers and future maintainers understand why it is here and whether its contents are expected to be "buggy". ## Overall The change is low-risk in isolation: it adds one fixture file under testdata/, which Go's tooling ignores for build/test, and the existing adversarial-fixture tests confirm the sanitization path correctly drops instruction/config files from similar fixtures. However, as committed the artifact has two real problems worth flagging before merge. (1) Lookup performs `values[index]` with no bounds check, so any negative, out-of-range, or empty/nil input triggers a runtime panic instead of a safe return; this is a genuine correctness/robustness defect unless it is an intentional bug target for the inline reviewer, in which case the intent should be documented with a comment so future readers and reviewers do not 'fix' or inherit it. (2) The new testdata/inline-review-fixture/ directory is not exercised by any Go test, shell script, or README; nothing in the repo references it, so it currently ships as unreferenced dead weight with no stated purpose or lifecycle. Given the PR title ("Test inline OpenCode review flow") and WIP status, these are likely placeholders, but the review should still surface them. No security, secret, or sanitization-boundary impact was found: the file is a plain regular .go file, contains no credentials, no instruction/config filenames that bypass sanitization, and no symlinks or executable content.

testdata/inline-review-fixture/lookup.go

					
				@ -0,0 +1,5 @@

				package inlinereviewfixture

				func Lookup(values []string, index int) string {

					return values[index]

[MEDIUM] Lookup indexes the slice with no bounds check (panic on empty/nil/out-of-range)

return values[index] panics whenever index < 0, index >= len(values), or values is nil/empty. There is no guard, no error return, and no default value. If this is a deliberate defect for the inline reviewer to flag, add a comment stating that intent so it is not treated as production-safe or silently inherited; otherwise it is a real robustness bug in code now committed to the repository.

Suggested replacement (manual):

func Lookup(values []string, index int) string {
	if index < 0 || index >= len(values) {
		return ""
	}
	return values[index]
}

**[MEDIUM] Lookup indexes the slice with no bounds check (panic on empty/nil/out-of-range)** `return values[index]` panics whenever `index < 0`, `index >= len(values)`, or `values` is nil/empty. There is no guard, no error return, and no default value. If this is a deliberate defect for the inline reviewer to flag, add a comment stating that intent so it is not treated as production-safe or silently inherited; otherwise it is a real robustness bug in code now committed to the repository. **Suggested replacement (manual):** func Lookup(values []string, index int) string { if index < 0 || index >= len(values) { return "" } return values[index] }

lambadalambda closed this pull request

2026-08-04 12:10:16 +00:00

lambadalambda referenced this pull request

2026-08-04 12:10:43 +00:00

Add disposable inline OpenCode reviews #3

opencode/review OpenCode execution completed

Details

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lambadalambda/opencode-forgejo-bot!2

Rows
Columns