Skip to content

Commit 7060153

Browse files
refactor(flaky-history-analysis): enhance state management and analysis windows
- Updated the state marker in `flaky-history-analysis.ts` and `flaky-sticky-comment.ts` to use base64 encoding, ensuring safe handling of payloads containing HTML comment sequences. - Adjusted the analysis window configuration in `flaky-history-analysis.ts` and the corresponding workflow file to include a 15-day bucket alongside the existing 7-day and 30-day windows. - Modified the `.gitignore` to include a new generated artifact from the flaky unit test detection workflow. - Added a test comment in `index.test.tsx` to trigger the flaky test detection process.
1 parent 3dafaa9 commit 7060153

5 files changed

Lines changed: 57 additions & 21 deletions

File tree

.github/scripts/flaky-history-analysis.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@ import { dirname, join } from 'path';
3939
type Octokit = ReturnType<typeof getOctokit>;
4040

4141
// Prefix of the hidden state block embedded in the sticky comment body.
42-
// Stage 1 reads this to determine which files need re-analysis.
43-
const STATE_MARKER = '<!-- metamask-flaky-test-detection:state';
42+
// Stage 1 reads this to determine which files need re-analysis. The state
43+
// payload is base64-encoded (see flaky-sticky-comment.ts buildStateBlock), so
44+
// the marker name says so and the base64 alphabet can never contain the `-->`
45+
// sequence that closes the HTML comment.
46+
const STATE_MARKER = '<!-- metamask-flaky-test-detection-metadata=';
4447
// Marker used by Stage 3 to identify the sticky comment (must stay in sync).
4548
const COMMENT_MARKER = '<!-- metamask-flaky-test-detection -->';
4649

@@ -58,7 +61,7 @@ const UNIT_TEST_JOB_PREFIX = 'Unit tests';
5861
// nested windows below, so a failure 5 days ago counts in both windows and
5962
// one 20 days ago counts only in the 30d bucket.
6063
const LOOKBACK_DAYS = 30;
61-
const WINDOWS_DAYS = [7, 30] as const;
64+
const WINDOWS_DAYS = [7, 15, 30] as const;
6265
type WindowKey = `${(typeof WINDOWS_DAYS)[number]}d`;
6366
const WINDOW_KEYS = WINDOWS_DAYS.map((d) => `${d}d` as WindowKey);
6467
type WindowCounts = Record<WindowKey, number>;
@@ -193,14 +196,19 @@ async function mapWithConcurrency<T, R>(
193196
}
194197

195198
// Extracts the per-file state JSON block embedded in a sticky comment body.
199+
// The payload is base64-encoded, so decoding it is safe even if the AI
200+
// findings inside contain a literal `-->` — that sequence cannot occur inside
201+
// base64 output, only in the (correctly detected) closing delimiter.
196202
function parseStateFromComment(body: string): CommentState | null {
197203
const idx = body.indexOf(STATE_MARKER);
198204
if (idx === -1) return null;
199205
const after = body.slice(idx + STATE_MARKER.length).trimStart();
200206
const closeIdx = after.indexOf(' -->');
201207
if (closeIdx === -1) return null;
208+
const encoded = after.slice(0, closeIdx).trim();
202209
try {
203-
return JSON.parse(after.slice(0, closeIdx)) as CommentState;
210+
const json = Buffer.from(encoded, 'base64').toString('utf8');
211+
return JSON.parse(json) as CommentState;
204212
} catch {
205213
return null;
206214
}
@@ -582,4 +590,8 @@ async function main(): Promise<void> {
582590

583591
main().catch((error: Error) => {
584592
core.warning(`Stage 1 failed: ${error.message}`);
593+
// A crash after should_analyze was emitted must not let Stage 3 run against a
594+
// missing history artifact and post a false all-clear.
595+
core.setOutput('should_analyze', 'false');
596+
core.setOutput('files_to_analyze', '');
585597
});

.github/scripts/flaky-sticky-comment.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ import { join } from 'path';
2727
const MARKER = '<!-- metamask-flaky-test-detection -->';
2828
// Prefix of the hidden state block appended to the comment body. Stage 1 reads
2929
// this on the next push to determine which files changed since last analysis.
30-
const STATE_MARKER = '<!-- metamask-flaky-test-detection:state';
30+
// The state payload is base64-encoded (see buildStateBlock below),
31+
// so base64 alphabet can never contain the `-->` sequence that closes the HTML comment.
32+
const STATE_MARKER = '<!-- metamask-flaky-test-detection-metadata=';
3133
// Canonical source of the flaky-test-detection skill. The synced copy at
3234
// .agents/skills/mms-flaky-test-detection/SKILL.md is .gitignore'd, so we link
3335
// to the real source repo (MetaMask/skills) instead of a 404 blob path.
@@ -128,9 +130,12 @@ const DEFAULT_WINDOWS = [7, 15, 30];
128130

129131
// Serializes per-file state into a hidden HTML comment appended to the comment
130132
// body. Stage 1 reads this on the next push to decide which files need
131-
// re-analysis vs. which can be preserved verbatim.
133+
// re-analysis vs. which can be preserved verbatim. The JSON is base64-encoded
134+
// so an AI finding's snippet/explanation/suggestedFix containing a literal
135+
// `-->` cannot truncate the payload before the real closing delimiter.
132136
function buildStateBlock(state: CommentState): string {
133-
return `${STATE_MARKER} ${JSON.stringify(state)} -->`;
137+
const encoded = Buffer.from(JSON.stringify(state), 'utf8').toString('base64');
138+
return `${STATE_MARKER}${encoded} -->`;
134139
}
135140

136141
// Builds the table (or an empty-state fallback line) for the "Run history
@@ -301,6 +306,11 @@ async function main(): Promise<void> {
301306
return;
302307
}
303308

309+
if (!existsSync(HISTORY_PATH)) {
310+
console.log('⏭️ History artifact missing — Stage 1 did not complete, skipping');
311+
return;
312+
}
313+
304314
const [owner, repo] = env.repo.split('/');
305315
const octokit = getOctokit(env.token);
306316

.github/workflows/flaky-unit-test-detection.yml

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#
66
# 1. Deterministic history — flaky-history-analysis.ts counts how often each
77
# modified test file failed on main over the last 30 days of completed
8-
# ci.yml runs, bucketed into 7d/30d windows.
8+
# ci.yml runs, bucketed into 7d/15d/30d windows.
99
# → writes .ai-pr-analyzer/flaky-history.json
1010
#
1111
# 2. AI analyzer — MetaMask/ai-analyzer's flaky-unit-test-analysis mode
@@ -69,8 +69,8 @@ jobs:
6969
command: cd .github/scripts && yarn --immutable
7070

7171
# Stage 1: deterministic historical analysis. Identifies modified unit
72-
# test files and counts their failures on main over the last 365 days of
73-
# completed ci.yml runs, bucketed into 7d/30d/90d/365d windows
72+
# test files and counts their failures on main over the last 30 days of
73+
# completed ci.yml runs, bucketed into 7d/15d/30d windows
7474
# (failure+success runs only; cancelled/timed_out are excluded from the
7575
# denominator). When no modified test file changed since its last-analyzed
7676
# SHA (stored in the sticky comment), the step emits should_analyze=false
@@ -95,12 +95,19 @@ jobs:
9595
# The synced SKILL.md is then copied into .ai-pr-analyzer/skills/ so the
9696
# analyzer's load_skill tool can find it (single source of truth — no
9797
# hand-duplicated pattern content in this repo).
98+
# Pinned to a reviewed commit SHA rather than `main`: this bootstrap runs
99+
# in the same workspace as later token-bearing steps, so executing code
100+
# from a moving branch would let an unreviewed upstream change affect
101+
# this PR's CI run. Bump the SHA deliberately when the skill needs
102+
# updating.
98103
- name: Sync flaky-test-detection skill
99104
if: >-
100105
steps.detect-history.outputs.should_analyze == 'true' &&
101106
!github.event.pull_request.head.repo.fork
107+
env:
108+
SKILLS_BOOTSTRAP_SHA: fad21fb8f6158a8f939e8b30bf8f8af755ce67cd
102109
run: |
103-
curl -fsSL https://raw.githubusercontent.com/MetaMask/skills/main/tools/bootstrap \
110+
curl -fsSL "https://raw.githubusercontent.com/MetaMask/skills/${SKILLS_BOOTSTRAP_SHA}/tools/bootstrap" \
104111
| bash -s -- --repo metamask-mobile --domain coding
105112
mkdir -p .ai-pr-analyzer/skills
106113
cp .agents/skills/mms-flaky-test-detection/SKILL.md \
@@ -134,6 +141,10 @@ jobs:
134141
# test files — the composite action always analyzes the full PR diff.
135142
# This also means the analyzer never has a chance to post its own PR
136143
# comment; Stage 3 below is the only PR-visible output.
144+
# --skip-scope bypasses the analyzer's default minimum-changed-lines
145+
# gate: Stage 1 already narrowed the input to modified unit test files,
146+
# so a 1-2 line edit that introduces a flaky pattern must still be
147+
# analyzed instead of being skipped for being "too small".
137148
- name: Run flaky unit test AI analysis
138149
if: >-
139150
steps.detect-history.outputs.should_analyze == 'true' &&
@@ -149,14 +160,20 @@ jobs:
149160
--config .ai-pr-analyzer \
150161
--mode flaky-unit-test-analysis \
151162
--base-branch "origin/${{ github.base_ref }}" \
152-
--changed-files "${{ steps.detect-history.outputs.files_to_analyze }}"
163+
--changed-files "${{ steps.detect-history.outputs.files_to_analyze }}" \
164+
--skip-scope
153165
154166
# Stage 3: unified sticky PR comment combining Stage 1 + Stage 2
155-
# output. Runs only when should_analyze is true — if Stage 1 determined
156-
# no modified test file changed since last analysis, the existing comment
157-
# is left untouched and Stage 3 is skipped entirely. Never blocks the PR.
167+
# output. Runs when should_analyze is true, or when the PR no longer
168+
# modifies any unit test file — the latter lets the 4-state matrix flip
169+
# a stale sticky comment to "all fixed" when test changes are removed.
170+
# If Stage 1 determined no modified test file changed since last
171+
# analysis (should_analyze=false but has_test_files=true), the existing
172+
# comment is left untouched and Stage 3 is skipped. Never blocks the PR.
158173
- name: Post unified sticky comment
159-
if: steps.detect-history.outputs.should_analyze == 'true'
174+
if: >-
175+
steps.detect-history.outputs.should_analyze == 'true' ||
176+
steps.detect-history.outputs.has_test_files == 'false'
160177
continue-on-error: true
161178
working-directory: .github/scripts
162179
env:

.gitignore

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,8 @@ release-signoffs.json
221221
.claude/skills/
222222
.claude/commands/
223223
.agents/skills/
224-
# Skills copied at CI time by .github/workflows/flaky-unit-test-detection.yml
225-
# from .agents/skills/mms-*/SKILL.md into ai-analyzer's skill lookup path.
226224
.ai-pr-analyzer/skills/*
227-
# Generated artifacts from the flaky-unit-test-detection workflow (Stage 1).
228-
.ai-pr-analyzer/flaky-history.json
229-
.ai-pr-analyzer/flaky-prior-state.json
225+
.ai-pr-analyzer/flaky*
230226
.cursor/rules/*
231227
!.cursor/rules/*.mdc
232228
.cursor/skills/

app/components/Base/RemoteImage/index.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// [mcwp-474-tmp]
12
import React from 'react';
23
import RemoteImage from './';
34
import { getFormattedIpfsUrl } from '@metamask/assets-controllers';

0 commit comments

Comments
 (0)