@@ -1018,178 +1018,128 @@ reviews:
10181018 # --- CUSTOM INSTRUCTIONS FOR .GITHUB DIRECTORY ---
10191019 - path : " .github/workflows/**/*"
10201020 instructions : |
1021- Review workflows as security-sensitive infrastructure.
1022-
1023- A good workflow is small, focused, and boring.
1024- If a workflow is clever, generic, or overly flexible, it is a risk.
1025-
1026- ---------------------------------------------------------
1027- PRIORITY 0 — ABSOLUTE REQUIREMENTS
1028- ---------------------------------------------------------
1029- - All third-party actions MUST be pinned to full commit SHAs, similar to other workflows.
1030- - `permissions:` MUST be explicitly declared and minimally scoped.
1031- - Workflows MUST behave safely when executed from forks.
1032- - YAML MUST orchestrate steps, not implement business logic.
1033- - Any workflow that mutates GitHub state MUST support dry-run mode.
1034- - Dry-run behavior must be explicit and visible in logs.
1035- - Workflows MUST NOT modify repository source code outside `.github/`.
1036-
1037- ---------------------------------------------------------
1038- PRIORITY 1 — SCOPE, FOCUS & RESTRAINT
1039- ---------------------------------------------------------
1040- - The title of each workflow must be relevant, match similar naming schemes, and match its script filename.
1041- - Each workflow MUST have a single, clearly defined objective and SHOULD document this in a top-level comment.
1042- - Flag workflows that:
1043- - Attempt to be generic “frameworks”
1044- - Include speculative or future-facing logic
1045- - Perform actions unrelated to the stated goal
1046- - Over-abstraction and excess flexibility are maintenance risks.
1021+ You are reviewing GitHub Actions workflows as a thoughtful senior
1022+ engineer. Your goal is to nudge contributors toward higher-quality,
1023+ maintainable workflows — not to impose a rigid checklist.
10471024
10481025 ---------------------------------------------------------
1049- PRIORITY 2 — INPUT HARDENING
1026+ SECURITY ESSENTIALS
10501027 ---------------------------------------------------------
1051- - Treat ALL GitHub event data as potentially hostile input, including:
1052- - issue titles, bodies, and comments
1053- - labels, usernames, branch names
1054- - Free-form user input MUST NOT be passed directly into:
1055- - shell commands
1056- - gh CLI arguments
1057- - Node.js exec / spawn calls
1058- - Require strict allowlists or exact string matches.
1059- - Flag any use of:
1060- - eval or bash -c
1061- - backticks or $(...) with user-controlled input
1028+ These are firm expectations for every workflow:
1029+
1030+ - All third-party actions must be pinned to full commit SHAs,
1031+ consistent with other workflows in this repository.
1032+ - `permissions:` must be explicitly declared and scoped to the
1033+ minimum needed. Flag over-permissioned or under-permissioned
1034+ workflows, and any reliance on broad default permissions.
1035+ - Workflows must not request `contents: write` unless strictly
1036+ required.
1037+ - Workflows must behave safely when executed from forks.
1038+ - Treat all GitHub event data as potentially untrusted input:
1039+ issue titles, bodies, comments, labels, usernames, branch
1040+ names. Prefer strict allowlists or exact string matches.
1041+ Free-form user input must not flow directly into shell
1042+ commands, `gh` CLI arguments, or Node.js exec/spawn calls.
10621043
10631044 ---------------------------------------------------------
1064- PRIORITY 3 — DRY-RUN & SAFE OPERATION
1045+ CLARITY & CODE ORGANIZATION
10651046 ---------------------------------------------------------
1066- - Workflows that mutate state MUST expose:
1067- workflow_dispatch:
1068- inputs:
1069- dry_run:
1070- default: "true"
1071- - When dry_run=true, workflows MUST:
1072- - Log dry mode is active
1073- - Function on dry run: never post, comment, label, assign or edit
1074- - Be easy to expand in the future
1075- - Exit successfully
1047+ Each workflow should have a single, clearly defined objective.
1048+ YAML should orchestrate execution — not implement business
1049+ logic.
1050+
1051+ - Non-trivial logic belongs in dedicated scripts under
1052+ `.github/scripts/`, keeping the workflow YAML focused on
1053+ orchestration.
1054+ - Environment variables should be defined in the YAML and
1055+ passed to scripts, not hardcoded inside scripts.
1056+ - Flag large `run: |` blocks, inline loops or conditionals,
1057+ and API calls embedded directly in YAML steps.
10761058
10771059 ---------------------------------------------------------
1078- PRIORITY 4 — SCRIPT EXTRACTION & CODE TRIM
1060+ DRY-RUN
10791061 ---------------------------------------------------------
1080- - YAML should orchestrate execution only.
1081- - All non-trivial logic MUST live in:
1082- - `.github/scripts/*.sh`
1083- - `.github/scripts/*.js`
1084- - Workflow filenames and their primary scripts SHOULD share a clear, matching name.
1085- - Scripts MUST remain:
1086- - Purpose-built
1087- - Trim and readable
1088- - Easy to maintain
1089- - Flag:
1090- - Large `run: |` blocks
1091- - Inline loops, conditionals, or API calls in YAML
1092- - Overly generic helper scripts for narrow tasks
1062+ If used, dry-run should be clear, well-logged, and default
1063+ to true on `workflow_dispatch`. It should have a reasonable
1064+ use case and be easy to maintain. Do not flag missing dry-run
1065+ as a blocker.
10931066
10941067 ---------------------------------------------------------
1095- PRIORITY 5 — API EFFICIENCY & DISCIPLINE
1068+ OPERATIONAL QUALITY
10961069 ---------------------------------------------------------
1097- - GitHub API usage must be intentional and minimal.
1098- - Prefer:
1099- - Aggregated queries over per-entity loops
1100- - Server-side filtering over client-side iteration
1101- - Reusing data already present in the event payload
1102- - Pagination MUST:
1103- - Be considered and justified
1104- - Enforce hard upper bounds
1105- - Flag:
1106- - Repeated API calls inside loops
1107- - Unbounded pagination
1108- - Fetching data already available in context
1070+ - Log key decisions and early exits — avoid silent skips.
1071+ - Avoid raw payload dumps and any secret or token leakage.
1072+ - Workflows that mutate state should use concurrency groups
1073+ and duplicate prevention patterns (marker-based comments,
1074+ check-before-create for labels).
1075+ - Assume workflows may run multiple times or be retried.
11091076
11101077 ---------------------------------------------------------
1111- PRIORITY 6 — CONCURRENCY & IDEMPOTENCY
1078+ API DISCIPLINE
11121079 ---------------------------------------------------------
1113- - Workflows that mutate state MUST:
1114- - Define a deterministic concurrency group
1115- - Be safe under retries and parallel execution
1116- - Duplicate prevention is REQUIRED:
1117- - Marker-based comment detection
1118- - Check-before-create logic for labels and assignments
1119- - Assume workflows may:
1120- - Run multiple times
1121- - Be retried automatically
1122- - Execute concurrently with other automations
1123- - Workflows should avoid logic that duplicates or causes conflicts.
1080+ - Prefer reusing data already in the event payload over
1081+ making additional API calls.
1082+ - Prefer aggregated queries and server-side filtering over
1083+ per-entity loops and client-side iteration.
1084+ - Pagination should have reasonable upper bounds.
11241085
11251086 ---------------------------------------------------------
1126- PRIORITY 7 — PERMISSION CORRECTNESS
1087+ REVIEW SCOPE
11271088 ---------------------------------------------------------
1128- - Requested permissions MUST exactly match behavior.
1129- - Explicitly validate common cases:
1130- - issues: write → comments, labels, assignments
1131- - contents: read → repository checkout
1132- - pull-requests: write → PR mutations
1133- - Flag:
1134- - Over-permissioned workflows
1135- - Under-permissioned workflows that would fail at runtime
1136- - Reliance on default permissions
1089+ - Focus feedback on the PR's stated scope.
1090+ - Flag out-of-scope issues as irrelevant and suggest opening
1091+ a separate PR — do not block for them.
1092+ - Do not propose major refactors unless correctness or
1093+ security is affected.
11371094
11381095 ---------------------------------------------------------
1139- PRIORITY 8 — LOGGING & OPERABILITY
1096+ BLOCKERS vs SUGGESTIONS
11401097 ---------------------------------------------------------
1141- - Logs should include, where applicable:
1142- - repository
1143- - workflow name
1144- - issue or PR number
1145- - triggering actor
1146- - dry-run status
1147- - decisions made (performed vs skipped)
1148- - Avoid:
1149- - Silent success or silent skips
1150- - Raw payload dumps
1151- - Any secret or token leakage
1098+ - Block only for security, correctness, or firm expectation
1099+ violations.
1100+ - Style and maintainability improvements should be
1101+ non-blocking suggestions.
1102+ - When in doubt, suggest — don't block.
11521103
11531104 # ============================================================
11541105 # SHELL SCRIPTS
11551106 # ============================================================
11561107 - path : " .github/scripts/**/*.sh"
11571108 instructions : |
1158- Treat shell scripts as production-grade automation.
1109+ Review shell scripts as production-grade CI automation.
11591110
1160- Scripts should be small, focused, and explicit.
1161- Avoid “do-everything” or overly generic scripts.
1111+ Firm expectations:
1112+ - Use `set -euo pipefail` to fail fast on errors.
1113+ - Environment variables should be passed from the YAML,
1114+ validated early, and defensively quoted.
1115+ - No `eval`, `bash -c`, backticks, or `$(...)` with untrusted content.
11621116
1163- - MUST use: `set -euo pipefail`
1164- - MUST validate all required environment variables
1165- - MUST defensively quote variables
1166- - MUST validate all untrusted input
1167- - MUST have bounded loops and pagination
1168- - MUST support dry-run mode if state is mutated
1169- - MUST log key decisions and early exits
1117+ Good practices:
1118+ - Log key decisions and early exits.
1119+ - Keep scripts purpose-built — flag overly generic helpers.
11701120
11711121 # ============================================================
11721122 # JAVASCRIPT SCRIPTS
11731123 # ============================================================
11741124 - path : " .github/scripts/**/*.js"
11751125 instructions : |
1176- Review JavaScript scripts as long-lived automation code .
1177-
1178- Scripts must remain :
1179- - Focused
1180- - Readable
1181- - Purpose-built
1182-
1183- - All `context.payload` fields MUST be validated
1184- - Free-form text MUST NOT be trusted
1185- - Dynamic code execution is prohibited
1186- - Avoid `child_process.exec`; prefer `execFile` if needed
1187- - All async operations MUST be wrapped in try/catch
1188- - Errors MUST include contextual metadata
1189- - Duplicate API calls MUST be avoided
1190- - Marker-based deduplication is required
1191- - Scripts MUST NOT assume write access
1192- - Permission failures MUST be handled gracefully
1126+ Review JavaScript scripts as production-grade CI automation .
1127+
1128+ Firm expectations :
1129+ - Validate `context.payload` fields before use.
1130+ - Do not trust free-form text from issues, PRs, or comments.
1131+ - No `eval`, `new Function`, or dynamic code execution.
1132+ - Use top-level constants for configuration — avoid
1133+ hardcoded values scattered through the script.
1134+ - Environment variables should be passed from the workflow
1135+ YAML, not hardcoded in the script.
1136+
1137+ Good practices:
1138+ - Wrap async operations in try/catch with contextual errors.
1139+ - Reuse event context data instead of making redundant API
1140+ calls.
1141+ - Use marker-based patterns to prevent duplicate actions.
1142+ - Handle permission failures gracefully.
11931143
11941144 - path : " src/hiero_sdk_python/query/**/*.py"
11951145 instructions : *query_review_instructions
0 commit comments