jason-admin/CodeSnuffler-FJ
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

CodeSnuffler Forgejo End-to-End historical review range codesnuffler-forgejo-live-20260630T024007740Z-2fdcb4d5-b11 #48

Closed
jason-admin wants to merge 2 commits from change_forgejo_end_to_end into base_forgejo_end_to_end
pull from: change_forgejo_end_to_end
merge into: jason-admin:base_forgejo_end_to_end
Conversation 3 Commits 2 Files changed 24 +408 -215
jason-admin commented 2026-06-30 02:40:50 +00:00
Owner
Copy link

Exercise the live Forgejo webhook review pipeline against a stable historical CodeSnuffler diff.

Run marker: codesnuffler-forgejo-live-20260630T024007740Z-2fdcb4d5-b11.
Run started at: 2026-06-30T02:40:07.740Z.

Fixture range: f9d210eccd0075b6b39b606cea9b1376719aa062..d8a3fc420b055e04df1d203706b89b1f1261a9c1.
This two-commit AI Harness range stops before 404b386c removed stale persona and old-shape compatibility, so the review is expected to produce actionable findings.

Exercise the live Forgejo webhook review pipeline against a stable historical CodeSnuffler diff. Run marker: codesnuffler-forgejo-live-20260630T024007740Z-2fdcb4d5-b11. Run started at: 2026-06-30T02:40:07.740Z. Fixture range: f9d210eccd0075b6b39b606cea9b1376719aa062..d8a3fc420b055e04df1d203706b89b1f1261a9c1. This two-commit AI Harness range stops before 404b386c removed stale persona and old-shape compatibility, so the review is expected to produce actionable findings.
This completes Step 12 of the AI harness plugin-instance plan by replacing the tool auth container session contract's persona field with explicit plugin instance identity. Controller request and response schemas, auth image refs, build payloads, dev-shell websocket forwarding, Docker labels, container env, and shared probe metadata now carry plugin_id, instance_id, and driver_id.

The controller now rejects mismatched plugin or driver identity, uses PLUGIN_INSTANCE_ID instead of TOOL_PERSONA_ID in auth containers, and labels containers/images with current plugin and driver keys. The app-side auth container client and session helpers send the new payload shape without accepting the old persona_id request field.

Validated with: pytest for the container image controller client, AI tool auth containers, and container image controller integration files; compileall for touched controller/app/contract/test modules; git diff --check; targeted cleanup search for removed tool-auth persona fields, env vars, labels, and probe keys in the edited boundary.
Update the AI harness runtime/status layer to project current plugin instance identity through auth runtime summaries, raw probe normalization, storage inventory, setup details, generic auth helpers, and system report rows. Runtime/status models now carry plugin_id, instance_id, and driver_id without a persona_id field, while later runner and review-policy persona cleanup remains explicitly scoped to follow-up plan steps.

Validated with: pytest for the AI tool instance suite, focused homepage runtime/status cases, the configured harness plugin catalog case, AI tool schema unit tests, compileall for the touched backend/test modules, git diff --check, and targeted persona cleanup searches.
jason-admin commented 2026-06-30 02:40:54 +00:00
Author
Owner
Copy link

CodeSnuffler review banner

CodeSnuffler Review Findings

Recommendation Risk Open findings Files touched by findings
Request Changes Medium 3 3

Findings

1. Dev shell auth still sends the old query parameter

Severity Category Confidence Location Status
Medium Correctness 95% app/dev_shell.py:320 Open

Details for this finding were posted as an inline review comment on the changed line.

Open finding in CodeSnuffler

Fix via CodeSnuffler

2. Old persona API shape remains exposed

Severity Category Confidence Location Status
Medium Maintainability 91% app/settings/ai_review_policy/schemas.py:76 Open

The PR standardizes tool auth identity on instance_id, but the AI review policy choice response still exposes persona_id, and generated frontend types still contain persona_id. This violates the repository's no-compatibility-shims policy for removed/standardized fields and leaves clients on the old shape.

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Replace this response field and related service/frontend usage with instance_id, regenerate API types, and keep only negative tests for rejected persona_id payloads where useful.

3. Compatibility alias still backs runtime storage paths

Severity Category Confidence Location Status
Low Maintainability 88% app/ai_tools/instances.py:226-227 Open

AiToolInstanceConfig.persona_id remains as an alias for instance_id, and runtime paths still call it through storage/workspace code. Because this PR is removing the persona-shaped identity, keeping the alias preserves the old model and makes future cleanup easy to miss.

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Remove the persona_id property and update remaining runtime callers, such as app/runners/ai_workspace.py and app/ai_tools/service.py, to use instance_id directly.

![CodeSnuffler review banner](https://ops.codesnuffler.com/codesnuffler_review/assets/banner_mediumIssues) ## CodeSnuffler Review Findings | Recommendation | Risk | Open findings | Files touched by findings | | --- | --- | ---: | ---: | | Request Changes | Medium | 3 | 3 | ### Findings #### 1. Dev shell auth still sends the old query parameter | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | Medium | Correctness | 95% | `app/dev_shell.py:320` | Open | Details for this finding were posted as an [inline review comment](https://forgejo.codesnuffler.com/jason-admin/CodeSnuffler-FJ/pulls/48#issuecomment-179) on the changed line. [Open finding in CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9) [Fix via CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9/fix) #### 2. Old persona API shape remains exposed | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | Medium | Maintainability | 91% | `app/settings/ai_review_policy/schemas.py:76` | Open | The PR standardizes tool auth identity on `instance_id`, but the AI review policy choice response still exposes `persona_id`, and generated frontend types still contain `persona_id`. This violates the repository's no-compatibility-shims policy for removed/standardized fields and leaves clients on the old shape. [Open finding in CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/10) [Fix via CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/10/fix) **Suggested fix:** Replace this response field and related service/frontend usage with `instance_id`, regenerate API types, and keep only negative tests for rejected `persona_id` payloads where useful. #### 3. Compatibility alias still backs runtime storage paths | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | Low | Maintainability | 88% | `app/ai_tools/instances.py:226-227` | Open | `AiToolInstanceConfig.persona_id` remains as an alias for `instance_id`, and runtime paths still call it through storage/workspace code. Because this PR is removing the persona-shaped identity, keeping the alias preserves the old model and makes future cleanup easy to miss. [Open finding in CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/11) [Fix via CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/11/fix) **Suggested fix:** Remove the `persona_id` property and update remaining runtime callers, such as `app/runners/ai_workspace.py` and `app/ai_tools/service.py`, to use `instance_id` directly.
jason-admin commented 2026-06-30 02:41:03 +00:00
Author
Owner
Copy link

CodeSnuffler review banner

CodeSnuffler Review Findings

Recommendation Risk Open findings Files touched by findings
Request Changes High 3 3

Findings

1. Dev shell auth websocket still sends removed persona_id query

Severity Category Confidence Location Status
High Correctness 96% frontend/src/dev-shell/components/DevShellTerminal.vue:155 Open

dev_shell_websocket now reads instance_id, but the frontend still writes persona_id. Selecting a non-primary plugin instance for a runner auth command will be ignored and the backend will fall back to primary, mounting or mutating the wrong auth storage.

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Send instance_id instead of persona_id and update the prop/name plumbing so selected plugin instances reach /api/dev-shell/ws.

2. AI review policy API still exposes old persona_id shape

Severity Category Confidence Location Status
Medium Correctness 93% app/settings/ai_review_policy/schemas.py:72-77 Open

The PR removes persona_id from AI tool instance/status contracts, but the AI review policy choice schema still requires persona_id and the service populates it from instance_id. This leaves a public API and generated frontend type on the old shape, contrary to the cleanup policy and likely to keep clients depending on the removed field.

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Replace this response field with plugin_id, instance_id, and driver_id as needed, update AiToolInstanceChoice, regenerate frontend API types, and update Vue consumers to use instance_id terminology.

3. Compatibility persona_id alias remains in the core instance model

Severity Category Confidence Location Status
Medium Maintainability 90% app/ai_tools/instances.py:225-227 Open

AiToolInstanceConfig.persona_id remains as a compatibility alias for instance_id, and live code still calls it. The repository instructions explicitly require removing runtime reads, fallbacks, adapters, and compatibility paths when standardizing a field; keeping this property hides stale callers and allows old-shape naming to continue spreading.

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Remove the persona_id property and update remaining callers such as app/runners/ai_workspace.py and app/ai_tools/service.py to use config.instance_id directly. Keep only tests that assert external persona_id payloads are rejected.

![CodeSnuffler review banner](https://providers.codesnuffler.com/codesnuffler_review/assets/banner_majorIssues) ## CodeSnuffler Review Findings | Recommendation | Risk | Open findings | Files touched by findings | | --- | --- | ---: | ---: | | Request Changes | High | 3 | 3 | ### Findings #### 1. Dev shell auth websocket still sends removed persona_id query | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | High | Correctness | 96% | `frontend/src/dev-shell/components/DevShellTerminal.vue:155` | Open | `dev_shell_websocket` now reads `instance_id`, but the frontend still writes `persona_id`. Selecting a non-primary plugin instance for a runner auth command will be ignored and the backend will fall back to `primary`, mounting or mutating the wrong auth storage. [Open finding in CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/7) [Fix via CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/7/fix) **Suggested fix:** Send `instance_id` instead of `persona_id` and update the prop/name plumbing so selected plugin instances reach `/api/dev-shell/ws`. #### 2. AI review policy API still exposes old persona_id shape | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | Medium | Correctness | 93% | `app/settings/ai_review_policy/schemas.py:72-77` | Open | The PR removes `persona_id` from AI tool instance/status contracts, but the AI review policy choice schema still requires `persona_id` and the service populates it from `instance_id`. This leaves a public API and generated frontend type on the old shape, contrary to the cleanup policy and likely to keep clients depending on the removed field. [Open finding in CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/8) [Fix via CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/8/fix) **Suggested fix:** Replace this response field with `plugin_id`, `instance_id`, and `driver_id` as needed, update `AiToolInstanceChoice`, regenerate frontend API types, and update Vue consumers to use `instance_id` terminology. #### 3. Compatibility persona_id alias remains in the core instance model | Severity | Category | Confidence | Location | Status | | --- | --- | ---: | --- | --- | | Medium | Maintainability | 90% | `app/ai_tools/instances.py:225-227` | Open | `AiToolInstanceConfig.persona_id` remains as a compatibility alias for `instance_id`, and live code still calls it. The repository instructions explicitly require removing runtime reads, fallbacks, adapters, and compatibility paths when standardizing a field; keeping this property hides stale callers and allows old-shape naming to continue spreading. [Open finding in CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9) [Fix via CodeSnuffler](https://providers.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9/fix) **Suggested fix:** Remove the `persona_id` property and update remaining callers such as `app/runners/ai_workspace.py` and `app/ai_tools/service.py` to use `config.instance_id` directly. Keep only tests that assert external `persona_id` payloads are rejected.
jason-admin reviewed 2026-06-30 02:42:46 +00:00
app/dev_shell.py
@ -316,3 +318,3 @@
async def dev_shell_websocket(websocket: WebSocket) -> None:
command_id = websocket.query_params.get("command", "bash")
persona_id = websocket.query_params.get("persona_id")
instance_id = websocket.query_params.get("instance_id")
jason-admin commented 2026-06-30 02:42:46 +00:00
Author
Owner
Copy link

CodeSnuffler finding: Dev shell auth still sends the old query parameter

Severity: Medium Category: Correctness

dev_shell_websocket now reads instance_id, but the existing frontend terminal URL still sends persona_id from DevShellTerminal.vue. For non-primary plugin instances, the backend ignores that value and _proxy_runner_auth_session falls back to primary, so interactive auth can write or read the wrong instance storage.

Commit: Open commit d8a3fc420b05

Open finding in CodeSnuffler

Fix via CodeSnuffler

Suggested fix: Rename the frontend prop/query plumbing to instance_id as part of this API change, including DevShellTerminal.vue and its call sites, and add coverage for a non-primary auth shell URL.

**CodeSnuffler finding:** Dev shell auth still sends the old query parameter Severity: **Medium** Category: **Correctness** `dev_shell_websocket` now reads `instance_id`, but the existing frontend terminal URL still sends `persona_id` from `DevShellTerminal.vue`. For non-primary plugin instances, the backend ignores that value and `_proxy_runner_auth_session` falls back to `primary`, so interactive auth can write or read the wrong instance storage. Commit: [Open commit `d8a3fc420b05`](https://forgejo.codesnuffler.com/jason-admin/CodeSnuffler-FJ/commit/d8a3fc420b055e04df1d203706b89b1f1261a9c1) [Open finding in CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9) [Fix via CodeSnuffler](https://ops.codesnuffler.com/codesnuffler_review/forgejo-jason-admin-CodeSnuffler-FJ-pr-48/findings/9/fix) **Suggested fix:** Rename the frontend prop/query plumbing to `instance_id` as part of this API change, including `DevShellTerminal.vue` and its call sites, and add coverage for a non-primary auth shell URL.
jason-admin closed this pull request 2026-06-30 04:18:51 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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
jason-admin/CodeSnuffler-FJ!48
No description provided.