Skip to content

Commit c6635b1

Browse files
authored
update GitHub workflows (#130)
* update GitHub workflows * update steps order * update configs
1 parent 7bbf49f commit c6635b1

6 files changed

Lines changed: 500 additions & 61 deletions

File tree

.github/copilot-instructions.md

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
# Agent-oriented Copilot instructions for PR checks
2+
3+
**Purpose.** Keep only the checks and guidance that an automated coding agent (Copilot-style) can perform reliably during a PR review for a Power BI custom visual repository. Interactive/manual steps live in `HUMAN-certification-checklist.md`.
4+
5+
**Context.** This repository contains a Microsoft custom visual for Power BI. All contributions must follow Microsoft coding standards and Power BI custom visual guidelines. The agent prioritizes checks that enforce those standards and flags deviations for human review.
6+
7+
---
8+
9+
## Summary of agent-capable checks (categories)
10+
11+
- **PR metadata**: non-empty description; conventional commit title.
12+
- **Manifests & capabilities (Power BI)**: presence & schema sanity of `capabilities.json`, `pbiviz.json`, `package.json`, `tsconfig.json`, `src/visual.ts`; no `WebAccess`; version bump rules.
13+
- **Security & forbidden patterns**: unsafe DOM, dynamic scripts, timers-with-strings, `eval/new Function`, network APIs, unsafe bindings.
14+
- **Secrets scanning**: common tokens/keys; urgent human review.
15+
- **Build artifacts & minification & assets**: `.min.*` in `src/`, overly large or minified-looking files.
16+
- **Linting, tests, CI**: scripts present; ESLint config; CI status present if `src/**` changed.
17+
- **Dependencies**: lockfile updated on dependency change; major version bumps flagged.
18+
- **Tests & localization**: unit tests reminder on logic changes; `stringResources/en-US/**` coverage; spellcheck.
19+
- **Documentation & changelog**: `changelog.md` on non-trivial changes; usage examples for public APIs.
20+
- **Code quality & architecture**: scope summary, performance & accessibility hints, state/event cleanup, error handling, maintainability notes.
21+
- **Reporting**: one-line summary counts; per-finding snippets; suggested fixes; auto-labels.
22+
23+
> **Comment limits**: Maximum 20 comments per review. Prioritize by severity levels (error > warning > info) to focus on breaking changes first.
24+
25+
> Maintainers: thresholds, regexes and message templates are the **single source of truth** in this file to avoid divergence.
26+
27+
---
28+
29+
## Detailed rules
30+
31+
### 1) Manifests & capabilities (Power BI)
32+
- **Presence**: `capabilities.json`, `pbiviz.json`, `package.json`, `tsconfig.json`, `src/visual.ts`.
33+
Missing → `error`.
34+
- **Capabilities**:
35+
- No `WebAccess` or privileges that permit arbitrary network calls → `error`.
36+
- `dataRoles` and `dataViewMappings` must be present → `error`.
37+
- **`pbiviz.json`**:
38+
- `visual.version` must bump for functional changes (semver).
39+
- `visual.guid`, `visual.displayName`, `author`, `supportUrl`, `apiVersion` present.
40+
- `apiVersion` major version must match the major version of `@types/powerbi-visuals-api` → mismatch → `warning`.
41+
42+
### 2) Security & forbidden patterns (report file:line)
43+
- Unsafe DOM:
44+
- `innerHTML\s*=``error` with safe alternative.
45+
- `.html\(` (D3 selections) → `error` when D3 imported; otherwise `warning`.
46+
- Dynamic scripts / code eval:
47+
- `createElement\(['"]script['"]\)` / `appendChild` of scripts → `error`.
48+
- `eval\(` or `new Function\(``error`.
49+
- String-based timers:
50+
`set(?:Timeout|Interval)\(\s*(['"]).*?\1``error`.
51+
- Network / runtime:
52+
- `fetch\(`, `XMLHttpRequest`, `WebSocket``error` (Power BI certified visuals constraint).
53+
- Prefer safe APIs:
54+
- `textContent`, `setAttribute` over `innerHTML`. Provide auto-fix snippet if RHS is a simple string literal.
55+
56+
### 3) Secrets & credentials
57+
- Run regex scans on changed text files (exclude binaries and lock files).
58+
- Examples (non-exhaustive):
59+
- `AKIA[0-9A-Z]{16}` (AWS)
60+
- `ghp_[A-Za-z0-9]{36,}` (GitHub)
61+
- `xox[baprs]-[A-Za-z0-9-]{10,48}` (Slack)
62+
- `eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}` (JWT)
63+
- `(AccountKey|SharedAccessKey|SAS|Sig|se=|sp=|sr=|spr=|sv=|st=|sk=|connection\s*string)\s*=\s*[^;'\n]+` (Azure)
64+
- `npm_[A-Za-z0-9]{36,}` (NPM)
65+
- `-----BEGIN (?:RSA |EC |DSA )?PRIVATE KEY-----`
66+
- Any hit → `error` + urgent human review. **Do not auto-edit.**
67+
68+
### 4) Build artifacts, minification & large assets
69+
- `error`: any `\.min\.(js|ts|css)$` under `src/**`.
70+
- `warning`: likely-minified file in `src/**` if **all** of the following apply:
71+
- avg line length > 300 and median > 120,
72+
- **and** (very low whitespace ratio (e.g., < 10% of characters are whitespace) **or** high variable name entropy (e.g., many short, non-dictionary variable names)).
73+
- `warning`: large files in `src/**` > 250 KB (exclude `assets/**` and PBIVIZ icons).
74+
- `warning`: assets > 500 KB — recommend re-evaluating bundling, compression, or CDN prohibition (if applicable).
75+
76+
### 5) Linting, tests
77+
- `package.json` scripts must include:
78+
- `lint`, `test`, `package` (or `pbiviz package`) → missing → `warning`.
79+
- ESLint configuration must exist at repo root:
80+
- Prefer `eslint.config.mjs`; if `.eslintrc.*` or `.eslintignore` or `eslintConfig` in `package.json` -> ask to migrate to `eslint.config.mjs`.
81+
- Missing → `warning` + suggest basic config for Power BI visuals.
82+
83+
### 6) Dependencies
84+
- On `dependencies`/`devDependencies` changes require updated `package-lock.json` or `yarn.lock``warning`.
85+
- Major-bump in `package.json``warning` with request to describe motivation/test-case.
86+
- When adding new features → ensure minor-version is bumped.
87+
- (Optional, as `info`) suggest running `npm audit` (at maintainers' discretion).
88+
89+
### 7) Tests & localization
90+
- If logic touched in `src/**` and no new/updated tests nearby → `warning`-reminder.
91+
- UI strings:
92+
- Check `stringResources/en-US/resources.resjson` and string correspondence from code.
93+
- Missing localization keys → `warning`.
94+
- Spellcheck (en-US as source):
95+
- Report probable typos with level (`info`/`warning`) and replacement suggestion.
96+
- Exclude identifiers/acronyms/brand-names.
97+
98+
### 8) Documentation & changelog
99+
- For non-trivial changes — update `changelog.md``info`/`warning`.
100+
- For new public APIs — add usage examples → `info`.
101+
102+
### 9) Code quality & architecture (senior review mindset)
103+
- Briefly summarize PR purpose and affected areas (render, data, settings, UI).
104+
- Highlight:
105+
- Potential performance bottlenecks (DOM in hot paths, unnecessary loops, re-renders).
106+
- Accessibility (ARIA, contrasts, keyboard navigation, screen reader).
107+
- Errors/edge-cases: null/undefined/empty data.
108+
- Resource management: cleanup D3-selectors, event handlers, timers.
109+
- State/races/leaks; excessive coupling; duplication.
110+
- Power BI SDK/utilities compliance, formatting, API contracts.
111+
112+
## Spellcheck Configuration
113+
114+
### What to Check:
115+
- UI strings in code (`src/**`)
116+
- localization files (`stringResources/en-US/**`)
117+
- PR title and description.
118+
119+
### Severity:
120+
- `warning` — UI strings and localization.
121+
- `info` — PR metadata and comments.
122+
123+
---
124+
125+
## Severity & automated labels
126+
127+
- **error** — must fix before merge (e.g., secrets, `WebAccess`, minified code in `src/**`, forbidden APIs).
128+
- **warning** — should fix soon (e.g., missing PR description/tests, major dep bump, large assets).
129+
- **info** — suggestions/style (typos, architecture improvements).
130+
131+
**Comment prioritization strategy** (max 20 comments per review but may leave less if no suggestions):
132+
1. **First priority**: All `error` level findings (critical security, breaking changes)
133+
2. **Second priority**: `warning` level findings affecting security/functionality
134+
3. **Third priority**: `warning` level findings affecting maintainability/quality
135+
4. **Fourth priority**: `info` level findings (style, optimization suggestions)
136+
5. **Distribution guideline**: ~8 error, ~6 warning, ~4 info, ~2 architectural feedback
137+
138+
**Comment efficiency guidelines**:
139+
- **Group related issues**: Combine similar findings in single comment when possible
140+
- **Provide context**: Include file:line references and brief explanations
141+
- **Suggest fixes**: Offer specific code snippets for common issues
142+
- **Link related**: Reference related findings across files
143+
- **Prioritize impact**: Focus on user-facing and security-critical changes first
144+
145+
**Auto-labels** (by highest severity and change type):
146+
`security`, `needs-review`, `tests`, `enhancement`, `performance`, `localization`.
147+
148+
---
149+
150+
## Canonical regex library (reference)
151+
```
152+
# Conventional commits
153+
^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\([a-z0-9-./]+\))?(!)?: .{1,72}$
154+
155+
# Unsafe DOM / HTML injection
156+
\binnerHTML\s*=
157+
\.html\s*\(
158+
159+
# Dynamic scripts / code eval
160+
createElement\s*\(\s*['"]script['"]\s*\)|appendChild\s*\([^)]*script[^)]*\)
161+
\beval\s*\(
162+
\bnew\s+Function\s*\(
163+
set(?:Timeout|Interval)\s*\(\s*(['"]).*?\1
164+
165+
# Network APIs
166+
\bXMLHttpRequest\b|\bWebSocket\b|\bfetch\s*\(
167+
168+
# Secrets (subset)
169+
AKIA[0-9A-Z]{16}
170+
ghp_[A-Za-z0-9]{36,}
171+
xox[baprs]-[A-Za-z0-9-]{10,48}
172+
eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}
173+
(AccountKey|SharedAccessKey|SAS|Sig|se=|sp=|sr=|spr=|sv=|st=|sk=|connection\s*string)\s*=\s*[^;'\n]+
174+
npm_[A-Za-z0-9]{36,}
175+
```
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
#!/usr/bin/env node
2+
const fs = require('fs');
3+
const path = require('path');
4+
5+
function usage() {
6+
console.error('Usage: node check-capabilities-compatibility.js --baseFile=capabilities.base.json --prFile=capabilities.json [--allowlist=.github/capabilities-compatibility-allowlist.json]');
7+
process.exit(2);
8+
}
9+
10+
const args = process.argv.slice(2).reduce((acc, cur) => {
11+
const [k, v] = cur.split('=');
12+
acc[k.replace(/^--/, '')] = v || true;
13+
return acc;
14+
}, {});
15+
16+
if (!args.baseFile || !args.prFile) usage();
17+
const baseFile = path.resolve(args.baseFile);
18+
const prFile = path.resolve(args.prFile);
19+
const allowlistFile = args.allowlist ? path.resolve(args.allowlist) : path.resolve('.github/capabilities-compatibility-allowlist.json');
20+
21+
function readJson(file) {
22+
try {
23+
const rawContent = fs.readFileSync(file, 'utf8');
24+
if (rawContent.length === 0) {
25+
console.warn(`Warning: ${file} is empty, treating as empty object`);
26+
return {};
27+
}
28+
const content = rawContent.trim();
29+
if (!content) {
30+
console.warn(`Warning: ${file} contains only whitespace, treating as empty object`);
31+
return {};
32+
}
33+
const parsed = JSON.parse(content);
34+
if (parsed === null) {
35+
console.warn(`Warning: ${file} contains JSON null, treating as empty object`);
36+
return {};
37+
}
38+
return parsed;
39+
} catch (e) {
40+
console.error(`Failed to read or parse JSON file: ${file}\n${e.message}`);
41+
process.exit(2);
42+
}
43+
}
44+
45+
const base = readJson(baseFile);
46+
const pr = readJson(prFile);
47+
48+
// Special case: if base is empty object (missing file), only validate PR structure
49+
if (typeof base === 'object' && base !== null && Object.keys(base).length === 0) {
50+
console.log('Base capabilities.json is missing - treating as new file addition.');
51+
console.log('Performing basic validation of new capabilities.json structure...');
52+
53+
// Basic validation for required properties in new capabilities.json
54+
const requiredProps = ['dataRoles', 'dataViewMappings'];
55+
const missingProps = requiredProps.filter(prop => !pr.hasOwnProperty(prop));
56+
if (missingProps.length > 0) {
57+
console.error(`\nNew capabilities.json is missing required properties: ${missingProps.join(', ')}`);
58+
process.exit(1);
59+
}
60+
61+
// Check for WebAccess (not allowed)
62+
if (pr.privileges && pr.privileges.includes('WebAccess')) {
63+
console.error('\nWebAccess privilege is not allowed in capabilities.json');
64+
process.exit(1);
65+
}
66+
67+
console.log('New capabilities.json structure is valid.');
68+
process.exit(0);
69+
}
70+
71+
let allowlist = [];
72+
if (fs.existsSync(allowlistFile)) {
73+
try {
74+
const allowlistContent = fs.readFileSync(allowlistFile, 'utf8');
75+
allowlist = JSON.parse(allowlistContent);
76+
} catch (e) {
77+
console.warn('Warning: failed to parse allowlist, continuing without it');
78+
}
79+
}
80+
81+
function isPrimitive(val) {
82+
return val === null || (typeof val !== 'object');
83+
}
84+
85+
function pathJoin(parent, key) {
86+
return parent ? `${parent}.${key}` : key;
87+
}
88+
89+
const issues = [];
90+
91+
function record(path, message) {
92+
// if allowlist contains exact path, skip
93+
if (allowlist && Array.isArray(allowlist) && allowlist.includes(path)) return;
94+
issues.push({ path, message });
95+
}
96+
97+
function compareObjects(baseNode, prNode, parentPath) {
98+
if (typeof baseNode !== typeof prNode) {
99+
// Allow object vs array difference? report as modified
100+
record(parentPath || '<root>', `Type changed from ${typeof baseNode} to ${typeof prNode}`);
101+
return;
102+
}
103+
104+
if (Array.isArray(baseNode)) {
105+
// Heuristics: if array of objects and elements have `name` property, match by name.
106+
if (baseNode.length > 0 && typeof baseNode[0] === 'object' && baseNode[0] !== null) {
107+
const byName = baseNode[0] && Object.prototype.hasOwnProperty.call(baseNode[0], 'name');
108+
if (byName) {
109+
const map = new Map();
110+
(prNode || []).forEach(item => { if (item && item.name) map.set(item.name, item); });
111+
baseNode.forEach((item, idx) => {
112+
const key = item && item.name ? item.name : null;
113+
const childPath = pathJoin(parentPath, `${idx}${key ? `(${key})` : ''}`);
114+
if (key && !map.has(key)) {
115+
record(childPath, `Array element with name='${key}' removed or renamed`);
116+
} else if (key) {
117+
compareObjects(item, map.get(key), pathJoin(parentPath, `name=${key}`));
118+
} else {
119+
// fallback to index compare
120+
const prItem = (prNode || [])[idx];
121+
if (prItem === undefined) record(childPath, `Array element at index ${idx} removed`);
122+
else compareObjects(item, prItem, childPath);
123+
}
124+
});
125+
} else {
126+
// compare by index
127+
for (let i = 0; i < baseNode.length; i++) {
128+
const childPath = pathJoin(parentPath, String(i));
129+
if (prNode.length <= i) {
130+
record(childPath, `Array element at index ${i} removed`);
131+
continue;
132+
}
133+
compareObjects(baseNode[i], prNode[i], childPath);
134+
}
135+
}
136+
} else {
137+
// base array of primitives - ensure not removed elements by index
138+
for (let i = 0; i < baseNode.length; i++) {
139+
const childPath = pathJoin(parentPath, String(i));
140+
if (!prNode || prNode.length <= i) record(childPath, `Array element at index ${i} removed`);
141+
else if (typeof baseNode[i] !== typeof prNode[i]) record(childPath, `Type changed at array index ${i} from ${typeof baseNode[i]} to ${typeof prNode[i]}`);
142+
}
143+
}
144+
return;
145+
}
146+
147+
if (isPrimitive(baseNode)) {
148+
// primitive - only check type compatibility
149+
if (isPrimitive(prNode) && typeof baseNode !== typeof prNode) {
150+
record(parentPath || '<root>', `Primitive type changed from ${typeof baseNode} to ${typeof prNode}`);
151+
}
152+
return;
153+
}
154+
155+
// both are objects
156+
for (const key of Object.keys(baseNode)) {
157+
const childPath = pathJoin(parentPath, key);
158+
if (!Object.prototype.hasOwnProperty.call(prNode || {}, key)) {
159+
record(childPath, `Property removed`);
160+
continue;
161+
}
162+
compareObjects(baseNode[key], prNode[key], childPath);
163+
}
164+
}
165+
166+
compareObjects(base, pr, '');
167+
168+
if (issues.length) {
169+
console.error('\n=== capabilities.json compatibility issues detected ===\n');
170+
issues.forEach((it, i) => {
171+
console.error(`${i + 1}. ${it.path} - ${it.message}`);
172+
});
173+
console.error('\nIf these changes are intentional, add the exact JSON paths to the allowlist file (one per line) or update the baseline.');
174+
process.exit(1);
175+
}
176+
177+
console.log('capabilities.json structure is compatible with baseline. No breaking changes detected.');
178+
process.exit(0);

0 commit comments

Comments
 (0)