diff --git a/.claude/references/evidence.md b/.claude/references/evidence.md new file mode 100644 index 0000000000..fa7888a724 --- /dev/null +++ b/.claude/references/evidence.md @@ -0,0 +1,133 @@ +# 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 `.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 | + +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 + +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 +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. 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. + +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. + +**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 + +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, including when it was you. 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