From 3695d10f6af704ed7c3cdd1284e19aa8661315d6 Mon Sep 17 00:00:00 2001 From: o-webdev Date: Sat, 28 Mar 2026 23:09:55 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20v0.6.0=20=E2=80=94=20lifecycle=20CO2e?= =?UTF-8?q?=20policy,=20suggestions=20pagination,=20address-based=20target?= =?UTF-8?q?ing,=20403=20hints,=20methodology=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- METHODOLOGY.md | 37 ++++++ dist/index.cjs | 200 ++++++++++++++++++++++++++--- package.json | 2 +- policy.test.ts | 77 +++++++++++ policy.ts | 45 +++++-- suggestions.ts | 339 +++++++++++++++++++++++++++++++++++++++++++------ 6 files changed, 634 insertions(+), 66 deletions(-) diff --git a/METHODOLOGY.md b/METHODOLOGY.md index e9a4855..38b9bb1 100644 --- a/METHODOLOGY.md +++ b/METHODOLOGY.md @@ -217,6 +217,43 @@ Carbon reduction is weighted at 60%, cost at 40%, both normalised to percentage- --- +## Coverage Boundaries and LOW_ASSUMED_DEFAULT + +GreenOps only applies emission formulas to instance types explicitly present in `factors.json`. When a resource is encountered that is not in the ledger, it is classified as `LOW_ASSUMED_DEFAULT` and **excluded from all calculations**. + +### What LOW_ASSUMED_DEFAULT means + +`LOW_ASSUMED_DEFAULT` is not an estimate — it is a deliberate null. The resource appears in the output as `⚠ UNKNOWN` in the table formatter and in the skipped section of the markdown PR comment, with the exact `unsupportedReason` explaining which instance type is missing and from which provider's ledger section. + +### Why this matters for FinOps auditors + +The formula `embodied_gco2e = (1,200,000g / 35,040h / 48 vCPUs) × vcpus × 730h` is validated against the 71 instance types in the current ledger. Applying it blindly to unsupported instances — particularly memory-optimised families (AWS `r6i`, Azure `Standard_M` series, GCP `m2` series) with non-standard vCPU-to-memory ratios — would produce numbers that cannot be defended under CSRD audit. + +The boundary is intentional. A tool that shows a wrong number is worse than a tool that shows no number. + +### Heuristic ceiling for auditors + +If you need a conservative upper-bound estimate for unsupported instances pending a formal ledger update: + +``` +Scope 2 upper bound = W_max × PUE × 730h / 1000 × grid_intensity_gco2e_per_kwh +Scope 3 upper bound = (1,200,000g / 35,040h / 48) × vcpus × 730h +``` + +These are the maximum-utilisation values. Actual emissions at typical utilisation (50%) will be lower. Open a PR to `factors.json` to add the instance type formally with validated coefficients. + +### Current ledger coverage + +| Provider | Instance types | Notable gaps | +|---|---|---| +| AWS | 40 | r6i, c6i, m6i (Intel v3), Graviton 4 (m8g, c8g) | +| Azure | 16 | Standard_M series, Standard_L series, Standard_NC (GPU) | +| GCP | 15 | n1 series (legacy), m2/m3 memory-optimised, A2 (GPU) | + +All gaps are tracked as open issues. Coverage PRs are the fastest to merge. + +--- + ## Known Limitations - **CPU-only power model.** Memory power draw is tracked in `factors.json` (`memory_gb`) but not yet included in calculations. diff --git a/dist/index.cjs b/dist/index.cjs index d7d590a..b10a67e 100755 --- a/dist/index.cjs +++ b/dist/index.cjs @@ -1968,7 +1968,7 @@ var factors_default = { // package.json var package_default = { name: "greenops-cli", - version: "0.5.4", + version: "0.6.0", description: "Carbon footprint linting for Terraform plans \u2014 AWS, Azure, and GCP. Analyses infrastructure changes for Scope 2, Scope 3, and water impact. Posts recommendations directly on GitHub PRs.", main: "dist/index.cjs", bin: { @@ -2581,7 +2581,8 @@ function loadPolicy(repoRoot = process.cwd()) { const numericFields = [ "max_pr_co2e_increase_kg", "max_pr_cost_increase_usd", - "max_total_co2e_kg" + "max_total_co2e_kg", + "max_lifecycle_co2e_kg" ]; for (const field of numericFields) { if (budgets[field] !== void 0) { @@ -2609,7 +2610,7 @@ function evaluatePolicy(result2, policy) { actual: Math.round(actualKg * 100) / 100, limit: b.max_pr_co2e_increase_kg, unit: "kg CO2e/month", - message: `This PR introduces ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_pr_co2e_increase_kg}kg limit defined in .greenops.yml.` + message: `This PR introduces ${actualKg.toFixed(2)}kg Scope 2 CO2e/month, exceeding the ${b.max_pr_co2e_increase_kg}kg limit defined in .greenops.yml.` }); } } @@ -2633,7 +2634,19 @@ function evaluatePolicy(result2, policy) { actual: Math.round(actualKg * 100) / 100, limit: b.max_total_co2e_kg, unit: "kg CO2e/month", - message: `Total analysed footprint is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_total_co2e_kg}kg ceiling defined in .greenops.yml.` + message: `Total Scope 2 footprint is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_total_co2e_kg}kg ceiling defined in .greenops.yml.` + }); + } + } + if (b.max_lifecycle_co2e_kg !== void 0) { + const actualKg = totals.currentLifecycleCo2eGramsPerMonth / 1e3; + if (actualKg > b.max_lifecycle_co2e_kg) { + violations.push({ + constraint: "max_lifecycle_co2e_kg", + actual: Math.round(actualKg * 100) / 100, + limit: b.max_lifecycle_co2e_kg, + unit: "kg lifecycle CO2e/month", + message: `Total lifecycle footprint (Scope 2 + Scope 3) is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_lifecycle_co2e_kg}kg limit defined in .greenops.yml.` }); } } @@ -2659,6 +2672,16 @@ async function githubRequest(method, path, token, body) { }); if (!response.ok) { const text = await response.text().catch(() => ""); + if (response.status === 403 || response.status === 401) { + throw new Error( + `GitHub API ${method} ${path} \u2192 ${response.status}. Ensure your workflow has "permissions: pull-requests: write" and that the provided github-token has not expired. Raw: ${text.slice(0, 200)}` + ); + } + if (response.status === 422) { + throw new Error( + `GitHub API ${method} ${path} \u2192 422 Unprocessable Entity. The line number may not exist in the PR diff \u2014 the file may not have been modified in this PR, or the diff context is too small. Raw: ${text.slice(0, 200)}` + ); + } throw new Error(`GitHub API ${method} ${path} \u2192 ${response.status}: ${text.slice(0, 200)}`); } if (response.status === 204) @@ -2666,11 +2689,33 @@ async function githubRequest(method, path, token, body) { return response.json(); } async function getPRFiles(token, repoFullName, pullNumber) { - return githubRequest( - "GET", - `/repos/${repoFullName}/pulls/${pullNumber}/files?per_page=100`, - token - ); + const allFiles = []; + let url = `${GITHUB_API}/repos/${repoFullName}/pulls/${pullNumber}/files?per_page=100`; + while (url) { + const response = await fetch(url, { + headers: { + "Authorization": `Bearer ${token}`, + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "greenops-cli" + } + }); + if (!response.ok) { + const text = await response.text().catch(() => ""); + if (response.status === 403 || response.status === 401) { + throw new Error( + `GitHub API GET /pulls/${pullNumber}/files \u2192 ${response.status}. Ensure your workflow has "permissions: pull-requests: write". Raw: ${text.slice(0, 200)}` + ); + } + throw new Error(`GitHub API GET /pulls files \u2192 ${response.status}: ${text.slice(0, 200)}`); + } + const page = await response.json(); + allFiles.push(...page); + const linkHeader = response.headers.get("link") ?? ""; + const nextMatch = linkHeader.match(/<([^>]+)>;\s*rel="next"/); + url = nextMatch ? nextMatch[1] : null; + } + return allFiles; } function buildLineMap(patch) { const map = /* @__PURE__ */ new Map(); @@ -2689,6 +2734,49 @@ function buildLineMap(patch) { } return map; } +function buildAddressFileMap(planFilePath) { + const map = /* @__PURE__ */ new Map(); + try { + const { readFileSync: readFileSync3 } = require("node:fs"); + const { resolve: resolve3, isAbsolute: isAbsolute2 } = require("node:path"); + const resolvedPath = isAbsolute2(planFilePath) ? planFilePath : resolve3(process.cwd(), planFilePath); + const raw = readFileSync3(resolvedPath, "utf8"); + const plan = JSON.parse(raw); + const rootModule = plan?.configuration?.root_module; + if (!rootModule) + return map; + extractAddressFileEntries(rootModule?.resources ?? [], map); + for (const [, mod] of Object.entries(rootModule?.module_calls ?? {})) { + extractModuleResources(mod?.module ?? {}, map, ""); + } + } catch { + } + return map; +} +function extractAddressFileEntries(resources, map) { + for (const res of resources) { + const r = res; + if (r.address && r.pos) { + const filename = r.pos.filename; + if (filename) + map.set(r.address, filename); + } + } +} +function extractModuleResources(mod, map, prefix) { + for (const res of mod.resources ?? []) { + const r = res; + if (r.address && r.pos) { + const filename = r.pos.filename; + const fullAddress = prefix ? `${prefix}.${r.address}` : r.address; + if (filename) + map.set(fullAddress, filename); + } + } + for (const [, child] of Object.entries(mod.module_calls ?? {})) { + extractModuleResources(child?.module ?? {}, map, prefix); + } +} function buildSuggestionBody(resourceId, recommendation, originalLine, attributeKey, newValue) { const indent = originalLine.match(/^(\s*)/)?.[1] ?? ""; const suggestedLine = `${indent}${attributeKey} = "${newValue}"`; @@ -2718,18 +2806,42 @@ function formatCostDelta(usd) { return `${sign}$${Math.abs(usd).toFixed(2)}`; } async function getExistingSuggestionComments(token, repoFullName, pullNumber) { - const comments = await githubRequest( - "GET", - `/repos/${repoFullName}/pulls/${pullNumber}/comments?per_page=100`, - token - ); - return comments.filter((c) => c.body.includes(GREENOPS_MARKER)); + const allComments = []; + let url = `${GITHUB_API}/repos/${repoFullName}/pulls/${pullNumber}/comments?per_page=100`; + while (url) { + const response = await fetch(url, { + headers: { + "Authorization": `Bearer ${token}`, + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "greenops-cli" + } + }); + if (!response.ok) { + const text = await response.text().catch(() => ""); + throw new Error(`GitHub API GET comments \u2192 ${response.status}: ${text.slice(0, 200)}`); + } + const page = await response.json(); + allComments.push(...page); + const linkHeader = response.headers.get("link") ?? ""; + const nextMatch = linkHeader.match(/<([^>]+)>;\s*rel="next"/); + url = nextMatch ? nextMatch[1] : null; + } + return allComments.filter((c) => c.body.includes(GREENOPS_MARKER)); +} +function resolveAttributeKey(provider, isDb) { + if (provider === "azure") + return "size"; + if (provider === "gcp") + return "machine_type"; + return isDb ? "instance_class" : "instance_type"; } async function postSuggestions(result2, ctx) { const output = { posted: 0, updated: 0, skipped: 0, warnings: [] }; const resourcesWithRecs = result2.resources.filter((r) => r.recommendation !== null); if (resourcesWithRecs.length === 0) return output; + const addressFileMap = buildAddressFileMap(ctx.planFilePath); let prFiles; let existingComments; try { @@ -2742,12 +2854,13 @@ async function postSuggestions(result2, ctx) { return output; } const tfFiles = prFiles.filter((f) => f.filename.endsWith(".tf") && f.patch); + const tfFileMap = new Map(tfFiles.map((f) => [f.filename, f])); for (const { input, recommendation } of resourcesWithRecs) { if (!recommendation) continue; - const provider = input.provider ?? "aws"; + const provider = input.provider; const isDb = provider === "aws" && (input.resourceId.includes("aws_db_instance") || input.instanceType.startsWith("db.")); - const attributeKey = provider === "azure" ? "size" : provider === "gcp" ? "machine_type" : isDb ? "instance_class" : "instance_type"; + const attributeKey = resolveAttributeKey(provider, isDb); const currentValue = isDb ? `db.${input.instanceType}` : input.instanceType; const newValue = recommendation.suggestedInstanceType ? isDb ? `db.${recommendation.suggestedInstanceType}` : recommendation.suggestedInstanceType : input.instanceType; if (!recommendation.suggestedInstanceType) { @@ -2758,12 +2871,25 @@ async function postSuggestions(result2, ctx) { continue; } const searchPattern = `${attributeKey} = "${currentValue}"`; + const knownFile = addressFileMap.get(input.resourceId); + const filesToSearch = knownFile && tfFileMap.has(knownFile) ? [tfFileMap.get(knownFile), ...tfFiles.filter((f) => f.filename !== knownFile)] : tfFiles; let matched = false; - for (const file of tfFiles) { + for (const file of filesToSearch) { if (!file.patch) continue; const lineMap = buildLineMap(file.patch); - const lineNumber = lineMap.get(searchPattern); + const resourceType = input.resourceId.split(".")[0] ?? ""; + const resourceName = input.resourceId.split(".").slice(1).join(".") ?? ""; + const lastDotParts = input.resourceId.split("."); + const bareType = lastDotParts.length >= 2 ? lastDotParts[lastDotParts.length - 2] : resourceType; + const bareName = lastDotParts[lastDotParts.length - 1] ?? resourceName; + const resourceHeaderPattern = `resource "${bareType}" "${bareName}"`; + const lineNumber = findAttributeLineAfterHeader( + file.patch, + resourceHeaderPattern, + searchPattern, + lineMap + ); if (!lineNumber) continue; const originalLine = ` ${attributeKey} = "${currentValue}"`; @@ -2815,12 +2941,46 @@ async function postSuggestions(result2, ctx) { if (!matched) { output.skipped++; output.warnings.push( - `[${input.resourceId}] Could not locate \`${searchPattern}\` in PR diff. Suggestion not posted \u2014 resource may be in a file not modified in this PR.` + `[${input.resourceId}] Could not locate \`${searchPattern}\` in PR diff. Suggestion not posted \u2014 the attribute may use a variable reference, or the file containing this resource was not modified in this PR.` ); } } return output; } +function findAttributeLineAfterHeader(patch, resourceHeaderPattern, attributePattern, lineMap) { + const lines = patch.split("\n"); + let lineNum = 0; + let inTargetBlock = false; + let blockDepth = 0; + for (const line of lines) { + const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/); + if (hunkMatch) { + lineNum = parseInt(hunkMatch[1], 10) - 1; + continue; + } + if (!line.startsWith("-")) + lineNum++; + const content = (line.startsWith("+") ? line.slice(1) : line).trim(); + if (!inTargetBlock) { + if (content.includes(resourceHeaderPattern)) { + inTargetBlock = true; + blockDepth = 1; + } + } else { + blockDepth += (content.match(/\{/g) ?? []).length; + blockDepth -= (content.match(/\}/g) ?? []).length; + if (blockDepth <= 0) { + inTargetBlock = false; + blockDepth = 0; + continue; + } + if (content === attributePattern && !line.startsWith("-")) { + return lineNum; + } + } + } + return lineMap.get(attributePattern) ?? null; +} // formatters/util.ts function formatDelta2(grams) { diff --git a/package.json b/package.json index 42068c7..a99603b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "greenops-cli", - "version": "0.5.4", + "version": "0.6.0", "description": "Carbon footprint linting for Terraform plans \u2014 AWS, Azure, and GCP. Analyses infrastructure changes for Scope 2, Scope 3, and water impact. Posts recommendations directly on GitHub PRs.", "main": "dist/index.cjs", "bin": { diff --git a/policy.test.ts b/policy.test.ts index d9a8a38..abf27db 100644 --- a/policy.test.ts +++ b/policy.test.ts @@ -152,6 +152,83 @@ describe('Policy Engine', () => { assert.equal(evaluation.shouldBlock, false, 'Should not block when fail_on_violation is unset'); }); + it('detects max_lifecycle_co2e_kg violation (Scope 2 + Scope 3)', () => { + // lifecycle = Scope2 + Scope3 = 5000 + 1041.7 = 6041.7g = 6.04kg + const result = makeMockResult({ + currentCo2eGramsPerMonth: 5000, + currentEmbodiedCo2eGramsPerMonth: 1041.7, + currentLifecycleCo2eGramsPerMonth: 6041.7, + }); + const policy = { + version: 1, + budgets: { max_lifecycle_co2e_kg: 5 }, // 6.04kg > 5kg — violation + fail_on_violation: false, + }; + + const evaluation = evaluatePolicy(result, policy); + assert.equal(evaluation.isCompliant, false); + assert.equal(evaluation.violations.length, 1); + assert.equal(evaluation.violations[0].constraint, 'max_lifecycle_co2e_kg'); + assert.ok(evaluation.violations[0].actual > 5, 'Actual should exceed limit'); + assert.ok(evaluation.violations[0].message.includes('Scope 2 + Scope 3'), 'Message should mention both scopes'); + }); + + it('max_lifecycle_co2e_kg is compliant when within limit', () => { + const result = makeMockResult({ + currentLifecycleCo2eGramsPerMonth: 4000, // 4kg lifecycle + }); + const policy = { + version: 1, + budgets: { max_lifecycle_co2e_kg: 10 }, // 4kg < 10kg — pass + fail_on_violation: true, + }; + + const evaluation = evaluatePolicy(result, policy); + assert.equal(evaluation.isCompliant, true); + assert.equal(evaluation.violations.length, 0); + assert.equal(evaluation.shouldBlock, false); + }); + + it('max_lifecycle_co2e_kg can trigger independently of Scope 2 constraints', () => { + // Scope 2 alone passes, but lifecycle (Scope 2 + Scope 3) fails + const result = makeMockResult({ + currentCo2eGramsPerMonth: 3000, // 3kg Scope 2 — under limit + currentEmbodiedCo2eGramsPerMonth: 4000, // 4kg Scope 3 + currentLifecycleCo2eGramsPerMonth: 7000, // 7kg lifecycle — over limit + }); + const policy = { + version: 1, + budgets: { + max_pr_co2e_increase_kg: 5, // 3kg < 5kg — passes + max_lifecycle_co2e_kg: 6, // 7kg > 6kg — fails + }, + fail_on_violation: true, + }; + + const evaluation = evaluatePolicy(result, policy); + assert.equal(evaluation.isCompliant, false); + assert.equal(evaluation.violations.length, 1); + assert.equal(evaluation.violations[0].constraint, 'max_lifecycle_co2e_kg'); + assert.equal(evaluation.shouldBlock, true); + }); + + it('loads max_lifecycle_co2e_kg from .greenops.yml', () => { + mkdirSync(TMP_DIR, { recursive: true }); + const policyPath = resolve(TMP_DIR, '.greenops.yml'); + writeFileSync(policyPath, [ + 'version: 1', + 'budgets:', + ' max_lifecycle_co2e_kg: 20', + 'fail_on_violation: true', + ].join('\n')); + + const policy = loadPolicy(TMP_DIR); + assert.ok(policy !== null); + assert.equal(policy!.budgets?.max_lifecycle_co2e_kg, 20); + + unlinkSync(policyPath); + }); + it('throws a descriptive error for malformed budgets values', () => { mkdirSync(TMP_DIR, { recursive: true }); const policyPath = resolve(TMP_DIR, '.greenops.yml'); diff --git a/policy.ts b/policy.ts index fead127..0036d5a 100644 --- a/policy.ts +++ b/policy.ts @@ -9,6 +9,16 @@ * - Offline-first: zero network calls, pure computation against local state. * - Transparent: every violation includes the constraint that was breached, * the actual value, and the allowed limit — suitable for PR comment output. + * + * Constraint semantics: + * max_pr_co2e_increase_kg — Scope 2 operational CO2e of all resources in plan (kg/month) + * max_pr_cost_increase_usd — Total infrastructure cost of all resources in plan (USD/month) + * max_total_co2e_kg — Same as max_pr_co2e_increase_kg (alias for clarity) + * max_lifecycle_co2e_kg — Scope 2 + Scope 3 combined lifecycle CO2e (kg/month) + * + * Note: all constraints evaluate the footprint of resources present in the plan, + * not a delta from a baseline. True delta calculation requires Terraform state + * access which is outside the scope of a plan-only analysis. */ import { readFileSync, existsSync } from 'node:fs'; @@ -32,17 +42,23 @@ import type { PlanAnalysisResult } from './types.js'; * max_pr_co2e_increase_kg: 10 * max_pr_cost_increase_usd: 500 * max_total_co2e_kg: 100 + * max_lifecycle_co2e_kg: 15 * fail_on_violation: true */ export interface GreenOpsPolicy { version: number; budgets?: { - /** Maximum CO2e increase (kg) this PR is allowed to introduce. */ + /** Maximum Scope 2 CO2e (kg/month) this PR is allowed to introduce. */ max_pr_co2e_increase_kg?: number; /** Maximum cost increase (USD/month) this PR is allowed to introduce. */ max_pr_cost_increase_usd?: number; - /** Maximum total CO2e (kg/month) across all analysed resources in this plan. */ + /** Maximum total Scope 2 CO2e (kg/month) across all analysed resources. */ max_total_co2e_kg?: number; + /** + * Maximum lifecycle CO2e (kg/month) — Scope 2 operational + Scope 3 embodied combined. + * Use this for CSRD reporting where full lifecycle emissions must be bounded. + */ + max_lifecycle_co2e_kg?: number; }; /** If true, CLI exits with code 1 when policy is violated. Default: false (warn-only). */ fail_on_violation?: boolean; @@ -198,6 +214,7 @@ export function loadPolicy(repoRoot: string = process.cwd()): GreenOpsPolicy | n 'max_pr_co2e_increase_kg', 'max_pr_cost_increase_usd', 'max_total_co2e_kg', + 'max_lifecycle_co2e_kg', ]; for (const field of numericFields) { @@ -237,9 +254,7 @@ export function evaluatePolicy( const { totals } = result; const b = policy.budgets; - // Constraint 1: max_pr_co2e_increase_kg - // The "increase" for a PR is the total new footprint introduced (currentCo2eGramsPerMonth). - // We don't have a pre-PR baseline here — the plan represents net-new resources being added. + // Constraint 1: max_pr_co2e_increase_kg (Scope 2 operational) if (b.max_pr_co2e_increase_kg !== undefined) { const actualKg = totals.currentCo2eGramsPerMonth / 1000; if (actualKg > b.max_pr_co2e_increase_kg) { @@ -248,7 +263,7 @@ export function evaluatePolicy( actual: Math.round(actualKg * 100) / 100, limit: b.max_pr_co2e_increase_kg, unit: 'kg CO2e/month', - message: `This PR introduces ${(actualKg).toFixed(2)}kg CO2e/month, exceeding the ${b.max_pr_co2e_increase_kg}kg limit defined in .greenops.yml.`, + message: `This PR introduces ${actualKg.toFixed(2)}kg Scope 2 CO2e/month, exceeding the ${b.max_pr_co2e_increase_kg}kg limit defined in .greenops.yml.`, }); } } @@ -267,7 +282,7 @@ export function evaluatePolicy( } } - // Constraint 3: max_total_co2e_kg + // Constraint 3: max_total_co2e_kg (Scope 2 operational — alias) if (b.max_total_co2e_kg !== undefined) { const actualKg = totals.currentCo2eGramsPerMonth / 1000; if (actualKg > b.max_total_co2e_kg) { @@ -276,7 +291,21 @@ export function evaluatePolicy( actual: Math.round(actualKg * 100) / 100, limit: b.max_total_co2e_kg, unit: 'kg CO2e/month', - message: `Total analysed footprint is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_total_co2e_kg}kg ceiling defined in .greenops.yml.`, + message: `Total Scope 2 footprint is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_total_co2e_kg}kg ceiling defined in .greenops.yml.`, + }); + } + } + + // Constraint 4: max_lifecycle_co2e_kg (Scope 2 + Scope 3 combined) + if (b.max_lifecycle_co2e_kg !== undefined) { + const actualKg = totals.currentLifecycleCo2eGramsPerMonth / 1000; + if (actualKg > b.max_lifecycle_co2e_kg) { + violations.push({ + constraint: 'max_lifecycle_co2e_kg', + actual: Math.round(actualKg * 100) / 100, + limit: b.max_lifecycle_co2e_kg, + unit: 'kg lifecycle CO2e/month', + message: `Total lifecycle footprint (Scope 2 + Scope 3) is ${actualKg.toFixed(2)}kg CO2e/month, exceeding the ${b.max_lifecycle_co2e_kg}kg limit defined in .greenops.yml.`, }); } } diff --git a/suggestions.ts b/suggestions.ts index 5f6ce66..31cdee9 100644 --- a/suggestions.ts +++ b/suggestions.ts @@ -10,9 +10,13 @@ * - Precise targeting: suggestions are posted on the exact line that contains * the attribute being changed (instance_type/instance_class for AWS, * size for Azure, machine_type for GCP). + * - Address-based file resolution: the Terraform resource address from plan.json + * is used to anchor the search to the correct .tf file before line matching. * - Idempotent: existing GreenOps suggestion comments are updated, not duplicated. * - Fail-open: if the GitHub API is unreachable or the plan file cannot be mapped * to a source file, the CLI exits 0 with a warning. Never blocks a deployment. + * - Paginated: fetches all PR files across multiple pages (100 per page) to + * handle large PRs correctly. * * How GitHub suggestion syntax works: * A PR review comment with a code block tagged ```suggestion replaces the @@ -20,7 +24,7 @@ * must be posted to a specific file path + line number that exists in the PR diff. */ -import type { PlanAnalysisResult, UpgradeRecommendation } from './types.js'; +import type { PlanAnalysisResult, UpgradeRecommendation, CloudProvider } from './types.js'; // --------------------------------------------------------------------------- // Types @@ -86,6 +90,21 @@ async function githubRequest( if (!response.ok) { const text = await response.text().catch(() => ''); + // 3D: Produce plain-English hints for common permission errors + if (response.status === 403 || response.status === 401) { + throw new Error( + `GitHub API ${method} ${path} → ${response.status}. ` + + `Ensure your workflow has "permissions: pull-requests: write" and that ` + + `the provided github-token has not expired. Raw: ${text.slice(0, 200)}` + ); + } + if (response.status === 422) { + throw new Error( + `GitHub API ${method} ${path} → 422 Unprocessable Entity. ` + + `The line number may not exist in the PR diff — the file may not have been ` + + `modified in this PR, or the diff context is too small. Raw: ${text.slice(0, 200)}` + ); + } throw new Error(`GitHub API ${method} ${path} → ${response.status}: ${text.slice(0, 200)}`); } @@ -95,28 +114,60 @@ async function githubRequest( } // --------------------------------------------------------------------------- -// PR diff file resolution +// PR diff file resolution — paginated (3C) // --------------------------------------------------------------------------- /** - * Fetches the list of files changed in a PR and finds .tf files. - * Used to map resource IDs to source file locations. + * Fetches ALL files changed in a PR across multiple pages. + * GitHub caps per_page at 100. PRs with >100 changed files require pagination + * via the Link: rel="next" response header. Without this, suggestions for + * resources in files beyond the first 100 are silently skipped. */ async function getPRFiles( token: string, repoFullName: string, pullNumber: number ): Promise { - return githubRequest( - 'GET', - `/repos/${repoFullName}/pulls/${pullNumber}/files?per_page=100`, - token - ); + const allFiles: GitHubPRFile[] = []; + let url: string | null = + `${GITHUB_API}/repos/${repoFullName}/pulls/${pullNumber}/files?per_page=100`; + + while (url) { + const response: Response = await fetch(url, { + headers: { + 'Authorization': `Bearer ${token}`, + 'Accept': 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'greenops-cli', + }, + }); + + if (!response.ok) { + const text: string = await response.text().catch(() => ''); + if (response.status === 403 || response.status === 401) { + throw new Error( + `GitHub API GET /pulls/${pullNumber}/files → ${response.status}. ` + + `Ensure your workflow has "permissions: pull-requests: write". Raw: ${text.slice(0, 200)}` + ); + } + throw new Error(`GitHub API GET /pulls files → ${response.status}: ${text.slice(0, 200)}`); + } + + const page = await response.json() as GitHubPRFile[]; + allFiles.push(...page); + + // Parse Link header for next page + const linkHeader: string = response.headers.get('link') ?? ''; + const nextMatch: RegExpMatchArray | null = linkHeader.match(/<([^>]+)>;\s*rel="next"/); + url = nextMatch ? nextMatch[1] : null; + } + + return allFiles; } /** * Parses a unified diff patch to build a line number map. - * Returns a map of { lineContent → lineNumber } for the "right" (new) side. + * Returns a map of { trimmedLineContent → lineNumber } for the "right" (new) side. * * GitHub review comments reference the line number in the new file. */ @@ -140,6 +191,87 @@ function buildLineMap(patch: string): Map { return map; } +// --------------------------------------------------------------------------- +// Address-based file resolution (3E) +// --------------------------------------------------------------------------- + +/** + * Builds a map of { resourceAddress → sourceFilePath } from the Terraform plan's + * configuration block. Full Terraform plans (generated by `terraform show -json`) + * include file path information in the configuration.root_module.resources array. + * + * This allows us to target the correct .tf file directly rather than searching + * all changed files for a matching attribute string, which fails when: + * - Two resources of the same instance type exist across different files + * - The instance type is set via a variable reference + * + * Falls back gracefully if the plan doesn't include configuration metadata + * (e.g. plans generated by older Terraform versions). + */ +function buildAddressFileMap(planFilePath: string): Map { + const map = new Map(); + + try { + const { readFileSync } = require('node:fs') as typeof import('node:fs'); + const { resolve, isAbsolute } = require('node:path') as typeof import('node:path'); + + const resolvedPath = isAbsolute(planFilePath) + ? planFilePath : resolve(process.cwd(), planFilePath); + + const raw = readFileSync(resolvedPath, 'utf8'); + const plan = JSON.parse(raw) as Record; + + // Terraform plan configuration block contains file metadata + const rootModule = (plan as any)?.configuration?.root_module; + if (!rootModule) return map; + + // Process root module resources + extractAddressFileEntries(rootModule?.resources ?? [], map); + + // Process child modules recursively + for (const [, mod] of Object.entries(rootModule?.module_calls ?? {})) { + extractModuleResources((mod as any)?.module ?? {}, map, ''); + } + } catch { + // Plan file unreadable or missing configuration block — fall back to + // content-based search across all changed .tf files + } + + return map; +} + +function extractAddressFileEntries( + resources: unknown[], + map: Map +): void { + for (const res of resources) { + const r = res as Record; + if (r.address && r.pos) { + // pos.filename is the .tf file path relative to the workspace root + const filename = (r.pos as Record).filename as string; + if (filename) map.set(r.address as string, filename); + } + } +} + +function extractModuleResources( + mod: Record, + map: Map, + prefix: string +): void { + for (const res of (mod.resources ?? []) as unknown[]) { + const r = res as Record; + if (r.address && r.pos) { + const filename = (r.pos as Record).filename as string; + const fullAddress = prefix ? `${prefix}.${r.address}` : r.address as string; + if (filename) map.set(fullAddress, filename); + } + } + for (const [, child] of Object.entries(mod.module_calls ?? {})) { + extractModuleResources((child as any)?.module ?? {}, map, prefix); + } +} + // --------------------------------------------------------------------------- // Suggestion body builder // --------------------------------------------------------------------------- @@ -201,12 +333,45 @@ async function getExistingSuggestionComments( repoFullName: string, pullNumber: number ): Promise { - const comments = await githubRequest( - 'GET', - `/repos/${repoFullName}/pulls/${pullNumber}/comments?per_page=100`, - token - ); - return comments.filter(c => c.body.includes(GREENOPS_MARKER)); + // Also paginated — PRs with many existing comments need full traversal + const allComments: GitHubReviewComment[] = []; + let url: string | null = + `${GITHUB_API}/repos/${repoFullName}/pulls/${pullNumber}/comments?per_page=100`; + + while (url) { + const response: Response = await fetch(url, { + headers: { + 'Authorization': `Bearer ${token}`, + 'Accept': 'application/vnd.github+json', + 'X-GitHub-Api-Version': '2022-11-28', + 'User-Agent': 'greenops-cli', + }, + }); + + if (!response.ok) { + const text: string = await response.text().catch(() => ''); + throw new Error(`GitHub API GET comments → ${response.status}: ${text.slice(0, 200)}`); + } + + const page = await response.json() as GitHubReviewComment[]; + allComments.push(...page); + + const linkHeader: string = response.headers.get('link') ?? ''; + const nextMatch: RegExpMatchArray | null = linkHeader.match(/<([^>]+)>;\s*rel="next"/); + url = nextMatch ? nextMatch[1] : null; + } + + return allComments.filter(c => c.body.includes(GREENOPS_MARKER)); +} + +// --------------------------------------------------------------------------- +// Attribute key resolution — provider-aware +// --------------------------------------------------------------------------- + +function resolveAttributeKey(provider: CloudProvider | undefined, isDb: boolean): string { + if (provider === 'azure') return 'size'; + if (provider === 'gcp') return 'machine_type'; + return isDb ? 'instance_class' : 'instance_type'; } // --------------------------------------------------------------------------- @@ -218,12 +383,16 @@ async function getExistingSuggestionComments( * recommendation in the analysis result. * * Targeting strategy: - * 1. Fetch all .tf files changed in the PR - * 2. For each resource with a recommendation, search changed .tf files for - * a line matching `instance_type = "current_type"` or `instance_class = "..."` - * 3. Post a suggestion comment on that exact line - * 4. If an existing GreenOps suggestion comment exists on that line, update it - * 5. If the line cannot be found in the diff, log a warning and skip + * 1. Build a resource-address → .tf-file map from the plan's configuration block + * 2. Fetch all .tf files changed in the PR (paginated, handles >100 files) + * 3. For each resource with a recommendation: + * a. If the address map has a file path, search that file first + * b. Otherwise, fall back to searching all changed .tf files + * c. Within the target file(s), match on resource block header + attribute line + * to disambiguate multiple resources of the same instance type + * 4. Post a suggestion comment on the matched line + * 5. If an existing GreenOps suggestion comment exists on that line, update it + * 6. If the line cannot be found in the diff, log a warning and skip * * Fail-open: any error is caught and returned as a warning, never throws. */ @@ -236,6 +405,9 @@ export async function postSuggestions( const resourcesWithRecs = result.resources.filter(r => r.recommendation !== null); if (resourcesWithRecs.length === 0) return output; + // Build address → file map from plan configuration block (best-effort) + const addressFileMap = buildAddressFileMap(ctx.planFilePath); + let prFiles: GitHubPRFile[]; let existingComments: GitHubReviewComment[]; @@ -251,27 +423,24 @@ export async function postSuggestions( // Only consider .tf files with a patch (i.e. modified lines we can target) const tfFiles = prFiles.filter(f => f.filename.endsWith('.tf') && f.patch); + const tfFileMap = new Map(tfFiles.map(f => [f.filename, f])); for (const { input, recommendation } of resourcesWithRecs) { if (!recommendation) continue; - // Determine which attribute and value we're targeting — provider-aware - const provider = input.provider ?? 'aws'; + // Determine attribute key and values — provider-aware + const provider = input.provider; const isDb = provider === 'aws' && ( input.resourceId.includes('aws_db_instance') || input.instanceType.startsWith('db.') ); - const attributeKey = - provider === 'azure' ? 'size' : - provider === 'gcp' ? 'machine_type' : - isDb ? 'instance_class' : 'instance_type'; + const attributeKey = resolveAttributeKey(provider, isDb); const currentValue = isDb ? `db.${input.instanceType}` : input.instanceType; const newValue = recommendation.suggestedInstanceType ? (isDb ? `db.${recommendation.suggestedInstanceType}` : recommendation.suggestedInstanceType) - : input.instanceType; // region-only suggestion — no instance change + : input.instanceType; - // For region-only suggestions we can't post a suggestion (no single line to target) - // Instead we post a review comment without a suggestion block + // Region-only suggestions cannot be expressed as a single-line suggestion if (!recommendation.suggestedInstanceType) { output.skipped++; output.warnings.push( @@ -281,18 +450,42 @@ export async function postSuggestions( continue; } - // The attribute line we are searching for in the PR diff + // The attribute pattern we are searching for const searchPattern = `${attributeKey} = "${currentValue}"`; - // Search for the matching line across all changed .tf files + // Determine which files to search: + // 1. If the address map has a file for this resource, search that file first + // 2. Fall back to all changed .tf files + const knownFile = addressFileMap.get(input.resourceId); + const filesToSearch: GitHubPRFile[] = knownFile && tfFileMap.has(knownFile) + ? [tfFileMap.get(knownFile)!, ...tfFiles.filter(f => f.filename !== knownFile)] + : tfFiles; + let matched = false; - for (const file of tfFiles) { + + for (const file of filesToSearch) { if (!file.patch) continue; const lineMap = buildLineMap(file.patch); - // Look for a line like: instance_type = "m5.large" - const lineNumber = lineMap.get(searchPattern); + // Primary strategy: anchor to resource block header, then find attribute + // This disambiguates when multiple resources use the same instance type + const resourceType = input.resourceId.split('.')[0] ?? ''; + const resourceName = input.resourceId.split('.').slice(1).join('.') ?? ''; + // Strip module prefix for matching — "module.x.aws_instance.web" → type="aws_instance", name="web" + const lastDotParts = input.resourceId.split('.'); + const bareType = lastDotParts.length >= 2 ? lastDotParts[lastDotParts.length - 2] : resourceType; + const bareName = lastDotParts[lastDotParts.length - 1] ?? resourceName; + + // Build candidate line numbers: lines matching the attribute pattern + // that appear after the resource block header in the diff + const resourceHeaderPattern = `resource "${bareType}" "${bareName}"`; + const lineNumber = findAttributeLineAfterHeader( + file.patch, + resourceHeaderPattern, + searchPattern, + lineMap + ); if (!lineNumber) continue; @@ -352,10 +545,82 @@ export async function postSuggestions( output.skipped++; output.warnings.push( `[${input.resourceId}] Could not locate \`${searchPattern}\` in PR diff. ` + - `Suggestion not posted — resource may be in a file not modified in this PR.` + `Suggestion not posted — the attribute may use a variable reference, or the ` + + `file containing this resource was not modified in this PR.` ); } } return output; } + +// --------------------------------------------------------------------------- +// Address-anchored line finding +// --------------------------------------------------------------------------- + +/** + * Finds the line number of an attribute pattern within the scope of a specific + * resource block in a unified diff patch. + * + * Strategy: + * 1. Scan the patch for the resource block header (e.g. `resource "aws_instance" "web" {`) + * 2. Once found, scan forward for the attribute pattern (e.g. `instance_type = "m5.large"`) + * 3. Stop scanning if we hit the end of the block (unindented `}`) or another resource header + * + * This prevents false positives when two resources of the same instance type + * exist in the same file. + * + * Falls back to a simple map lookup if the header is not found in the diff + * (e.g. the resource block was not modified but the attribute line was). + */ +function findAttributeLineAfterHeader( + patch: string, + resourceHeaderPattern: string, + attributePattern: string, + lineMap: Map +): number | null { + const lines = patch.split('\n'); + let lineNum = 0; + let inTargetBlock = false; + let blockDepth = 0; + + for (const line of lines) { + // Track line numbers (right side only) + const hunkMatch = line.match(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/); + if (hunkMatch) { + lineNum = parseInt(hunkMatch[1], 10) - 1; + continue; + } + if (!line.startsWith('-')) lineNum++; + + const content = (line.startsWith('+') ? line.slice(1) : line).trim(); + + if (!inTargetBlock) { + // Look for the resource block header + if (content.includes(resourceHeaderPattern)) { + inTargetBlock = true; + blockDepth = 1; + } + } else { + // Track block depth to know when we've exited the resource block + blockDepth += (content.match(/\{/g) ?? []).length; + blockDepth -= (content.match(/\}/g) ?? []).length; + + if (blockDepth <= 0) { + // Exited the resource block — attribute not found in this block + inTargetBlock = false; + blockDepth = 0; + continue; + } + + // Check if this line matches the attribute pattern + if (content === attributePattern && !line.startsWith('-')) { + return lineNum; + } + } + } + + // Header not found in diff — fall back to simple map lookup + // (handles cases where only the attribute line changed, not the whole block) + return lineMap.get(attributePattern) ?? null; +}