From de88efee4dce14f6e95fb7e83461feea6f5c8351 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 21 Aug 2026 09:03:21 -0400 Subject: [PATCH 1/3] Add evidence framework for screenshots in PRs and Jira issues A visible change should ship with a picture, and the repo already has the hard half: headless Skia capture for Avalonia, MCP capture for WinForms, and an Output/ManualEvidence naming convention. What was missing is the last mile -- trimming, captioning, provenance labelling, and any route at all from a PNG to a PR body or a ticket. The rule evidence.md turns on is that the test is the evidence and the screenshot is the courtesy. A control-level headless capture is not a screenshot of the product, and every image now says which it is. Publishing probes three routes and reports the one it used: native gh attach once it ships, gh image where a session cookie is reachable, and an orphan evidence branch with sha-pinned raw URLs otherwise. Only the third works under an Actions token, which is why it stays. jira_add_attachment uploads to Jira. It drives client.session directly because AtlassianClient.post sends JSON only, while multipart needs the XSRF header and no Content-Type. Its validation paths are exercised; the upload path has not yet been run against a live issue. A screenshot of a live project is a data disclosure exactly as a project file is, so the permission gate covers both. Co-Authored-By: Claude Opus 5 --- .claude/references/evidence.md | 140 ++++++++++++++++++ .claude/skills/atlassian-skills/SKILL.md | 9 ++ .../scripts/jira_attachments.py | 139 +++++++++++++++++ .../skills/fieldworks-avalonia-ui/SKILL.md | 23 +++ .claude/skills/jira-issue/SKILL.md | 4 +- .../skills/jira-issue/references/publish.md | 19 ++- .claude/skills/pr-pitch/SKILL.md | 12 ++ 7 files changed, 343 insertions(+), 3 deletions(-) create mode 100644 .claude/references/evidence.md create mode 100644 .claude/skills/atlassian-skills/scripts/jira_attachments.py diff --git a/.claude/references/evidence.md b/.claude/references/evidence.md new file mode 100644 index 0000000000..65a17db77f --- /dev/null +++ b/.claude/references/evidence.md @@ -0,0 +1,140 @@ +# Screenshots and visual evidence + +Shared reference. Pointed at by `.claude/skills/pr-pitch/SKILL.md`, +`.claude/skills/jira-issue/SKILL.md` and +`.claude/skills/fieldworks-avalonia-ui/SKILL.md`. + +Three stages, and skipping the middle one is the usual failure: +**capture, curate, publish.** + +## The rule that outranks the rest + +**The test is the evidence. The screenshot is the courtesy.** + +A headless capture proves nothing a reviewer can re-run. The assertion does. +So a PR carries the test name *and* the picture, and the picture never carries +a claim the test does not. + +The corollary: **a control-level headless capture is not a screenshot of the +product.** If nothing writes the operation at runtime yet, the image shows a +renderer, not a feature. Say which it is, in the caption, every time. That +distinction evaporates the moment an image is pasted without one. + +## Capture + +| Surface | Who captures | How | +| --- | --- | --- | +| Avalonia | The agent, automated | Headless Skia. See `fieldworks-avalonia-ui/references/visual-snapshot-testing.md` | +| WinForms, live project | The developer | Real scenarios need real data, and real data needs permission | +| WinForms, throwaway project | Either | `fieldworks-winapp/navigation/screenshot-evidence.md`, MCP-driven | + +Captures land in `Output/ManualEvidence//NN-name.png`. That directory +is gitignored, which is correct -- captures are working output, not artifacts. + +## Curate + +An uncaptured curation step is why evidence reads as decoration. Three things: + +**Trim to content.** Renderer captures are mostly background. A 520x180 capture +with content in the top quarter reads as an empty box at thumbnail size, which +is the size it is first seen at in both GitHub and Jira. Crop to the content +bounds plus about 8 pixels. + +**Caption every image.** What to look at, not what it is. "Before" is not a +caption; "Before -- the Lexeme field lists both seh and pt" is. + +**Label provenance in the caption.** One of: headless control-level capture, +live FLEx desktop, or mockup. Never leave it to be inferred. + +Name files `NN-state-subject.png` so they sort into reading order: +`01-before-writing-systems.png`, `02-after-writing-systems.png`. + +## Publish -- GitHub + +Probe for a route in this order and say which one was used. + +**1. `gh --attach`, once it ships.** Native upload on six commands (issue and +PR create, edit, comment), tracked by `github/roadmap#1324`. It uses the +ordinary `gh` token, so no cookie and no committed file. Constraints: write +access required, **Actions tokens excluded** so CI cannot use it, nine file +types, images under 10 MB. Detect it rather than assuming a version: + +```powershell +if ((gh pr comment --help 2>&1 | Out-String) -match '--attach') { "native upload available" } +``` + +**2. `gh image`** (`drogers0/gh-image`, MIT). Drives the web UI's own upload +flow and returns a real `user-attachments` URL. It needs a GitHub **session +cookie**, not the `gh` token: `--token`, `GH_SESSION_TOKEN`, or extraction +from a browser cookie store. Chrome 127 and later encrypt cookies in a way +that defeats extraction on Windows, so a Chrome-only machine will report +`session token is empty`. + +- Check availability with `gh image check-token`, which prints a username. +- **Never run `gh image extract-token` in an agent session.** It prints a + full-account credential to stdout, and stdout becomes conversation context. +- A `user_session` cookie grants complete account access and bypasses 2FA. If + a developer chooses this route, they set `GH_SESSION_TOKEN` in their own + shell before starting the session -- never pasted into a prompt. + +**3. Orphan `evidence` branch.** Always available, no credential, and the only +route in CI. The branch is created once, never merged, and never appears in a +diff: + +```powershell +git switch --orphan evidence +``` + +Reference the file by a **sha-pinned** raw URL so a merged PR's images can +never change under it: + +```markdown +![Before -- Lexeme lists seh and pt (headless capture)](https://raw.githubusercontent.com/sillsdev/FieldWorks//LT-22691d/01-before.png) +``` + +`sillsdev/FieldWorks` is public, so these render for everyone with no auth. +The branch must be protected: deleting it breaks every image in every PR that +ever referenced it. + +## Publish -- Jira + +Jira takes native attachments, which is better than a URL there because they +outlive any branch: + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_attachments import jira_add_attachment +print(jira_add_attachment("LT-22715", ["01-before.png", "02-after.png"])) +'@ +``` + +Then reference them from the description or comment with `!01-before.png!`, +or `!01-before.png|thumbnail!` to keep a long description scannable. Images +belong in the analysis comment unless the picture *is* the bug report. + +## Permission + +**Hard stop, every time, before anything leaves the machine:** + +> Do you have permission to post this? + +A screenshot of a live project is a data disclosure exactly as a sample +project is: vernacular text, speaker names, unpublished lexical data, +community-owned material. Jira attachments are visible to everyone with +project access, and a GitHub attachment on a public repo is public. + +- Never publish a capture the agent found on disk without being told to. +- Agent-captured WinForms evidence comes from a throwaway test project only. +- If permission is unclear, describe the image instead and say in the ticket + that a capture exists but was not attached, so nobody re-asks. + +## Checklist + +- [ ] The claim the image supports is also pinned by a test, or the image is + labelled as the only evidence. +- [ ] Trimmed to content. +- [ ] Captioned with what to look at. +- [ ] Provenance named: headless, live, or mockup. +- [ ] Permission asked and answered before upload. +- [ ] The publish route used is stated, and any URL is sha-pinned. diff --git a/.claude/skills/atlassian-skills/SKILL.md b/.claude/skills/atlassian-skills/SKILL.md index ed05cb53b7..7f44b60592 100644 --- a/.claude/skills/atlassian-skills/SKILL.md +++ b/.claude/skills/atlassian-skills/SKILL.md @@ -281,6 +281,15 @@ from scripts.jira_issues import ( jira_add_comment # Add comment to issue ) +# Attachments live in their own module because they need a multipart POST +from scripts.jira_attachments import ( + jira_add_attachment # Upload one or more files to an issue +) + +# Attachments are visible to everyone who can see the issue. Confirm +# permission before uploading user data -- see .claude/references/evidence.md +jira_add_attachment("LT-22715", ["01-before.png", "02-after.png"]) + # Create issue with full options jira_create_issue( project_key="PROJ", diff --git a/.claude/skills/atlassian-skills/scripts/jira_attachments.py b/.claude/skills/atlassian-skills/scripts/jira_attachments.py new file mode 100644 index 0000000000..e51e8921dd --- /dev/null +++ b/.claude/skills/atlassian-skills/scripts/jira_attachments.py @@ -0,0 +1,139 @@ +"""Jira attachment tools. + +Tools: + - jira_add_attachment: Upload one or more files to an issue + +Attachments need a multipart POST, which AtlassianClient.post cannot do -- +it only sends JSON. This module therefore drives client.session directly, +reusing the client's base URL, auth, SSL setting and error handling. +""" + +import mimetypes +import os +import sys +from pathlib import Path +sys.path.insert(0, str(Path(__file__).parent)) + +from typing import Any, Dict, List, Optional, Union + +from _common import ( + AtlassianCredentials, + get_jira_client, + format_json_response, + format_error_response, + ConfigurationError, + AuthenticationError, + ValidationError, + NotFoundError, + APIError, + NetworkError, +) + +# Jira Data Center's default ceiling. A larger file fails server-side with a +# message that does not name the limit, so check it here where we can say so. +DEFAULT_MAX_BYTES = 10 * 1024 * 1024 + + +def jira_add_attachment( + issue_key: str, + file_paths: Union[str, List[str]], + credentials: Optional[AtlassianCredentials] = None, + max_bytes: int = DEFAULT_MAX_BYTES +) -> str: + """Attach one or more files to a Jira issue. + + Args: + issue_key: Issue key (e.g., 'LT-22715') + file_paths: A path, or a list of paths, to upload + credentials: Optional AtlassianCredentials for Agent environments. + If not provided, uses environment variables. + max_bytes: Reject any file larger than this before uploading + + Returns: + JSON string with one entry per attachment, each carrying id, + filename, size and the content URL, or error information + + Note: + Attachments are visible to everyone who can see the issue. Confirm + permission to publish before calling this with user data -- + FieldWorks projects, screenshots of live data, and logs frequently + contain unpublished language material. + """ + handles = [] + try: + client = get_jira_client(credentials) + + if not issue_key: + raise ValidationError('issue_key is required') + if not file_paths: + raise ValidationError('at least one file path is required') + + if isinstance(file_paths, str): + file_paths = [file_paths] + + for path in file_paths: + if not os.path.isfile(path): + raise ValidationError(f'file not found: {path}') + size = os.path.getsize(path) + if size == 0: + raise ValidationError(f'file is empty: {path}') + if size > max_bytes: + raise ValidationError( + f'file is {size} bytes, over the {max_bytes} byte limit: {path}' + ) + + files = [] + for path in file_paths: + name = os.path.basename(path) + mime = mimetypes.guess_type(name)[0] or 'application/octet-stream' + handle = open(path, 'rb') + handles.append(handle) + files.append(('file', (name, handle, mime))) + + url = f"{client.config.url}{client.api_path(f'issue/{issue_key}/attachments')}" + + # X-Atlassian-Token defeats Jira's XSRF check, which otherwise rejects + # the upload. Content-Type must be cleared so requests can set the + # multipart boundary; the session sets application/json for every + # other call, and a None value here removes it for this one. + response = client.session.post( + url, + files=files, + headers={'X-Atlassian-Token': 'no-check', 'Content-Type': None}, + timeout=120, + verify=client.ssl_verify + ) + client._handle_error(response) + + uploaded: List[Dict[str, Any]] = [] + for item in (response.json() if response.content else []): + uploaded.append({ + 'id': item.get('id', ''), + 'filename': item.get('filename', ''), + 'size': item.get('size', 0), + 'mimeType': item.get('mimeType', ''), + 'content': item.get('content', ''), + 'thumbnail': item.get('thumbnail', '') + }) + + return format_json_response({ + 'issue_key': issue_key, + 'count': len(uploaded), + 'attachments': uploaded + }) + + except ConfigurationError as e: + return format_error_response('ConfigurationError', str(e)) + except AuthenticationError as e: + return format_error_response('AuthenticationError', str(e)) + except ValidationError as e: + return format_error_response('ValidationError', str(e)) + except NotFoundError as e: + return format_error_response('NotFoundError', str(e)) + except (APIError, NetworkError) as e: + return format_error_response(type(e).__name__, str(e)) + except Exception as e: + return format_error_response('UnexpectedError', f'Unexpected error: {str(e)}') + finally: + for handle in handles: + handle.close() diff --git a/.claude/skills/fieldworks-avalonia-ui/SKILL.md b/.claude/skills/fieldworks-avalonia-ui/SKILL.md index 09c5b64e6b..be0bbc9a3a 100644 --- a/.claude/skills/fieldworks-avalonia-ui/SKILL.md +++ b/.claude/skills/fieldworks-avalonia-ui/SKILL.md @@ -173,6 +173,29 @@ Rules specific to dialogs: `../fieldworks-winforms-to-avalonia-migration/references/parity-evidence.md` ยง"Evidence language"). +## Evidence for a PR or a ticket + +Avalonia is the surface where capture is automated, so a visible change ships +with a picture. Do not leave it to the reviewer to imagine the before and +after. + +1. Capture both states from a permanent headless test, not a throwaway + fixture, so the evidence regenerates. `references/visual-snapshot-testing.md` + has the harness. +2. Assert the behaviour deterministically in that same test. **The test is the + evidence; the screenshot is the courtesy.** Keep PNGs as subjective + evidence rather than pixel-golden tests. +3. Trim, caption and label before publishing, then upload by the routes in + `.claude/references/evidence.md`. + +Label every capture **control-level headless**, never "screenshot of FLEx", +unless the product actually drives the code path. When an operation exists but +nothing writes it at runtime yet, say so beside the image; a reader who +assumes otherwise believes a feature has shipped. + +Captures belong in `Output/ManualEvidence//`, which is gitignored. +Do not commit files from `Output`. + ## Handoff Report Avalonia docs consulted, tests run, remaining prototype gaps, diff --git a/.claude/skills/jira-issue/SKILL.md b/.claude/skills/jira-issue/SKILL.md index f06dc240fc..e1b1b0604a 100644 --- a/.claude/skills/jira-issue/SKILL.md +++ b/.claude/skills/jira-issue/SKILL.md @@ -12,6 +12,7 @@ description is length on the screen for every reader, permanently. Keep the description short and put the depth in the first comment. Style contract: `.claude/references/compact-style.md`. Read it first. +Screenshots: `.claude/references/evidence.md`, before publishing any image. ## Phases @@ -23,7 +24,7 @@ Style contract: `.claude/references/compact-style.md`. Read it first. | 2 | Duplicates | Search **before** drafting. Show at most 5 candidates as a table with verdicts | | 3 | Lede | Three labelled lines, **approved before anything else is written**. Max 3 revisions, then ask which line is wrong | | 4 | Body | Track file, plus the budgets in `references/format.md` | -| 5 | Permission | "Do you have permission to post this?" Hard stop before anything leaves the machine | +| 5 | Permission | "Do you have permission to post this?" Hard stop. A screenshot of a live project counts | | 6 | Publish | `references/publish.md` | | 7 | Report | Key, URL, one `Next:` line. Nothing else | | 8 | Start now? | Assign, transition, comment the branch and worktree, hand to `jira-bugfix` at its Step 3 | @@ -91,5 +92,6 @@ wrong one is visible. - [ ] Description 250 words or fewer, ending in one `*Next:*` line. - [ ] Every unknown is a `*Not known:*` line rather than a guess. - [ ] No section emitted that does not apply. +- [ ] Every image trimmed, captioned, and labelled headless / live / mockup. - [ ] The title and the last line alone tell the reader what is wrong and what happens next. diff --git a/.claude/skills/jira-issue/references/publish.md b/.claude/skills/jira-issue/references/publish.md index 0200812320..fe348ec5a7 100644 --- a/.claude/skills/jira-issue/references/publish.md +++ b/.claude/skills/jira-issue/references/publish.md @@ -15,8 +15,23 @@ print(jira_create_issue("LT", "", "Bug", description=desc, '@ ``` -Then `jira_add_comment(key, comment)`, then `jira_add_attachment(key, paths)` -once Phase 5 is answered, then one link per related ticket. +Then `jira_add_comment(key, comment)`, then attachments once Phase 5 is +answered, then one link per related ticket. + +## Attachments + +```powershell +python -c @' +import sys; sys.path.insert(0, ".claude/skills/atlassian-skills/scripts") +from jira_attachments import jira_add_attachment +print(jira_add_attachment("LT-XXXXX", ["01-before.png", "02-after.png"])) +'@ +``` + +Reference them with `!01-before.png!`, or `!01-before.png|thumbnail!` to keep a +long description scannable. Images belong in the analysis comment unless the +picture *is* the report. Trimming, captioning and provenance labelling are in +`.claude/references/evidence.md`; they are not optional. ## Fields that need the custom_fields back door diff --git a/.claude/skills/pr-pitch/SKILL.md b/.claude/skills/pr-pitch/SKILL.md index 579af0abac..c899110402 100644 --- a/.claude/skills/pr-pitch/SKILL.md +++ b/.claude/skills/pr-pitch/SKILL.md @@ -18,6 +18,10 @@ Read `.claude/references/compact-style.md` before writing the pitch. It is the shared style contract for issues and PR bodies, and it is where the banned openers, the five-item list cap and the pre-send check live. +Read `.claude/references/evidence.md` before publishing any screenshot. It +covers trimming, captioning, provenance labelling, the three upload routes, +and the permission question that precedes all of them. + ## What this produces Two artifacts, always together, never one without the other: @@ -157,6 +161,14 @@ Open with the concrete thing, not the framing. A screenshot or GIF if the change is visible; otherwise one sentence naming what a user or caller can now do that they could not before. Never open with "This PR refactors...". +**If the change is visible, a picture is expected, not optional.** For +Avalonia work the capture is automated and there is no excuse for its absence; +for WinForms work the author supplies it. Every image is trimmed, captioned +with what to look at, and labelled with its provenance -- a headless +control-level capture is not a screenshot of the product, and a body that +blurs the two misleads the reviewer about what has been proven. The image +supports the claim; the test pins it. See `.claude/references/evidence.md`. + ### 2. The unknown the reviewer starts with (one paragraph) State the question the reviewer will actually have on opening a diff this From 0b1a07649e1ce1be6117d9cdf71fa3c3b643cacc Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 21 Aug 2026 11:41:14 -0400 Subject: [PATCH 2/3] Fix a reference path that does not resolve from its own file evidence.md sits in .claude/references/ and pointed at fieldworks-avalonia-ui/references/visual-snapshot-testing.md as if it were a sibling. The file is under .claude/skills/, so the pointer resolved from nowhere. Made absolute from the repo root. Co-Authored-By: Claude Opus 5 --- .claude/references/evidence.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/references/evidence.md b/.claude/references/evidence.md index 65a17db77f..99214d3590 100644 --- a/.claude/references/evidence.md +++ b/.claude/references/evidence.md @@ -24,7 +24,7 @@ distinction evaporates the moment an image is pasted without one. | Surface | Who captures | How | | --- | --- | --- | -| Avalonia | The agent, automated | Headless Skia. See `fieldworks-avalonia-ui/references/visual-snapshot-testing.md` | +| Avalonia | The agent, automated | Headless Skia. See `.claude/skills/fieldworks-avalonia-ui/references/visual-snapshot-testing.md` | | WinForms, live project | The developer | Real scenarios need real data, and real data needs permission | | WinForms, throwaway project | Either | `fieldworks-winapp/navigation/screenshot-evidence.md`, MCP-driven | From d9890cbcd64bec5ff6d967b529c0e6546da358f5 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 21 Aug 2026 11:47:24 -0400 Subject: [PATCH 3/3] Drop the orphan evidence branch from the publish routes The third upload route proposed creating a never-merged orphan branch to host screenshots and referencing them by sha-pinned raw URL. It was speculative, nothing had been built, and it solved a problem lasting one review by putting binaries in history permanently. Route three is now the honest one: when no upload path is available, say which was tried and hand the file to the author to drag in. The skill also states plainly that images are not committed to the repository and no side branch is created to host them. Refs LT-22723 --- .claude/references/evidence.md | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/.claude/references/evidence.md b/.claude/references/evidence.md index 99214d3590..fa7888a724 100644 --- a/.claude/references/evidence.md +++ b/.claude/references/evidence.md @@ -51,7 +51,7 @@ Name files `NN-state-subject.png` so they sort into reading order: ## Publish -- GitHub -Probe for a route in this order and say which one was used. +Try these in order and say which one was used. **1. `gh --attach`, once it ships.** Native upload on six commands (issue and PR create, edit, comment), tracked by `github/roadmap#1324`. It uses the @@ -77,24 +77,17 @@ that defeats extraction on Windows, so a Chrome-only machine will report a developer chooses this route, they set `GH_SESSION_TOKEN` in their own shell before starting the session -- never pasted into a prompt. -**3. Orphan `evidence` branch.** Always available, no credential, and the only -route in CI. The branch is created once, never merged, and never appears in a -diff: +**3. Ask the author to drag it in.** When neither route above is available -- +no `--attach` yet, no session cookie, or a CI run, where the official flag +excludes Actions tokens anyway -- say so and hand the file over. Name the exact +path to drop into the comment box, then splice the returned URL into the body. -```powershell -git switch --orphan evidence -``` - -Reference the file by a **sha-pinned** raw URL so a merged PR's images can -never change under it: - -```markdown -![Before -- Lexeme lists seh and pt (headless capture)](https://raw.githubusercontent.com/sillsdev/FieldWorks//LT-22691d/01-before.png) -``` +That is a real answer, not a failure. An agent that cannot upload should say +which route it tried and stop, rather than inventing somewhere to put the file. -`sillsdev/FieldWorks` is public, so these render for everyone with no auth. -The branch must be protected: deleting it breaks every image in every PR that -ever referenced it. +**Do not commit images to the repository** to work around this, and do not +create a side branch to host them. Both put binaries in history permanently to +solve a problem that lasts one review. ## Publish -- Jira @@ -137,4 +130,4 @@ project access, and a GitHub attachment on a public repo is public. - [ ] Captioned with what to look at. - [ ] Provenance named: headless, live, or mockup. - [ ] Permission asked and answered before upload. -- [ ] The publish route used is stated, and any URL is sha-pinned. +- [ ] The publish route used is stated, including when it was you.