Review feedback: attach stack

This PR addresses review feedback across the --attach stack, PRs #14177 through #14184.

1/8 #14177 Report what kind of token is active

Consolidate token type constants

Thread: #14177 (comment)
By: babakks
Commit: e3d0326

Consolidates two identical lists of token prefixes into a single definition. A third copy remains elsewhere to keep this change focused.

Move TODOs to call sites

Thread: #14177 (comment)
By: babakks
Commit: 804304a

Replaces an outdated central TODO comment with individual notes directly at the four locations that still manually inspect tokens.

Detach shared comment

Thread: #14177 (comment)
By: babakks
Commit: 697605b

Detaches a shared comment from a single field so it reads as a note for the group.

Remove redundant tests

Threads: #14177 (comment) + #14177 (comment)
By: babakks
Commit: 06b86ae

Removes two tests that evaluate standard library behavior. The compiler and other tests already catch the faults they target.

Remove test reference

Thread: #14177 (comment)
By: babakks
Commit: bfffa55

Removes a comment line that references a specific test.

2/8 #14178 Describe and validate a file gh can attach

Correct package documentation name

Thread: #14178 (comment)
By: Copilot
Commit: 89ca08a

Corrects a stale name in a package documentation comment.

Correct stale video test comment

Thread: #14178 (comment)
By: Copilot
Commit: 51b3aa2

Rewrites an inaccurate test comment to correctly explain why video attachments retain their filenames.

Refactor validation types and update size limit message

Threads: #14178 (comment) + #14178 (comment) + #14178 (comment) + #14178 (comment)
By: babakks and Copilot
Commit: 7f5be71

This simplifies the internal abstraction by moving size validation into the constructors and separating data from its accessors. The file size error message now accurately states the limit is inclusive and formats the byte value with a shared helper, producing a slightly different decimal display.

Validate rendered image markdown

Thread: #14178 (comment)
By: babakks
Commit: f0b5f18

Parses the assembled image markdown during validation to confirm it produces exactly one image element with an unchanged destination. If the text creates any unexpected structure, the attachment is refused before the upload begins.

Remove duplicate test cases

Thread: #14178 (comment)
By: babakks
Commit: d038799

Removes the duplicate test flagged in review and a second identical case.

3/8 #14179 Rewrite markdown references to uploaded assets

Document unrewritable reference limitation

Thread: #14179 (comment)
By: Copilot
Commit: 27dd72f

Adds a tracking comment to document a known limitation where complex reference definitions are skipped and remain as local paths. The existing behaviour is preserved because alternative fixes introduce worse problems.

Remote link detection

Thread: #14179 (comment)
By: Copilot
Commit: cf3bc43

The markdown upload step skips all remote destinations instead of only those containing a colon followed by two slashes. This prevents collisions between uploaded files and mail, telephone, or data links. A local file with a scheme prefix in its name is now treated as a remote link and appended to the document instead of being rewritten inline.

Whitespace in bracketed destinations

Thread: #14179 (comment)
By: Copilot
Commit: 6d27fc9

The attachment feature retains whitespace inside angle brackets as part of the filename. A file with a leading or trailing space in its name now links as expected. A padded link intended for an unpadded filename no longer triggers a rewrite and instead appends the image to the end of the document.

Accidental test placeholder removal

Thread: #14179 (comment)
By: Copilot
Commit: a04dcf4

This removes an accidental placeholder from the test suite. Other cases already cover the same scenario.

Explicit test intent assertions

Thread: #14179 (comment)
Thread: #14179 (comment)
By: babakks
Commit: 47b232e

The modifications document an intentional omission and apply a more precise assertion. The tests now state exactly what they expect.

Golden file test harness

Thread: #14179 (comment)
Thread: #14179 (comment)
By: babakks
Commit: df67f15

The command flag that regenerates test output adopts a specific name to prevent collisions with future testing tools. The suite now includes document references written without a relative prefix to prove they resolve identically to prefixed alternatives.

Obscure test group headings

Thread: #14179 (comment)
By: babakks
Commit: a075e86

The test group headings describe an unwritten structure and are no longer accurate. A heading that requires decoding fails its purpose and is better deleted than rewritten.

Mixed reference ordering tests

Thread: #14179 (comment)
By: babakks
Commit: 56f07af

New tests cover documents containing valid and invalid references to the same video. The relative ordering of these references matters. A specific arrangement exposes a deduplication flaw that escapes all other checks.

Reference link test consolidation

Thread: #14179 (comment)
By: babakks
Commit: 43f3568

A separate scenario for reference style links is removed because the main suite already exercises this behaviour. The central suite also applies stricter assertions to the outcome.

4/8 #14180 Upload an asset to GitHub

Upload transport authentication tests

Thread: #14180 (comment)
By: Copilot
Commit: 6b970a2

New test coverage routes upload requests through the production authentication transport. This proves both GitHub.com and tenant upload hosts normalize to the configured host and receive the configured token.

Upload client follow-up

Thread: #14180 (comment)
By: babakks
Commit: 319a0bd

The upload follow-up now points to the pending API client work that can carry caller-built requests through the shared client. This replaces the stale dependency on a go-gh change.

Permission validation clarity

Thread: #14180 (comment)
Thread: #14180 (comment)
By: babakks
Commit: 91cdea2

The missing-permission error now identifies file attachment as the operation that lacked repository context. The invalid-permission test uses an unmistakable unsupported value.

Upload test readability

Thread: #14180 (comment)
Thread: #14180 (comment)
By: babakks
Commit: 81e3b32

The request test now asserts the complete upload URL, including deterministic query encoding. The error table explains why one response bypasses JSON marshaling without depending on a distant row.

Upload error guidance

Thread: #14180 (comment)
Thread: #14180 (comment)
By: babakks
Commit: 4687c5d

Upload errors now keep server validation details on one line. Rate-limit failures tell users to wait and retry, including the server's retry window when provided.

In-memory upload file tests

Thread: #14180 (comment)
Thread: #14180 (comment)
By: babakks
Commit: 1eaba3c

Upload tests now stub file opening with in-memory readers instead of creating temporary files. The production opener uses the narrow reader interface those tests require.

5/8 #14181 Add the flag and upload behind it

Multi-hash argument coverage

Thread: #14181 (comment)
By: Copilot
Commit: 6dda338

The flag table now covers hashes in filenames and alt text, the longest existing path, and missing-path fallback. It also asserts parsed alt text so the last-to-first scan cannot regress while paths still look correct.

Partial upload contract

Thread: #14181 (comment)
By: Copilot
Commit: ea0e187

The upload API now states that assets are attempted in order and stop at the first failure. It also tells callers to persist returned markdown whenever any upload succeeded, including when the operation returns an error.

Image alt text help

Thread: #14181 (comment)
Thread: #14181 (comment)
By: Copilot, babakks
Commit: 52d5a3f

The attachment flag now labels the hash suffix as image alt text instead of implying videos accept it. The combined path-and-alt format is quoted as one unit.

Conflict helper caller

Thread: #14181 (comment)
By: Copilot
Commit: 6c46619

The conflict helper comment now names the entry point that actually invokes it. This removes a reference to a nonexistent symbol while the helper remains in place.

Attachment argument naming

Thread: #14181 (comment)
By: babakks
Commit: fb6d377

The upload orchestration now names its attachment argument slice after its actual type. This avoids confusion with the distinct markdown reference type.

Empty addition preservation

Thread: #14181 (comment)
By: babakks
Commit: bb63c24

Appending an empty value now returns the source markdown byte-for-byte instead of trimming trailing whitespace. The existing table row now proves that no-op behavior.

6/8 #14182 Add the flag to gh pr comment and gh issue comment

Comment attachment help

Thread: #14182 (comment)
Thread: #14182 (comment)
Thread: #14182 (comment)
Thread: #14182 (comment)
By: Copilot, babakks
Commit: 886af95

The issue and pull request comment help now says attachments alone suppress the interactive prompt. Both commands also show that repeating the flag attaches multiple files without alt text.

Comment table formatting

Thread: #14182 (comment)
By: babakks
Commit: 6abc068

Three adjacent comment pre-run cases now close their structs on separate lines. This matches the surrounding table style and keeps field boundaries scannable.

7/8 #14183 Add the flag to gh pr create and gh pr edit

Permission stub documentation

Thread: #14183 (comment)
By: Copilot
Commit: 49e92c2

The repository-info stub with configurable permissions now documents its relationship to the default helper. This satisfies exported API documentation without restating the parameters.

PR attachment help

Thread: #14183 (comment)
Thread: #14183 (comment)
Thread: #14183 (comment)
Thread: #14183 (comment)
By: babakks
Commit: ae5d443

PR create and edit now document that partial attachment success still writes the pull request, prints its URL, and exits non-zero. Both commands also show multiple files attached by repeating the flag without alt text.

Prompt guard cleanup

Thread: #14183 (comment)
Thread: #14183 (comment)
By: babakks
Commit: 912e411

The PR create and edit tests now rely on unset mock methods to fail if asset errors reach prompting. This removes custom error stubs while preserving the same ordering guard.

8/8 #14184 Add the flag to gh issue create and gh issue edit

Issue attachment help

Thread: #14184 (comment)
Thread: #14184 (comment)
Thread: #14184 (comment)
Thread: #14184 (comment)
By: babakks
Commit: c81d081

Issue create and edit now document that partial attachment success still writes the issue, prints its URL, and exits non-zero. Both commands also show multiple files attached by repeating the flag without alt text.

Issue type validation order

Thread: #14184 (comment)
By: Copilot
Commit: eb9839e

Non-interactive issue edits now resolve the requested issue type before uploading attachments. Invalid types therefore fail without creating an asset that no issue update can reference.

Editor attachment coverage

Thread: #14184 (comment)
By: Copilot
Commit: ac2222a

Issue create now tests an attachment reference returned by the editor and requires the create mutation to receive its uploaded URL. This joins editor input and attachment rewriting in one run path.

Issue prompt guard cleanup

Thread: #14184 (comment)
By: babakks
Commit: cd94f8c

The issue-create permission test now relies on unset mock methods to fail if asset validation reaches prompting. This removes a custom error stub while preserving the same ordering guard.