Merge design-diff and design-drift into one design check - #3
Merged
Merged
Conversation
The two workflows answered neighbouring questions — what changed in the design, and whether the code follows it — and both ended up telling the reviewer what to change in code: one guessed from the design diff, the other checked the code. Two agents, two comments, sometimes disagreeing, and every push added another copy of each (PR #2 has five inline comments on the same line). One job now runs one agent. The agent no longer posts: it writes human.md and findings.json, and scripts/publish.mjs publishes. That keeps publishing deterministic, posts the mechanical result even when the agent fails, and makes dedup possible. The summary comment is edited in place; an inline finding is posted only when its key — path, rule and the text of the line — is new, so it survives lines inserted above and the agent rewording itself. Both are found by hidden markers, not by author, because the author depends on the token. Findings outside the diff go into the summary instead of a hack line in the .pen. What runs is decided from what the PR touched (scripts/check-gate.sh). Drafts get only the design diff. Forks get no secrets, so they get the text diff and the regex scan in the job summary; pull_request_target stays out, since the job installs and builds the PR's code. Screens come from the .pen itself on each side (pen-screens.sh), not from a constant that silently left new screens out, and only screens whose render changed get a composition diff and go into the artifact. The app is measured on every page in design/screen-routes.json instead of one fixed URL, which is what finally covers Field / Text. The numbered rules move to DESIGN-SYSTEM §12 as the single list: the rule number is part of the dedup key. The logic lives in scripts and the YAML only chains them, so each step runs locally. Testing that way caught failures that would have been silent in CI: diff under pipefail killing the script with no message, a comparator crash exiting 1 and reading as "found divergences", and Chromium left open after an exception, which would have held the job until its timeout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
The workflow calls it as ./scripts/measure-app.sh, so without the bit the measurement step fails with permission denied before measuring anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
Design check ·
|
claude-code-action exchanges OIDC for the Claude app token only when the workflow file is identical to the default branch's. On a PR that touches the workflow — including the one that adds it — the action exited in two seconds with success and never ran Claude. The verdict caught it (no findings.json), but every workflow change would have gone red for a reason that has nothing to do with the design. The agent never posts, so it does not need the app token. With github_token the action skips the exchange, and the job token exposes nothing a same-repo PR workflow does not already have. id-token: write goes away with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
Design-only change to exercise the design check on a real PR; it is reverted right after the run is analysed. Two layers on purpose: the Chip's horizontal padding goes from $space-3 to $space-4 (a component change) and the Filter Chips row gap on Channel — Videos from $space-2 to $space-3 (a composition change). Neither components/chip.tsx nor the videos page follows, so the check should report both as findings outside the diff and go red. Edited through pen interactive with save(), not by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
On the first real design change the agent opened human.md with its own "### O que mudou no design", and the summary printed the heading twice. The publisher owns that heading, so it drops a leading heading from human.md, and the prompt now asks for no title. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
Reverts 9feb34b. It widened the Chip padding and the Filter Chips gap only to exercise the design check on this PR; the runs on 9feb34b and d5c3a4f reported both as findings outside the diff (rule 10 on components/chip.tsx:9, rule 9 on the videos page gap) and went red, as intended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
The status line read "0 fora do diff (abaixo)" on PR #4, pointing at a list that was not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS
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
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.
Troca
design-diff.ymledesign-drift.ymlpor um workflow só,design-check.yml: um job, um agente, um comentário.Por quê
Os dois workflows respondiam perguntas vizinhas e acabavam dizendo a mesma coisa de dois jeitos. Os dois agentes diziam o que mudar no código — um deduzindo do diff do design, o outro conferindo o código — e às vezes discordavam. E cada push repetia tudo: o PR #2 tem 6 comentários "Design vs
main" e 5 inline na mesma linha do.pen.A parte mecânica duplicada custava ~20s; o custo real estava nos dois agentes (87s e 97–375s nas últimas execuções).
O que muda
human.mddescreve só o design, o que mudar no código sai só como achado verificadofindings.json;scripts/publish.mjspublica — o mecânico sai mesmo se o agente falhar.penChannel — Videos;Channel — Home.pen, de cada lado (pen-screens.sh)/@FullCycle/videosdesign/screen-routes.json— oField / Textpassa a ser medidoDESIGN-SYSTEM.mdO que roda em cada PR sai de
scripts/check-gate.sh: mexer emdesign/pendevouscripts/roda diff e auditoria; mexer só emapp/,components/, no doc ou no mapa de rotas roda só a auditoria.A lógica mora em
scripts/e o YAML só encadeia, então cada passo roda local (§14, "Rodar local"). O fluxo está desenhado emdesign/diagrams/design-check.excalidraw.Como foi testado (local)
.pendomaincontra o dachannel-videos(3 telas novas), com e sem render, e um fixture com Chip e Sign In alterados: a tela que só mudou por componente sai como "composição igual"/signin: 13 componentes comparados contra 12 só com Videos; divergência, 404 e mapa desatualizado conferidosshellcheckeactionlintlimposO que só este PR confirma
Este PR mexe em
scripts/, então roda o pipeline inteiro contra ele mesmo — com o.penigual ao domain, então sem mudança de design e provavelmente sem achado. Ele valida gate, render, medição, agente e a edição do comentário-resumo. O caminho da review com inline novo só roda num PR que tenha achado em linha do diff.Depois do merge, a
channel-videosprecisa: descartar as edições antigas não commitadas em.github/escripts/, apagar os dois diagramas antigos e registrar o Sign In emdesign/screen-routes.json.🤖 Generated with Claude Code
https://claude.ai/code/session_01DaeGveq15N8P4exKh46iaS