Skip to content

Add Loupe Code Review plugin - #25

Open
pallgeuer wants to merge 4 commits into
openai:mainfrom
pallgeuer:add-la-review
Open

pallgeuer wants to merge 4 commits into
openai:mainfrom
pallgeuer:add-la-review

Conversation

@pallgeuer

@pallgeuer pallgeuer commented Sep 29, 2026 •

Copy link
Copy Markdown

Summary

Add Loupe Code Review as an unreleased 0.2.6 community package. The submitted package is based on pallgeuer/la-dev-codex-plugins release v0.5.5 (la-review 0.2.5), then updates the package version and packaged README for this review. The bundled skill and Python runtime scripts remain byte-for-byte unchanged from that release. A coordinated source release is planned after Loupe and Perform are accepted into this community marketplace.

Plugin impact and data boundaries

Users explicitly invoke $la-review:loupe to review a working tree, branch, commit, range, or pull request. The active Codex session captures a verification diff; the local Python runner launches available authenticated codex and/or claude reviewer CLIs. Child reviewers may inspect repository files and command output beyond the captured diff and send context to the user's configured OpenAI or Anthropic provider. Loupe has no hosted service, plugin-owned account, telemetry collector, credential store, or external data store.

The child Codex processes retain workspace-write and Claude retains automatic permission mode. The review-only instruction prohibits intentional repository edits, staging, commits, and dependency installation while allowing local validation and manual tests. These modes are deliberately write-capable because tests can create caches and temporary files; the packaged README now states this boundary without implying filesystem enforcement. The outer runner retains access to normal CLI configuration and session storage under the user's account.

The active session writes review.diff and reviewers.json to a private temporary directory. The JSON includes full reviewer output, errors, and session metadata and is also printed to the active session. The runner closes its intermediate capture files. Claude can retain a session transcript for recovery; the provider CLIs may retain their own normal runtime state. Loupe removes its two named artifacts after an uncomplicated successful review and retains and reports the directory when timeout or diagnosis requires it. Provider and CLI retention follow the user's settings and policies.

Runtime requirements are Codex CLI 0.137.0+, Python 3.6+ with only the standard library for shipped scripts, Bash, Git, jq, and at least one authenticated reviewer CLI. No npm or third-party plugin runtime dependency is added.

Review response

  • Expanded the packaged README with off-machine data flow, child permissions, artifact cleanup, and transcript retention.
  • Kept the established reviewer modes so reviewer-initiated tests remain useful; I would appreciate maintainer assessment of this documented review-only boundary.
  • Added offline tests of the cold copied runner for missing providers/helpers, exact scope forwarding, partial failures, timeout process cleanup, and JSON output retention. The tests use local simulated reviewer commands and no provider credentials or calls.
  • Kept one Loupe master test command and one workflow job. The existing package, catalog, and security checks remain connected to that master.

Verification

  • npm run validate:marketplace — passed.
  • npm run validate — passed.
  • npm run test:la-review — passed, including the new offline runner cases.
  • The complete marketplace dispatcher was attempted locally and reached ReviewOps, where its build stopped because this clean clone lacks that unrelated plugin's development dependency ajv. Upstream CI runs the repository-wide jobs; its existing Autodesk Fusion and ReviewOps dependency-audit failures are outside Loupe's package.
  • The packaged skill and both runtime scripts match source release v0.5.5 byte-for-byte.

No credentials, customer data, private URLs, personal paths, dependency changes, or third-party-notice changes are included.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@pallgeuer

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 29, 2026
@tonyloehr

Copy link
Copy Markdown
Collaborator

Thanks for Loupe. Before merge, please document provider data flows and artifact/transcript retention in the packaged README, align reviewer permissions with the review-only contract, and add offline runner tests for missing tools, scope handling, partial failures, timeouts/cleanup, and output retention. Required CI and final CDE review are still needed.

@pallgeuer

Copy link
Copy Markdown
Author

Thanks for the review. I pushed c46c104 to address the packaged README and offline runner-test requests. The README now explains provider data flow, the child CLIs' permissions and session storage, the active session's diff/JSON artifacts, and success versus diagnostic retention. The new tests exercise missing tools, exact scope forwarding, mixed reviewer outcomes, timeout process cleanup, and exact output-file retention from a cold copied package. The Loupe master suite and marketplace validation pass locally.

On reviewer permissions, I retained the existing codex --sandbox workspace-write and claude --permission-mode auto modes intentionally. Loupe's correctness review relies on running existing and manual tests to verify candidate findings, and those tests can write caches, temporary files, or build outputs. Enforcing read-only tool execution would remove that validation capability. The reviewer prompt still prohibits intentional source edits, staging, commits, and dependency installation. The README now makes clear that this is a behavioral review-only contract, not a filesystem guarantee, and that the outer runner needs the CLIs' normal user-level session storage. This setup has been used in practice for months without a review-workflow issue.

Would you consider that documented boundary acceptable for this contribution? If your review requires a different permission boundary, please identify the required behavior so it can be evaluated without silently reducing the reviewers' ability to run tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants