Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
# Agent-oriented Copilot instructions for PR checks

**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`.

**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.

---

## Summary of agent-capable checks (categories)

- **PR metadata**: non-empty description; conventional commit title.
- **Manifests & capabilities (Power BI)**: presence & schema sanity of `capabilities.json`, `pbiviz.json`, `package.json`, `tsconfig.json`, `src/visual.ts`; no `WebAccess`; version bump rules.
- **Security & forbidden patterns**: unsafe DOM, dynamic scripts, timers-with-strings, `eval/new Function`, network APIs, unsafe bindings.
- **Secrets scanning**: common tokens/keys; urgent human review.
- **Build artifacts & minification & assets**: `.min.*` in `src/`, overly large or minified-looking files.
- **Linting, tests, CI**: scripts present; ESLint config; CI status present if `src/**` changed.
- **Dependencies**: lockfile updated on dependency change; major version bumps flagged.
- **Tests & localization**: unit tests reminder on logic changes; `stringResources/en-US/**` coverage; spellcheck.
- **Documentation & changelog**: `changelog.md` on non-trivial changes; usage examples for public APIs.
- **Code quality & architecture**: scope summary, performance & accessibility hints, state/event cleanup, error handling, maintainability notes.
- **Reporting**: one-line summary counts; per-finding snippets; suggested fixes; auto-labels.

> Maintainers: thresholds, regexes and message templates are the **single source of truth** in this file to avoid divergence.

---

## Detailed rules

### 1) Manifests & capabilities (Power BI)
- **Presence**: `capabilities.json`, `pbiviz.json`, `package.json`, `tsconfig.json`, `src/visual.ts`.
Missing → `error`.
- **Capabilities**:
- No `WebAccess` or privileges that permit arbitrary network calls → `error`.
- `dataRoles` and `dataViewMappings` must be present → `error`.
- **`pbiviz.json`**:
- `visual.version` must bump for functional changes (semver).
- `visual.guid`, `visual.displayName`, `author`, `supportUrl`, `apiVersion` present.
- **`visual.guid` must NEVER be changed** → any modification → `error` (breaks existing Power BI reports).
- `apiVersion` compatible with `@types/powerbi-visuals-api` (major alignment) → mismatch → `warning`.

### 2) Security & forbidden patterns (report file:line)
- Unsafe DOM:
- `innerHTML\s*=` → `error` with safe alternative.
- `.html\(` (D3 selections) → `error` when D3 imported; otherwise `warning`.
- Dynamic scripts / code eval:
- `createElement\(['"]script['"]\)` / `appendChild` of scripts → `error`.
- `eval\(` or `new Function\(` → `error`.
- String-based timers:
`set(?:Timeout|Interval)\(\s*(['"]).*?\1` → `error`.
- Network / runtime:
- `fetch\(`, `XMLHttpRequest`, `WebSocket` → `error` (Power BI certified visuals constraint).
- Prefer safe APIs:
- `textContent`, `setAttribute` over `innerHTML`. Provide auto-fix snippet if RHS is a simple string literal.

### 3) Secrets & credentials
- Run regex scans on changed text files (exclude binaries and lock files).
- Examples (non-exhaustive):
- `AKIA[0-9A-Z]{16}` (AWS)
- `ghp_[A-Za-z0-9]{36,}` (GitHub)
- `xox[baprs]-[A-Za-z0-9-]{10,48}` (Slack)
- `eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}` (JWT)
- `(AccountKey|SharedAccessKey|SAS|Sig|se=|sp=|sr=|spr=|sv=|st=|sk=|connection\s*string)\s*=\s*[^;'\n]+` (Azure)
- `npm_[A-Za-z0-9]{36,}` (NPM)
- `-----BEGIN (?:RSA |EC |DSA )?PRIVATE KEY-----`
- Any hit → `error` + urgent human review. **Do not auto-edit.**

### 4) Build artifacts, minification & large assets
- `error`: any `\.min\.(js|ts|css)$` under `src/**`.
- `warning`: likely-minified file (avg line length > 300 and median > 120) in `src/**`.
- `warning`: large files in `src/**` > 250 KB (exclude `assets/**` and PBIVIZ icons).
- `warning`: assets > 500 KB — recommend re-evaluating bundling, compression, or CDN prohibition (if applicable).

### 5) Linting, tests
- `package.json` scripts must include:
- `lint`, `test`, `package` (or `pbiviz package`) → missing → `warning`.
- ESLint configuration must exist at repo root:
- Prefer `eslint.config.mjs`; if `.eslintrc.*` or `.eslintignore` or `eslintConfig` in `package.json` -> ask to migrate to `eslint.config.mjs`.
- Missing → `warning` + suggest basic config for Power BI visuals.

### 6) Dependencies
- On `dependencies`/`devDependencies` changes require updated `package-lock.json` or `yarn.lock` → `warning`.
- Major-bump in `package.json` → `warning` with request to describe motivation/test-case.
- When adding new features → ensure minor-version is bumped.
- (Optional, as `info`) suggest running `npm audit` (at maintainers' discretion).

### 7) Tests & localization
- If logic touched in `src/**` and no new/updated tests nearby → `warning`-reminder.
- UI strings:
- Check `stringResources/en-US/resources.resjson` and string correspondence from code.
- Missing localization keys → `warning`.
- Spellcheck (en-US as source):
- Report probable typos with level (`info`/`warning`) and replacement suggestion.
- Exclude identifiers/acronyms/brand-names based on `.spellcheck-whitelist`.

### 8) Documentation & changelog
- For non-trivial changes — update `changelog.md` → `info`/`warning`.
- For new public APIs — add usage examples → `info`.

### 9) Code quality & architecture (senior review mindset)
- Briefly summarize PR purpose and affected areas (render, data, settings, UI).
- Highlight:
- Potential performance bottlenecks (DOM in hot paths, unnecessary loops, re-renders).
- Accessibility (ARIA, contrasts, keyboard navigation, screen reader).
- Errors/edge-cases: null/undefined/empty data.
- Resource management: cleanup D3-selectors, event handlers, timers.
- State/races/leaks; excessive coupling; duplication.
- Power BI SDK/utilities compliance, formatting, API contracts.

## Spellcheck Configuration

### What to Check:
- UI strings in code (`src/**`)
- localization files (`stringResources/en-US/**`)
- PR title and description.

### Severity:
- `warning` — UI strings and localization.
- `info` — PR metadata and comments.

---

## Severity & automated labels

- **error** — must fix before merge (e.g., secrets, `WebAccess`, minified code in `src/**`, forbidden APIs).
- **warning** — should fix soon (e.g., missing PR description/tests, major dep bump, large assets).
- **info** — suggestions/style (typos, architecture improvements).

**Auto-labels** (by highest severity and change type):
`security`, `needs-review`, `tests`, `enhancement`, `performance`, `localization`.

---

## Canonical regex library (reference)
```
# Conventional commits
^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test)(\([a-z0-9-./]+\))?(!)?: .{1,72}$

# Unsafe DOM / HTML injection
\binnerHTML\s*=
\.html\s*\(

# Dynamic scripts / code eval
createElement\s*\(\s*['"]script['"]\s*\)|appendChild\s*\([^)]*script[^)]*\)
\beval\s*\(
\bnew\s+Function\s*\(
set(?:Timeout|Interval)\s*\(\s*(['"]).*?\1

# Network APIs
\bXMLHttpRequest\b|\bWebSocket\b|\bfetch\s*\(

# Secrets (subset)
AKIA[0-9A-Z]{16}
ghp_[A-Za-z0-9]{36,}
xox[baprs]-[A-Za-z0-9-]{10,48}
eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}
(AccountKey|SharedAccessKey|SAS|Sig|se=|sp=|sr=|spr=|sv=|st=|sk=|connection\s*string)\s*=\s*[^;'\n]+
npm_[A-Za-z0-9]{36,}
```
173 changes: 173 additions & 0 deletions .github/scripts/check-capabilities-compatibility.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
#!/usr/bin/env node
const fs = require('fs');
const path = require('path');

function usage() {
console.error('Usage: node check-capabilities-compatibility.js --baseFile=capabilities.base.json --prFile=capabilities.json [--allowlist=.github/capabilities-compatibility-allowlist.json]');
process.exit(2);
}

const args = process.argv.slice(2).reduce((acc, cur) => {
const [k, v] = cur.split('=');
acc[k.replace(/^--/, '')] = v || true;
return acc;
}, {});

if (!args.baseFile || !args.prFile) usage();
const baseFile = path.resolve(args.baseFile);
const prFile = path.resolve(args.prFile);
const allowlistFile = args.allowlist ? path.resolve(args.allowlist) : path.resolve('.github/capabilities-compatibility-allowlist.json');

function readJson(file) {
try {
const content = fs.readFileSync(file, 'utf8').trim();
if (!content || content.toLowerCase() === 'null') {
console.warn(`Warning: ${file} is empty or 'null', treating as empty object`);
return {};
}
const parsed = JSON.parse(content);
if (parsed === null) {
console.warn(`Warning: ${file} contains JSON null, treating as empty object`);
return {};
}
return parsed;
} catch (e) {
console.error(`Failed to read or parse JSON file: ${file}\n${e.message}`);
process.exit(2);
}
}

const base = readJson(baseFile);
const pr = readJson(prFile);

// Special case: if base is empty object (missing file), only validate PR structure
if (typeof base === 'object' && base !== null && Object.keys(base).length === 0) {
console.log('Base capabilities.json is missing - treating as new file addition.');
console.log('Performing basic validation of new capabilities.json structure...');

// Basic validation for required properties in new capabilities.json
const requiredProps = ['dataRoles', 'dataViewMappings'];
const missingProps = requiredProps.filter(prop => !pr.hasOwnProperty(prop));
if (missingProps.length > 0) {
console.error(`\nNew capabilities.json is missing required properties: ${missingProps.join(', ')}`);
process.exit(1);
}

// Check for WebAccess (not allowed)
if (pr.privileges && pr.privileges.includes('WebAccess')) {
console.error('\nWebAccess privilege is not allowed in capabilities.json');
process.exit(1);
}

console.log('New capabilities.json structure is valid.');
process.exit(0);
}

let allowlist = [];
if (fs.existsSync(allowlistFile)) {
try {
const allowlistContent = fs.readFileSync(allowlistFile, 'utf8');
allowlist = JSON.parse(allowlistContent);
} catch (e) {
console.warn('Warning: failed to parse allowlist, continuing without it');
}
}

function isPrimitive(val) {
return val === null || (typeof val !== 'object');
}

function pathJoin(parent, key) {
return parent ? `${parent}.${key}` : key;
}

const issues = [];

function record(path, message) {
// if allowlist contains exact path, skip
if (allowlist && Array.isArray(allowlist) && allowlist.includes(path)) return;
issues.push({ path, message });
}

function compareObjects(baseNode, prNode, parentPath) {
if (typeof baseNode !== typeof prNode) {
// Allow object vs array difference? report as modified
record(parentPath || '<root>', `Type changed from ${typeof baseNode} to ${typeof prNode}`);
return;
}

if (Array.isArray(baseNode)) {
// Heuristics: if array of objects and elements have `name` property, match by name.
if (baseNode.length > 0 && typeof baseNode[0] === 'object' && baseNode[0] !== null) {
const byName = baseNode[0] && Object.prototype.hasOwnProperty.call(baseNode[0], 'name');
if (byName) {
const map = new Map();
(prNode || []).forEach(item => { if (item && item.name) map.set(item.name, item); });
baseNode.forEach((item, idx) => {
const key = item && item.name ? item.name : null;
const childPath = pathJoin(parentPath, `${idx}${key ? `(${key})` : ''}`);
if (key && !map.has(key)) {
record(childPath, `Array element with name='${key}' removed or renamed`);
} else if (key) {
compareObjects(item, map.get(key), pathJoin(parentPath, `name=${key}`));
} else {
// fallback to index compare
const prItem = (prNode || [])[idx];
if (prItem === undefined) record(childPath, `Array element at index ${idx} removed`);
else compareObjects(item, prItem, childPath);
}
});
} else {
// compare by index
for (let i = 0; i < baseNode.length; i++) {
const childPath = pathJoin(parentPath, String(i));
if (prNode.length <= i) {
record(childPath, `Array element at index ${i} removed`);
continue;
}
compareObjects(baseNode[i], prNode[i], childPath);
}
}
} else {
// base array of primitives - ensure not removed elements by index
for (let i = 0; i < baseNode.length; i++) {
const childPath = pathJoin(parentPath, String(i));
if (!prNode || prNode.length <= i) record(childPath, `Array element at index ${i} removed`);
else if (typeof baseNode[i] !== typeof prNode[i]) record(childPath, `Type changed at array index ${i} from ${typeof baseNode[i]} to ${typeof prNode[i]}`);
}
}
return;
}

if (isPrimitive(baseNode)) {
// primitive - only check type compatibility
if (isPrimitive(prNode) && typeof baseNode !== typeof prNode) {
record(parentPath || '<root>', `Primitive type changed from ${typeof baseNode} to ${typeof prNode}`);
}
return;
}

// both are objects
for (const key of Object.keys(baseNode)) {
const childPath = pathJoin(parentPath, key);
if (!Object.prototype.hasOwnProperty.call(prNode || {}, key)) {
record(childPath, `Property removed`);
continue;
}
compareObjects(baseNode[key], prNode[key], childPath);
}
}

compareObjects(base, pr, '');

if (issues.length) {
console.error('\n=== capabilities.json compatibility issues detected ===\n');
issues.forEach((it, i) => {
console.error(`${i + 1}. ${it.path} - ${it.message}`);
});
console.error('\nIf these changes are intentional, add the exact JSON paths to the allowlist file (one per line) or update the baseline.');
process.exit(1);
}

console.log('capabilities.json structure is compatible with baseline. No breaking changes detected.');
process.exit(0);
54 changes: 54 additions & 0 deletions .github/workflows/capabilities-compatibility.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: Capabilities compatibility check

on:
pull_request:

jobs:
check-capabilities:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
with:
fetch-depth: 0 # Fetches all history to allow diffing against the base branch

- name: Check for capabilities.json changes
id: check_changes
run: |
# Compare the PR branch with the base branch to find changed files
CHANGED_FILES=$(git diff --name-only origin/${{ github.event.pull_request.base.ref }}...HEAD)
echo "Files changed in this PR:"
echo "$CHANGED_FILES"

if echo "$CHANGED_FILES" | grep -q "capabilities.json"; then
echo "capabilities.json was modified."
echo "any_changed=true" >> $GITHUB_OUTPUT
else
echo "capabilities.json was not modified. Skipping compatibility check."
echo "any_changed=false" >> $GITHUB_OUTPUT
fi
shell: bash

- name: Determine base ref
if: steps.check_changes.outputs.any_changed == 'true'
id: vars
run: |
echo "BASE_REF=${{ github.event.pull_request.base.ref }}" >> $GITHUB_OUTPUT
echo "PR_REF=${{ github.head_ref }}" >> $GITHUB_OUTPUT

- name: Checkout base branch file
if: steps.check_changes.outputs.any_changed == 'true'
run: |
git fetch origin ${{ github.event.pull_request.base.ref }} --depth=1
if git show origin/${{ github.event.pull_request.base.ref }}:capabilities.json > capabilities.base.json 2>/dev/null; then
echo "Base capabilities.json found"
else
echo "No capabilities.json in base branch - treating as new file"
echo '{}' > capabilities.base.json
fi

- name: Run compatibility script
if: steps.check_changes.outputs.any_changed == 'true'
run: |
node ./.github/scripts/check-capabilities-compatibility.js --baseFile=capabilities.base.json --prFile=capabilities.json || exit 1
shell: bash
Loading
Loading