fix(forge): issue view lists the files attached to an issue and its comments #298

Amateurwizrd · 2026-11-13T20:26:33Z

Amateurwizrd commented

2026-11-13 20:26:33 +00:00

Type: fix · Risk: low · Tested on: Windows 11 (no GPU involved) · Needs: none

Why

A bug report's log is usually an attachment, and forge.py issue view printed only the issue's text, so a report with a log read as if it had none. Fixes #289.

How to reproduce

Seen on: Windows 11, any GPU (a script, no game)
Settings and switches: defaults, signed in with forge.py login
Steps: python -I scripts/forge.py issue view 278, then python -I scripts/forge.py issue view 261 --comments
Expected: both logs on 278, and the six files on 261 (one on the issue, five on a comment), each with a link
Actual: on main, no files are listed at all

What changed

issue view (and --comments) now lists each file attached to the issue and to each comment by name, size and link, without downloading anything. Names and links are escaped so a hostile or odd filename can't break the terminal or the output's lines.

forge: show_attachments() in scripts/forge.py prints attached: "NAME" (SIZE) LINK, the name and link JSON-escaped (the formatting of #300's d39f01dd), so a newline, ESC or CR/LF stays one printable line and a name a cp1252 pipe can't take still prints. cmd_issue calls it for the issue and show_comments() for each comment. It reads the assets list the two GETs already return: no new request.
forge: a file whose link the text already shows is not listed again (what pr comment --attach leaves), so pr view N --comments reads as before for those.
docs: a paragraph under Other commands in docs/forge.md, and the usage line in scripts/forge.py.
tests: test_issue_view_lists_the_attachments and test_attachment_names_and_links_are_escaped in tests/test_forge_pr.py; its stand-in Forgejo now answers GET /issues/N.

Measurements

Verdict: it works: issue 278 lists its 2 logs and 261 its 6 files (0 on main), pr view 292 --comments unchanged, and the two new tests fail on main and pass here.

Hardware: none involved (a script and its test); run on Windows 11 with Python 3 from the builder's tools
Scene: none; the tracker's issues 278 and 261, read with a signed-in session
Method: the command's output on each side, counted with grep -c "^attached:", each command run twice on each side, alternated, with the same count both times (the output does not vary); the new test run on each side
How to reproduce: python -I scripts/forge.py issue view 278, python -I scripts/forge.py issue view 261 --comments, python -I -B tests/test_forge_pr.py

	Before: main @ `9e6f231d`	After: this PR @ `7892cefa`
`issue view 278`: attached files listed	0	2 (both logs)
`issue view 261 --comments`: attached files listed	0	6 (1 on the issue, 5 on a comment)
`pr view 292 --comments` (a comment with 4 pictures put up by `--attach`): attached files listed	0	0
`tests/test_forge_pr.py` with this PR's tests	2 of 34 fail	34 pass

this PR @ 7892cefa, issue view 278 (the lines it adds):

attached: "longshot_log.txt" (13.4 KiB) https://git.magicbean.dev/attachments/3ea47eeb-f712-4feb-9961-60c209b393e9
attached: "longshot_log.1.txt" (10.7 KiB) https://git.magicbean.dev/attachments/9e2be9cc-ccb3-47ff-bbd3-ceb1dbdfd0df

The new tests on main @ 9e6f231d (this PR's tests/test_forge_pr.py with main's scripts/forge.py):

FAIL: test_attachment_names_and_links_are_escaped (__main__.PR.test_attachment_names_and_links_are_escaped)
FAIL: test_issue_view_lists_the_attachments (__main__.PR.test_issue_view_lists_the_attachments)
Ran 34 tests in 0.593s
FAILED (failures=2)

and on this PR:

Ran 34 tests in 1.547s
OK

Flags and settings

None.

Testing

Tested: on Windows 11.

The three commands in the table, against the live tracker, read only, twice each side, alternated.
python -I -B tests/test_forge_pr.py: 34 pass, including the escape test (newline, ESC and CR/LF in a name or link, and a CJK name into a cp1252 pipe, in the issue's rows and a comment's). tests/test_forge_session_lock.py and tests/test_check_pr_description.py pass.
python -I -B tests/run_all.py with CXX=g++ (this PC has no llvm-mingw at the moment): 475 passed, 39 failed, 79 skipped, the same failures as main @ 9e6f231d on this PC, all toolchain or environment checks this PR doesn't touch: test_appimage_player_build.py, test_bake_portability.py, test_control_pipe_security.py, test_cpm_discovery.py, test_cpp_preflight.py, test_crash_dialog.py, test_env_on.py, test_exclusive_marks.py, test_fpx_sticky.py, test_gpu_hang_watch.py, test_lift_cleanup_error.py, test_linux_preflight.py, test_lower_depbar.py, test_lower_lds_after_sts.py, test_lower_register_markers.py, test_make_game_cache.py, test_make_game_prepare.py, test_missing_nup.py, test_native_suatom.py, test_nvn_null_pool.py, test_nvn_tex_builder_guard.py, test_nvn_texture_handle.py, test_oot_state_tap.py, test_options_budget_once.py, test_pipeline_cache_file.py, test_pipeline_play_cache.py, test_portable_builder_home.py, test_portable_builder_startup.py, test_precompile_compute_guard.py, test_prepare_stall.py, test_readback_late.py, test_rt_pool_minimum.py, test_shader_module_guard.py, test_spawn_emit.py, test_split_giants.py, test_tested_configurations.py, test_utf8_paths.py, test_vk_pgo.py, test_walk_key.py.

Not tested: the game (no game code changes). Linux (standard-library Python, no path or console handling). An attachment the signed-in user may not read: the link is printed as the API gives it. A pull request's own attachments: pr view does not list them, before or after.

Must be tested: None.

Risks

What could break: issue view and --comments print extra lines for issues and comments with files; a script parsing that output would see them. Nothing is written or fetched.

Turning it off: revert the commit.

Compatibility: Nothing changes.

Issues

Fixes #289

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched).
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named).
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A game-free test added for the fix, failing on main and passing here.
A check under tests/ that reads changed source updated in this PR, not weakened.
Docs updated.

**Type:** fix · **Risk:** low · **Tested on:** Windows 11 (no GPU involved) · **Needs:** none ## Why A bug report's log is usually an attachment, and `forge.py issue view` printed only the issue's text, so a report with a log read as if it had none. Fixes #289. ## How to reproduce **Seen on:** Windows 11, any GPU (a script, no game) **Settings and switches:** defaults, signed in with `forge.py login` **Steps:** `python -I scripts/forge.py issue view 278`, then `python -I scripts/forge.py issue view 261 --comments` **Expected:** both logs on 278, and the six files on 261 (one on the issue, five on a comment), each with a link **Actual:** on main, no files are listed at all ## What changed `issue view` (and `--comments`) now lists each file attached to the issue and to each comment by name, size and link, without downloading anything. Names and links are escaped so a hostile or odd filename can't break the terminal or the output's lines. - forge: `show_attachments()` in `scripts/forge.py` prints `attached: "NAME" (SIZE) LINK`, the name and link JSON-escaped (the formatting of https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/300's d39f01dd), so a newline, ESC or CR/LF stays one printable line and a name a cp1252 pipe can't take still prints. `cmd_issue` calls it for the issue and `show_comments()` for each comment. It reads the `assets` list the two GETs already return: no new request. - forge: a file whose link the text already shows is not listed again (what `pr comment --attach` leaves), so `pr view N --comments` reads as before for those. - docs: a paragraph under Other commands in `docs/forge.md`, and the usage line in `scripts/forge.py`. - tests: `test_issue_view_lists_the_attachments` and `test_attachment_names_and_links_are_escaped` in `tests/test_forge_pr.py`; its stand-in Forgejo now answers `GET /issues/N`. ## Measurements **Verdict:** it works: issue 278 lists its 2 logs and 261 its 6 files (0 on main), `pr view 292 --comments` unchanged, and the two new tests fail on main and pass here. Hardware: none involved (a script and its test); run on Windows 11 with Python 3 from the builder's tools Scene: none; the tracker's issues 278 and 261, read with a signed-in session Method: the command's output on each side, counted with `grep -c "^attached:"`, each command run twice on each side, alternated, with the same count both times (the output does not vary); the new test run on each side How to reproduce: `python -I scripts/forge.py issue view 278`, `python -I scripts/forge.py issue view 261 --comments`, `python -I -B tests/test_forge_pr.py` | | Before: main @ 9e6f231d | After: this PR @ 7892cefa | |---|---|---| | `issue view 278`: attached files listed | 0 | 2 (both logs) | | `issue view 261 --comments`: attached files listed | 0 | 6 (1 on the issue, 5 on a comment) | | `pr view 292 --comments` (a comment with 4 pictures put up by `--attach`): attached files listed | 0 | 0 | | `tests/test_forge_pr.py` with this PR's tests | 2 of 34 fail | 34 pass | this PR @ 7892cefa, `issue view 278` (the lines it adds): ``` attached: "longshot_log.txt" (13.4 KiB) https://git.magicbean.dev/attachments/3ea47eeb-f712-4feb-9961-60c209b393e9 attached: "longshot_log.1.txt" (10.7 KiB) https://git.magicbean.dev/attachments/9e2be9cc-ccb3-47ff-bbd3-ceb1dbdfd0df ``` The new tests on main @ 9e6f231d (this PR's `tests/test_forge_pr.py` with main's `scripts/forge.py`): ``` FAIL: test_attachment_names_and_links_are_escaped (__main__.PR.test_attachment_names_and_links_are_escaped) FAIL: test_issue_view_lists_the_attachments (__main__.PR.test_issue_view_lists_the_attachments) Ran 34 tests in 0.593s FAILED (failures=2) ``` and on this PR: ``` Ran 34 tests in 1.547s OK ``` ### Flags and settings None. ## Testing **Tested:** on Windows 11. - The three commands in the table, against the live tracker, read only, twice each side, alternated. - `python -I -B tests/test_forge_pr.py`: 34 pass, including the escape test (newline, ESC and CR/LF in a name or link, and a CJK name into a cp1252 pipe, in the issue's rows and a comment's). `tests/test_forge_session_lock.py` and `tests/test_check_pr_description.py` pass. - `python -I -B tests/run_all.py` with `CXX=g++` (this PC has no llvm-mingw at the moment): 475 passed, 39 failed, 79 skipped, the same failures as main @ 9e6f231d on this PC, all toolchain or environment checks this PR doesn't touch: `test_appimage_player_build.py`, `test_bake_portability.py`, `test_control_pipe_security.py`, `test_cpm_discovery.py`, `test_cpp_preflight.py`, `test_crash_dialog.py`, `test_env_on.py`, `test_exclusive_marks.py`, `test_fpx_sticky.py`, `test_gpu_hang_watch.py`, `test_lift_cleanup_error.py`, `test_linux_preflight.py`, `test_lower_depbar.py`, `test_lower_lds_after_sts.py`, `test_lower_register_markers.py`, `test_make_game_cache.py`, `test_make_game_prepare.py`, `test_missing_nup.py`, `test_native_suatom.py`, `test_nvn_null_pool.py`, `test_nvn_tex_builder_guard.py`, `test_nvn_texture_handle.py`, `test_oot_state_tap.py`, `test_options_budget_once.py`, `test_pipeline_cache_file.py`, `test_pipeline_play_cache.py`, `test_portable_builder_home.py`, `test_portable_builder_startup.py`, `test_precompile_compute_guard.py`, `test_prepare_stall.py`, `test_readback_late.py`, `test_rt_pool_minimum.py`, `test_shader_module_guard.py`, `test_spawn_emit.py`, `test_split_giants.py`, `test_tested_configurations.py`, `test_utf8_paths.py`, `test_vk_pgo.py`, `test_walk_key.py`. **Not tested:** the game (no game code changes). Linux (standard-library Python, no path or console handling). An attachment the signed-in user may not read: the link is printed as the API gives it. A pull request's own attachments: `pr view` does not list them, before or after. **Must be tested:** None. ## Risks **What could break:** `issue view` and `--comments` print extra lines for issues and comments with files; a script parsing that output would see them. Nothing is written or fetched. **Turning it off:** revert the commit. **Compatibility:** Nothing changes. ## Issues Fixes #289 ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.

Amateurwizrd added 1 commit

2026-11-13 20:26:34 +00:00

fix(forge): issue view lists the files attached to an issue and its comments

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

dc8d93979f

A bug report's log is an attachment, not text, and issue view printed only the text: a report looked as if it had no
log. It now prints each file of the issue, and with --comments of each comment, as "attached: NAME (SIZE) LINK". The
link is printed and nothing is downloaded. A picture a comment's text already links (pr comment --attach) is not
listed again, so pr view --comments reads as before.

Assisted-by: Claude Fable 5.2
Co-Authored-By: Claude Fable 5.2 <[email protected]>

monsterlag requested changes

2026-11-13 21:18:57 +00:00

monsterlag left a comment

[P2] Escape attachment metadata before printing it. At dc8d9397, an otherwise ASCII issue with a CJK attachment name raises UnicodeEncodeError in a cp1252 pipe at show_attachments, so the command stops instead of displaying the log link. A filename containing newline/ESC and a URL containing CR/LF also reach the terminal unchanged. Two native Windows interleaved pairs of the same guarded CLI fixtures reproduce both behaviors here; the equivalent formatting in PR300 passes both each time. Please add lossless escaping/quoting and regression coverage for both issue and comment attachment rows. PR120's broader UTF-8 stream setup is separate and does not escape terminal controls.

This is a focused display review, not a full-suite or attachment-download claim. The two checks used authored API data and the existing no-network guards; no attachment content was fetched.

This PR and my PR300 overlap on issue289. I have put PR300 back in WIP and preserved its branch/evidence so only one implementation proceeds. This broader PR's comment-attachment coverage is useful; please reuse equivalent safe formatting and boundary tests from commit d39f01dd9d rather than merge both implementations unchanged.

1. [P2] Escape attachment metadata before printing it. At dc8d9397, an otherwise ASCII issue with a CJK attachment name raises UnicodeEncodeError in a cp1252 pipe at show_attachments, so the command stops instead of displaying the log link. A filename containing newline/ESC and a URL containing CR/LF also reach the terminal unchanged. Two native Windows interleaved pairs of the same guarded CLI fixtures reproduce both behaviors here; the equivalent formatting in PR300 passes both each time. Please add lossless escaping/quoting and regression coverage for both issue and comment attachment rows. PR120's broader UTF-8 stream setup is separate and does not escape terminal controls. This is a focused display review, not a full-suite or attachment-download claim. The two checks used authored API data and the existing no-network guards; no attachment content was fetched. This PR and my PR300 overlap on issue289. I have put PR300 back in WIP and preserved its branch/evidence so only one implementation proceeds. This broader PR's comment-attachment coverage is useful; please reuse equivalent safe formatting and boundary tests from commit d39f01dd9da9b3d24a3fb1c127ed49794b7f64c4 rather than merge both implementations unchanged.

scripts/forge.py Outdated

					
				@ -289,0 +293,4 @@

				    for a in item.get("assets") or []:

				        size, link = a.get("size") or 0, a["browser_download_url"]

				        if link not in (item.get("body") or ""):

				            print("attached: %s (%s) %s" % (a["name"], "%d bytes" % size if size < 1024 else "%.1f KiB" % (size / 1024)

Please escape or quote attachment names and URLs before printing. Native guarded cases reproduce UnicodeEncodeError for a CJK name in a cp1252 pipe and raw newline/ESC/CRLF output. PR300 has passing regression coverage and safe formatting to reuse.

mooki added the

kind

bug

label

2026-11-14 01:46:00 +00:00

mooki added the

priority

high

label

2026-11-14 04:18:33 +00:00

APusija04 referenced this pull request

2026-11-14 05:05:21 +00:00

[RX 6600] Game crashes with GPU timeout #365

mooki added the

platform/linux

platform/windows

labels

2026-11-14 18:22:52 +00:00

Amateurwizrd force-pushed fix/forge-issue-view-attachments from dc8d93979f

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

to 7892cefa3d

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Guide screenshots / Guide images and local UI source review (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 15s

Details

2026-11-15 09:15:13 +00:00

Compare

Amateurwizrd commented

2026-11-15 09:17:35 +00:00

Rebased onto main 9e6f231d and took monsterlag's review: attachment names are quoted and both name and link JSON-escaped (the formatting of #300's d39f01dd), so a newline, ESC or CR/LF stays one printable line and a CJK name prints into a cp1252 pipe. New test test_attachment_names_and_links_are_escaped covers issue and comment rows; it fails on main and passes here. Re-measured on the live tracker at 7892cefa: 278 lists 2, 261 --comments lists 6, as before. run_all: no failure that main 9e6f231d doesn't also have on this PC.

Rebased onto main 9e6f231d and took monsterlag's review: attachment names are quoted and both name and link JSON-escaped (the formatting of https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/300's d39f01dd), so a newline, ESC or CR/LF stays one printable line and a CJK name prints into a cp1252 pipe. New test test_attachment_names_and_links_are_escaped covers issue and comment rows; it fails on main and passes here. Re-measured on the live tracker at 7892cefa: 278 lists 2, 261 --comments lists 6, as before. run_all: no failure that main 9e6f231d doesn't also have on this PC.

Amateurwizrd commented

2026-11-15 09:18:11 +00:00