Add disposable inline OpenCode reviews #3

lambadalambda · 2026-08-04T12:10:43Z

lambadalambda commented

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

Summary

replace the separate sanitized preparer/read-only worker flow with one disposable writable rootless agent using normal coding tools
emit strictly validated structured review results and publish actionable findings as Forgejo inline pull-review comments
keep the canonical issue comment compact, reconcile ambiguous publications, preserve discussed reviews, and resume persisted publication intent after lease recovery
expand bounded public package/test egress and raise the coordinated review timeout budget to 50/55/65 minutes

Verification

go test ./...
go vet ./...
go test -race ./internal/app ./internal/store ./internal/control ./internal/forgejo ./internal/launcher
sh scripts/adversarial-check.sh
dispatcher and worker Podman image builds
production smoke test on PR #2: exact-head inline review publication and unanswered-review replacement on rerun

Deployment

The reviewed worktree is already deployed on ci.pleroma.com; rollback images, launcher binary, and a stopped SQLite backup were preserved.

## Summary - replace the separate sanitized preparer/read-only worker flow with one disposable writable rootless agent using normal coding tools - emit strictly validated structured review results and publish actionable findings as Forgejo inline pull-review comments - keep the canonical issue comment compact, reconcile ambiguous publications, preserve discussed reviews, and resume persisted publication intent after lease recovery - expand bounded public package/test egress and raise the coordinated review timeout budget to 50/55/65 minutes ## Verification - `go test ./...` - `go vet ./...` - `go test -race ./internal/app ./internal/store ./internal/control ./internal/forgejo ./internal/launcher` - `sh scripts/adversarial-check.sh` - dispatcher and worker Podman image builds - production smoke test on PR #2: exact-head inline review publication and unanswered-review replacement on rerun ## Deployment The reviewed worktree is already deployed on `ci.pleroma.com`; rollback images, launcher binary, and a stopped SQLite backup were preserved.

lambadalambda added 2 commits

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

feat: add disposable inline review agent e410a12708

docs: document disposable inline reviews

opencode/review OpenCode execution completed

Details

02292106e3

lambadalambda commented

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

/oc review

opencode-bot commented

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

OpenCode Review

Execution completed for lambadalambda/opencode-forgejo-bot at 69e8a409e573 (job #28).

Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass.

View the pull review with 1 finding(s).

## OpenCode Review _Execution completed for `lambadalambda/opencode-forgejo-bot` at `69e8a409e573` (job #28)._ Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass. [View the pull review with 1 finding(s)](https://git.pleroma.social/lambadalambda/opencode-forgejo-bot/pulls/3#issuecomment-117576).

lambadalambda added 2 commits

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

fix: preserve complete rendered reviews ad469771c3

docs: clarify disposable agent credential risk

opencode/review OpenCode execution completed

Details

f9dee494fa

lambadalambda commented

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

/oc rerun

lambadalambda commented

2026-08-04 13:00:03 +00:00

/oc rerun

lambadalambda added 4 commits

2026-08-04 15:54:23 +00:00

feat: persist bounded review discussions df9c8d057c

feat: expose review discussions over MCP 231bf05cb9

fix: leave margin for publication timeouts 1aa36bba30

docs: describe discussion-aware reviews

opencode/review OpenCode review queued

Details

a767700079

lambadalambda commented

2026-08-04 16:02:07 +00:00

/oc rerun

lambadalambda commented

2026-08-04 16:22:13 +00:00

/oc rerun

lambadalambda added 1 commit

2026-08-04 16:30:26 +00:00

fix: require review discussion intake

opencode/review OpenCode execution completed

Details

0ac9e7a2a8

lambadalambda commented

2026-08-04 16:31:27 +00:00

/oc rerun

lambadalambda added 2 commits

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

fix: harden discussion review follow-ups d2689a6833

docs: document bounded webhook capture

opencode/review OpenCode execution completed

Details

721a6d5e0d

lambadalambda commented

2026-08-05 04:45:08 +00:00

/oc rerun

lambadalambda added 1 commit

2026-08-05 05:40:18 +00:00

fix: harden review follow-up races

opencode/review OpenCode execution failed

Details

1406952094

lambadalambda commented

2026-08-05 05:46:12 +00:00

/oc rerun

lambadalambda commented

2026-08-05 05:54:37 +00:00

/oc rerun

lambadalambda added 1 commit

2026-08-05 07:22:28 +00:00

feat: configure bounded agent resources b82e3252ee

lambadalambda added 1 commit

2026-08-05 07:57:17 +00:00

feat: allow explicit outer resource limits 2709af8ac3

lambadalambda added 1 commit

2026-08-05 08:49:53 +00:00

feat: add rootless agent identity mode

opencode/review OpenCode execution completed

Details

69e8a409e5

lambadalambda commented

2026-08-05 09:56:29 +00:00

/oc review

opencode-bot reviewed

2026-08-05 10:13:19 +00:00

opencode-bot left a comment

OpenCode Review

Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass.

1 finding(s) are attached to changed lines.

Overall

This PR replaces the read-only patch model with a disposable, writable-checkout OpenCode agent whose strictly-validated JSON report the dispatcher publishes as a Forgejo pull review with inline comments plus one compact canonical issue comment. ReviewReport and DiscussionSnapshot are validated strictly (unique keys, exact fields, safe paths, anchored findings, per-item and aggregate size bounds, forbidden-marker rejection, dual-position threads). The launcher resumes publication from the persisted result without a second model call; the in-container preparer checks out the exact head via anonymous HTTPS with hooks/redirects/submodules disabled and verifies advertised refs resolve to the queued full SHAs. Publication is fenced by repeated PR-freshness checks; visible output is rolled back on staleness or generation loss with background-context compensation that survives request cancellation, and inline publication reconciles ambiguous creates, parses the PR diff to confirm each anchored line is inside an added hunk, falls back to the full canonical comment on transient failure, and refuses to leave orphan reviews when cleanup fails. The discussion capture is bounded (2 s nested deadline inside a 4 s webhook budget with a 1.5 s required tail, <=4 review pages, aggregate count/size limits) and binds the snapshot to the queued SHAs via a post-capture freshness re-check. The read-only MCP server is strict (bounded requests/results, exact-field pagination, no mutation tools, no network) and re-validates the snapshot; its child environment clears ZHIPU_API_KEY. The disposable boundary is preserved: random tmpfs workspace, internal-network verification, single read-only discussion mount, random container names with forced cleanup, env-file holding only the model key. The broadened capability set and the model-key exfiltration risk are clearly documented in AGENTS.md and README. The diff parser correctly handles binary/renamed/deleted/no-newline files and rejects malformed hunks; the builder's incremental byte accounting is asserted at finish(). SQLite publication transitions use atomic CAS-style WHERE clauses keyed on state/lease/head/generation. Only one LOW refinement remains; it is not a blocker.

## OpenCode Review Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass. _1 finding(s) are attached to changed lines._ ## Overall This PR replaces the read-only patch model with a disposable, writable-checkout OpenCode agent whose strictly-validated JSON report the dispatcher publishes as a Forgejo pull review with inline comments plus one compact canonical issue comment. ReviewReport and DiscussionSnapshot are validated strictly (unique keys, exact fields, safe paths, anchored findings, per-item and aggregate size bounds, forbidden-marker rejection, dual-position threads). The launcher resumes publication from the persisted result without a second model call; the in-container preparer checks out the exact head via anonymous HTTPS with hooks/redirects/submodules disabled and verifies advertised refs resolve to the queued full SHAs. Publication is fenced by repeated PR-freshness checks; visible output is rolled back on staleness or generation loss with background-context compensation that survives request cancellation, and inline publication reconciles ambiguous creates, parses the PR diff to confirm each anchored line is inside an added hunk, falls back to the full canonical comment on transient failure, and refuses to leave orphan reviews when cleanup fails. The discussion capture is bounded (2 s nested deadline inside a 4 s webhook budget with a 1.5 s required tail, <=4 review pages, aggregate count/size limits) and binds the snapshot to the queued SHAs via a post-capture freshness re-check. The read-only MCP server is strict (bounded requests/results, exact-field pagination, no mutation tools, no network) and re-validates the snapshot; its child environment clears ZHIPU_API_KEY. The disposable boundary is preserved: random tmpfs workspace, internal-network verification, single read-only discussion mount, random container names with forced cleanup, env-file holding only the model key. The broadened capability set and the model-key exfiltration risk are clearly documented in AGENTS.md and README. The diff parser correctly handles binary/renamed/deleted/no-newline files and rejects malformed hunks; the builder's incremental byte accounting is asserted at finish(). SQLite publication transitions use atomic CAS-style WHERE clauses keyed on state/lease/head/generation. Only one LOW refinement remains; it is not a blocker.

internal/launcher/launcher.go

					
				@ -52,0 +56,4 @@

						}

						return nil

					}

					if err := verifyAgentResourceEnforcement(ctx, l.cfg); err != nil {

[LOW] Agent uid/gid identity is not verified in the default required cgroup mode

verifyConfiguredAgentRuntime verifies the agent identity (id -u/id -g equals 1000/1000 for keep-id, or 0/0 for rootless-root) only when AgentCgroupMode is outer. In the default required mode it runs verifyAgentResourceEnforcement alone, and the resource probe script only cats cgroup files (memory.max, memory.swap.max, cpu.max, pids.max) without printing id -u/id -g. cgroup limits are independent of the in-container uid, so a worker image whose primary user is not uid 1000 (or a podman userns regression that silently alters the keep-id mapping) would pass startup verification and run reviews under an unexpected identity. The resource probe container already launches with agentIdentityArgs, so the script could validate identity at negligible cost. The outer path shows the project considers identity worth checking; the default path should too.

Suggested replacement (manual):

// Run verifyAgentIdentity unconditionally (or extend agentResourceProbeScript to also
// printf "$(id -u)\n$(id -g)" and validate it in parseAgentResourceProbe) so the
// required cgroup mode catches a misconfigured keep-id mapping before claiming work.

**[LOW] Agent uid/gid identity is not verified in the default required cgroup mode** verifyConfiguredAgentRuntime verifies the agent identity (id -u/id -g equals 1000/1000 for keep-id, or 0/0 for rootless-root) only when AgentCgroupMode is outer. In the default required mode it runs verifyAgentResourceEnforcement alone, and the resource probe script only cats cgroup files (memory.max, memory.swap.max, cpu.max, pids.max) without printing id -u/id -g. cgroup limits are independent of the in-container uid, so a worker image whose primary user is not uid 1000 (or a podman userns regression that silently alters the keep-id mapping) would pass startup verification and run reviews under an unexpected identity. The resource probe container already launches with agentIdentityArgs, so the script could validate identity at negligible cost. The outer path shows the project considers identity worth checking; the default path should too. **Suggested replacement (manual):** // Run verifyAgentIdentity unconditionally (or extend agentResourceProbeScript to also // printf "$(id -u)\n$(id -g)" and validate it in parseAgentResourceProbe) so the // required cgroup mode catches a misconfigured keep-id mapping before claiming work.

opencode-bot reviewed

2026-08-05 10:13:19 +00:00

opencode-bot left a comment

OpenCode Review

Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass.

1 finding(s) are attached to changed lines.

Overall

This PR replaces the read-only patch model with a disposable, writable-checkout OpenCode agent whose strictly-validated JSON report the dispatcher publishes as a Forgejo pull review with inline comments plus one compact canonical issue comment. ReviewReport and DiscussionSnapshot are validated strictly (unique keys, exact fields, safe paths, anchored findings, per-item and aggregate size bounds, forbidden-marker rejection, dual-position threads). The launcher resumes publication from the persisted result without a second model call; the in-container preparer checks out the exact head via anonymous HTTPS with hooks/redirects/submodules disabled and verifies advertised refs resolve to the queued full SHAs. Publication is fenced by repeated PR-freshness checks; visible output is rolled back on staleness or generation loss with background-context compensation that survives request cancellation, and inline publication reconciles ambiguous creates, parses the PR diff to confirm each anchored line is inside an added hunk, falls back to the full canonical comment on transient failure, and refuses to leave orphan reviews when cleanup fails. The discussion capture is bounded (2 s nested deadline inside a 4 s webhook budget with a 1.5 s required tail, <=4 review pages, aggregate count/size limits) and binds the snapshot to the queued SHAs via a post-capture freshness re-check. The read-only MCP server is strict (bounded requests/results, exact-field pagination, no mutation tools, no network) and re-validates the snapshot; its child environment clears ZHIPU_API_KEY. The disposable boundary is preserved: random tmpfs workspace, internal-network verification, single read-only discussion mount, random container names with forced cleanup, env-file holding only the model key. The broadened capability set and the model-key exfiltration risk are clearly documented in AGENTS.md and README. The diff parser correctly handles binary/renamed/deleted/no-newline files and rejects malformed hunks; the builder's incremental byte accounting is asserted at finish(). SQLite publication transitions use atomic CAS-style WHERE clauses keyed on state/lease/head/generation. Only one LOW refinement remains; it is not a blocker.

## OpenCode Review Disposable writable-checkout inline review pipeline with persisted, MCP-exposed discussion snapshots. All three prior LOW findings are confirmed addressed: diff line bound raised to 8 MiB to match the diff response cap, discussion capture now normalizes one item per append (O(N) total), and enqueue/publication use separate lock classes. Build, go vet, full test suite, and adversarial-check all pass. _1 finding(s) are attached to changed lines._ ## Overall This PR replaces the read-only patch model with a disposable, writable-checkout OpenCode agent whose strictly-validated JSON report the dispatcher publishes as a Forgejo pull review with inline comments plus one compact canonical issue comment. ReviewReport and DiscussionSnapshot are validated strictly (unique keys, exact fields, safe paths, anchored findings, per-item and aggregate size bounds, forbidden-marker rejection, dual-position threads). The launcher resumes publication from the persisted result without a second model call; the in-container preparer checks out the exact head via anonymous HTTPS with hooks/redirects/submodules disabled and verifies advertised refs resolve to the queued full SHAs. Publication is fenced by repeated PR-freshness checks; visible output is rolled back on staleness or generation loss with background-context compensation that survives request cancellation, and inline publication reconciles ambiguous creates, parses the PR diff to confirm each anchored line is inside an added hunk, falls back to the full canonical comment on transient failure, and refuses to leave orphan reviews when cleanup fails. The discussion capture is bounded (2 s nested deadline inside a 4 s webhook budget with a 1.5 s required tail, <=4 review pages, aggregate count/size limits) and binds the snapshot to the queued SHAs via a post-capture freshness re-check. The read-only MCP server is strict (bounded requests/results, exact-field pagination, no mutation tools, no network) and re-validates the snapshot; its child environment clears ZHIPU_API_KEY. The disposable boundary is preserved: random tmpfs workspace, internal-network verification, single read-only discussion mount, random container names with forced cleanup, env-file holding only the model key. The broadened capability set and the model-key exfiltration risk are clearly documented in AGENTS.md and README. The diff parser correctly handles binary/renamed/deleted/no-newline files and rejects malformed hunks; the builder's incremental byte accounting is asserted at finish(). SQLite publication transitions use atomic CAS-style WHERE clauses keyed on state/lease/head/generation. Only one LOW refinement remains; it is not a blocker.

internal/launcher/launcher.go

					
				@ -52,0 +56,4 @@

						}

						return nil

					}

					if err := verifyAgentResourceEnforcement(ctx, l.cfg); err != nil {

[LOW] Agent uid/gid identity is not verified in the default required cgroup mode

verifyConfiguredAgentRuntime verifies the agent identity (id -u/id -g equals 1000/1000 for keep-id, or 0/0 for rootless-root) only when AgentCgroupMode is outer. In the default required mode it runs verifyAgentResourceEnforcement alone, and the resource probe script only cats cgroup files (memory.max, memory.swap.max, cpu.max, pids.max) without printing id -u/id -g. cgroup limits are independent of the in-container uid, so a worker image whose primary user is not uid 1000 (or a podman userns regression that silently alters the keep-id mapping) would pass startup verification and run reviews under an unexpected identity. The resource probe container already launches with agentIdentityArgs, so the script could validate identity at negligible cost. The outer path shows the project considers identity worth checking; the default path should too.

Suggested replacement (manual):

// Run verifyAgentIdentity unconditionally (or extend agentResourceProbeScript to also
// printf "$(id -u)\n$(id -g)" and validate it in parseAgentResourceProbe) so the
// required cgroup mode catches a misconfigured keep-id mapping before claiming work.

**[LOW] Agent uid/gid identity is not verified in the default required cgroup mode** verifyConfiguredAgentRuntime verifies the agent identity (id -u/id -g equals 1000/1000 for keep-id, or 0/0 for rootless-root) only when AgentCgroupMode is outer. In the default required mode it runs verifyAgentResourceEnforcement alone, and the resource probe script only cats cgroup files (memory.max, memory.swap.max, cpu.max, pids.max) without printing id -u/id -g. cgroup limits are independent of the in-container uid, so a worker image whose primary user is not uid 1000 (or a podman userns regression that silently alters the keep-id mapping) would pass startup verification and run reviews under an unexpected identity. The resource probe container already launches with agentIdentityArgs, so the script could validate identity at negligible cost. The outer path shows the project considers identity worth checking; the default path should too. **Suggested replacement (manual):** // Run verifyAgentIdentity unconditionally (or extend agentResourceProbeScript to also // printf "$(id -u)\n$(id -g)" and validate it in parseAgentResourceProbe) so the // required cgroup mode catches a misconfigured keep-id mapping before claiming work.

lambadalambda merged commit ec54e40af8 into main

2026-08-05 10:16:25 +00:00

lambadalambda referenced this pull request from a commit

2026-08-05 10:16:25 +00:00

Merge pull request 'Add disposable inline OpenCode reviews' (#3) from fix/restore-review-tools into main

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!3

Rows
Columns