Add visual previews to generated pull requests - #2102
Merged
Conversation
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
Owner
|
/review opus |
Contributor
Author
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
Contributor
Author
🔍 AI Code Review — opusOverall EvaluationThis PR introduces a branch-independent, opt-in "visual preview" policy: repository-level configuration (UI + CLI + API validation + core resolver), an agent prompt addendum, a collector that turns committed ✅ Defensive asset collection — ✅ Credential hygiene in the publisher — ✅ Layered, tested failure recovery — Attachment failures fall back to commit-pinned One PR-introduced regression needs correcting before merge: the "no preview section" path of Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Empty-section path rewrites unrelated PR and comment bodies
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Escape
|
Owner
|
Don't commit the preview files, they can attached to PRs directly • Yes—GitHub CLI 2.99 makes this practical. It supports --attach on both gh pr edit and gh pr comment, producing GitHub-hosted user-attachments URLs without storing media in Git. GitHub’s announcement PR #2102 currently still commits previews because:
I’d change the lifecycle to: Agent generates preview Concretely:
A cleaner later design would expose a worker-managed directory such as PROPR_VISUAL_PREVIEW_DIR inside the agent container, outside the repository from the start. That removes the need to scrub the worktree. One item needs real integration validation: #2102 passes a GitHub App installation token through GH_TOKEN, while GitHub’s documentation explicitly mentions OAuth and classic PAT authentication. I’d test an actual ProPR App upload before relying on it. Also, the current head still contains the review blocker in appendVisualPreviewSection(): its empty-preview path globally normalizes whitespace in unrelated PR/comment bodies. That should be fixed independently. |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 95e0e05 Summary of ChangesImplemented the PR #2102 follow-up without committing changes. Key changes:
Validation passed:
A live GitHub App upload was not performed: the environment has GitHub CLI 2.96.0, not 2.99, and directly modifying PR #2102 is outside the allowed actions. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented the PR #2102 follow-up without committing changes. Key changes: - Preview evidence is collected before commit, copied to `/tmp/propr-previews/<task-id>-*/`, and removed/reset from the worktree. - `.propr/previews` is excluded defensively from `commitChanges()`. - Temporary files are always cleaned up after publication. - PR and comment uploads fetch the resulting GitHub body and reject leaked local paths. - Upload failures now publish text-only explanations; commit-pinned fallbacks were removed. - Fixed `appendVisualPreviewSection()` so empty previews no longer normalize unrelated whitespace. - Updated tests and [visual preview documentation](/tmp/git-processor/worktrees/integry/propr/pr-2102-followup-2026-09-03T11-36-55/docs/docs/features/visual-previews.md). Validation passed: - 26 focused/lifecycle tests - Root and core typechecks - Root and core lint - Root and core builds - `git diff --check` - 146 related helper/git-hook tests A live GitHub App upload was not performed: the environment has GitHub CLI 2.96.0, not 2.99, and directly modifying PR #2102 is outside the allowed actions. PR: #2102 Comment by: @integry (ID: 5525114134) Model: gpt-5.6-sol
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Owner
|
github-advanced-security Bot found potential problems 33 minutes ago function safeTemporaryName(taskId: string): string { Show more details |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 37113e7 Summary of ChangesUpdated visualPreviewService.ts. Replaced the CodeQL-flagged trimming regex with linear index scans while preserving normalization behavior and the 80-character limit. Validation passed:
No commit created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Owner
|
Also upgrade gh to the latest version, here's the release info which is relevant: Release list Attach files when creating or editing an issuegh issue create --attach './repro.png#The error state' Attach files when creating or editing a pull requestgh pr create --attach ./before.png Attach files to commentsgh issue comment 123 --attach ./repro.png For more information see https://gh.io/gh-attach and https://github.blog/changelog/2026-09-01-github-cli-media-in-issues-pull-requests-and-comments/ Worktree support extended to gh issue develop Create a linked branch for an issue and check it out in a worktreegh issue develop 123 --checkout --worktree /path/to/wt-feature |
…ees/integry/propr/pr-2102-followup-2026-09-03T12-28-37/packages/core/src/services/visualPreviewService.ts:217). Updated [visualPreviewService.ts](/tmp/git-processor/worktrees/integry/propr/pr-2102-followup-2026-09-03T12-28-37/packages/core/src/services/visualPreviewService.ts:217). Replaced the CodeQL-flagged trimming regex with linear index scans while preserving normalization behavior and the 80-character limit. Validation passed: - 7 focused visual-preview tests - Core typecheck - Core lint - `git diff --check` No commit created. PR: #2102 Comment by: @integry (ID: 5525753130) Model: gpt-5.6-sol
Contributor
Author
|
ℹ️ Analyzed the follow-up request by @integry Analysis SummaryThe requested upgrade is already present on this PR branch, so no additional edits were needed:
No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit 8d50d3f Summary of ChangesFixed the failed CI check in test/agentImagePreparation.test.ts by using Node’s correct Validation:
🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…st.ts](/tmp/git-processor/worktrees/integry/propr/pr-2102-followup-2026-09-03T15-37-17/test/agentImagePreparation.test.ts:12) by using Node’s correct `namedExports` module-mocking option. Fixed the failed CI check in [test/agentImagePreparation.test.ts](/tmp/git-processor/worktrees/integry/propr/pr-2102-followup-2026-09-03T15-37-17/test/agentImagePreparation.test.ts:12) by using Node’s correct `namedExports` module-mocking option. Validation: - Fast unit suite: 284 passed, 0 failed - Root TypeScript check passed - `git diff --check` passed - No visual preview generated because this test-only change is not user-visible - No commit created, as requested PR: #2102 Comment by: @github-actions[bot] (ID: 5528151996) Model: gpt-5.6-sol
Owner
|
/review opus |
Contributor
Author
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
Contributor
Author
🔍 AI Code Review — opusOverall EvaluationThis PR adds an end-to-end visual-preview capability: per-repository policy (UI/CLI/API/config), an agent prompt contract with a The problem is not the feature; it is the collateral change to the shared browser-session authentication path. Converting the proactive GitHub token refresh from fire-and-forget to an awaited, fatal check makes ✅ Preview media is genuinely kept out of Git — ✅ Uploads are verified rather than assumed — ✅ Credential storage is defensive — AES-256-GCM envelope encryption, explicit rejection of Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F2: 🔴 Proactive refresh now 401s sessions that have no refresh token
F3: 🔴
|
Owner
|
/ultrafix |
Contributor
Author
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
Contributor
Author
|
✅ Applied the requested follow-up changes in commit ea8bbf5
Summary of ChangesImplemented selected findings F1–F3 only.
Validation passed:
No visual previews were generated because these changes have no visual UI impact. No commit was created. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented selected findings F1–F3 only. - F1: Already fixed on this branch; empty sections only remove the placeholder without normalizing body whitespace. - F2: Unexpired sessions without refresh tokens now continue normally; explicit invalidation remains fatal. - F3: Removed invalid `offline_access` GitHub scope. Validation passed: - 16 focused tests - Core and API typechecks - Core and API lint - `git diff --check` No visual previews were generated because these changes have no visual UI impact. No commit was created. PR: #2102 Comment by: @integry (ID: 5533427617) Model: gpt-5.6-sol
Contributor
Author
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThis PR adds repository-scoped visual-preview policies, ephemeral evidence handling, GitHub attachment publication, dedicated encrypted upload credentials, and administrator/CLI configuration. The visible implementation is thoughtfully integrated and well tested, and all authoritative active checks pass. However, two localized correctness issues prevent merge; the review is also partial because the supplied diff explicitly omits ranges. ✅ Ephemeral preview lifecycle — Both initial and follow-up paths prepare preview evidence before committing, exclude ✅ Credential separation — Preview upload credentials are managed through dedicated API/UI paths while installation-token Octokit operations remain separate, with targeted tests covering validation and token non-disclosure. Overall, the PR needs minor but required changes before merge. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F4: 🔴 Branch deletion can disable previews
F5: 🔴 Proactive refresh can block requests
SuggestionsThese are optional follow-ups and are not sent to No suggestions. ScoreThe design and targeted coverage are strong, and current active checks pass, but the repository-policy data-loss case and unbounded authenticated-request stall are merge blockers in changed behavior. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
Contributor
Author
|
✅ Applied the requested follow-up changes in commit a279a3c
Summary of ChangesImplemented only F4 and F5.
Validation passed:
Only the two cited implementation files changed. No visual previews were created because the changes are backend-only. 🤖 Implementation Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented only F4 and F5. - F4: Preserves the prior repository-wide visual-preview policy using enabled-wins semantics when branch entries are deleted or unchanged. - F5: Adds a 20-second timeout to GitHub/Connect token refresh requests; aborts remain `temporarily-unavailable`. Validation passed: - Focused config and authentication tests - Branch-deletion regression check - Timeout/abort regression check - API typecheck and lint - `git diff --check` Only the two cited implementation files changed. No visual previews were created because the changes are backend-only. PR: #2102 Comment by: @propr-ultrafix (ID: 0) Model: gpt-5.6-sol
Contributor
Author
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
Contributor
Author
🔍 AI Code Review — codex:gpt-5.6-solOverall EvaluationThe PR adds repository-level visual-preview policy, secure upload credentials, preview staging/publication, CLI setup integration, and administrator UI controls. The visible implementation is cohesive and well tested, with no verified merge blocker. However, the supplied diff explicitly omits part of the central attachment implementation, so the PR cannot receive final merge approval from this partial review alone. ✅ Credential isolation — Upload credentials are encrypted, excluded from API responses, reject incompatible GitHub App tokens, and keep static PATs separate from browser sessions. ✅ Safe preview lifecycle — Generated media is staged outside the repository, restored before committing, and published through both initial-PR and preview-only follow-up paths. ✅ Backward-compatible policy handling — Legacy repository settings receive safe defaults, branch entries are synchronized, and API, CLI, and UI inputs are normalized and validated. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Restore mocked global fetchThe new Connect-refresh test in ScoreThe reviewed portion is strong and all authoritative head checks pass, but omitted uploader and credential-service ranges create a material verification gap that prevents a complete merge-readiness determination. Score: 7/10 🤖 Review Details
🤖 Review by ProPR |
Contributor
Author
|
|
Full Test Suite ResultsValidation failed during: Full test suite.
View Test Output |
Contributor
Author
|
❌ Failed to apply follow-up changes requested by @github-actions[bot] An error occurred while processing your request: Comment ID: 5537687536✓ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
propr setupimport a compatiblegh auth tokenautomatically when no working preview credential exists; imported static tokens remain isolated from browser sessions.Validation
The pull request remains a draft for review.