feat(ci): no-magic-numbers (eslint) + PLR2004 (ruff) with allowlist (#258) (#310)

krokoko · bgagent · cursoragent · web-flow · commit d97b780587fc · 2026-06-10T19:43:31.000Z
* feat(ci): no-magic-numbers (eslint) + PLR2004 (ruff) with allowlist (#258) Inline magic numbers are a classic AI-generated-code smell (AI007): locally plausible, globally inconsistent. Add lint gates that steer hard-coded values into named constants — or contracts/constants.json when they must agree across languages: - cli: @typescript-eslint/no-magic-numbers as blocking 'error'; baseline fixed (20 violations promoted to named constants). - cdk: same rule as advisory 'warn' (162 baseline hits); flips to 'error' in a follow-up once the baseline is clean. - agent: ruff PLR2004 blocking; baseline fixed (5 violations). - Shared allowlist: 0/1/-1/2, HTTP status codes, radix and unit-conversion factors. Tests are exempt in all three packages. - max_budget_usd bounds (0.01–100) promoted to contracts/constants.json — they were duplicated as literals in cdk validation.ts and cli submit.ts, exactly the drift this contract exists to prevent. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(cli): promote post-merge magic number 72 to BANNER_WIDTH (#258) PR #241 merged into main with a literal banner width in the new `bgagent github` command, tripping the no-magic-numbers rule this branch enables. Same constant name as linear.ts. --------- Co-authored-by: bgagent <bgagent@noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
diff --git a/agent/pyproject.toml b/agent/pyproject.toml
@@ -83,6 +83,10 @@ select = [
     "SIM",   # flake8-simplify
     "TCH",   # flake8-type-checking
     "RUF",   # ruff-specific rules
+    # AI007 guard (#258): magic-value comparisons should use named constants.
+    # Values shared across Python/TypeScript belong in contracts/constants.json
+    # (see contracts/constants.md).
+    "PLR2004",  # pylint magic-value-comparison
 ]
 ignore = [
     "S603",  # subprocess call — allowed for agent CLI operations
@@ -93,7 +97,7 @@ ignore = [
 "src/entrypoint.py" = ["E402"]  # re-export shim: importlib.reload() call must precede re-export from-imports
 "src/system_prompt.py" = ["E501"]  # long prompt strings
 "src/prompts/*.py" = ["E501"]  # long prompt strings
-"tests/**" = ["S101", "S106", "S108", "E402"]  # assert; test tokens; /tmp paths; importorskip
+"tests/**" = ["S101", "S106", "S108", "E402", "PLR2004"]  # assert; test tokens; /tmp paths; importorskip; literal assertions
 
 [tool.pytest.ini_options]
 testpaths = ["tests"]
diff --git a/agent/src/hooks.py b/agent/src/hooks.py
@@ -972,6 +972,11 @@ async def post_tool_use_hook(
 # leak across tasks in the same runtime.
 _INJECTED_NUDGES: dict[str, set[str]] = {}
 
+# Preview length for the first nudge message in the milestone details
+# string.  Kept short so the whole details line stays under ~120 chars
+# (single terminal line in ``bgagent watch``).
+_NUDGE_PREVIEW_LEN = 60
+
 
 def _reset_injected_nudges_for_tests() -> None:
     """Test-only helper to clear the in-process injected-nudge dedup set."""
@@ -1098,10 +1103,10 @@ def _nudge_between_turns_hook(ctx: dict) -> list[str]:
     # Short details string for the stream — preview the first nudge, total
     # count, and the nudge IDs for traceability.  Kept under ~120 chars so
     # it fits on a single terminal line.
-    first_msg = (pending[0].get("message") or "")[:60]
+    first_msg = (pending[0].get("message") or "")[:_NUDGE_PREVIEW_LEN]
     ids = ",".join(str(n.get("nudge_id", ""))[-8:] for n in pending)
     details = f"{count} nudge(s) acknowledged (ids=…{ids}): {first_msg}" + (
-        "…" if count > 1 or len(first_msg) == 60 else ""
+        "…" if count > 1 or len(first_msg) == _NUDGE_PREVIEW_LEN else ""
     )
     # AD-5: emit the ack BEFORE returning the injection list.
     _emit_nudge_milestone(ctx, "nudge_acknowledged", details)
diff --git a/agent/src/linear_reactions.py b/agent/src/linear_reactions.py
@@ -28,6 +28,7 @@
 import os
 import threading
 import time
+from http import HTTPStatus
 from typing import Any
 
 import requests
@@ -177,7 +178,7 @@ def _graphql(query: str, variables: dict[str, Any]) -> dict[str, Any] | None:
             log("WARN", f"linear_reactions: HTTP {resp.status_code} from Linear (auth)")
         return None
 
-    if resp.status_code != 200:
+    if resp.status_code != HTTPStatus.OK:
         log("WARN", f"linear_reactions: HTTP {resp.status_code} from Linear")
         return None
 
diff --git a/agent/src/models.py b/agent/src/models.py
@@ -47,6 +47,9 @@ class MemoryContext(BaseModel):
 # Attachment types — mirrors AttachmentType in cdk/src/handlers/shared/types.ts.
 AttachmentType = Literal["image", "file", "url"]
 
+# A SHA-256 digest rendered as lowercase hex is always 64 characters.
+SHA256_HEX_LEN = 64
+
 
 class AttachmentConfig(BaseModel):
     """Attachment descriptor from the orchestrator — mirrors AgentAttachmentPayload in types.ts."""
@@ -71,7 +74,7 @@ def _validate_integrity_fields(self) -> Self:
         if not self.checksum_sha256:
             raise ValueError("checksum_sha256 is required for integrity verification")
         # checksum must be lowercase hex (SHA-256 = 64 hex chars)
-        if len(self.checksum_sha256) != 64 or not all(
+        if len(self.checksum_sha256) != SHA256_HEX_LEN or not all(
             c in "0123456789abcdef" for c in self.checksum_sha256
         ):
             raise ValueError("checksum_sha256 must be a 64-character lowercase hex string")
diff --git a/agent/src/server.py b/agent/src/server.py
@@ -42,13 +42,17 @@
 _debug_cw_failures_lock = threading.Lock()
 _DEBUG_CW_FAILURE_EMIT_EVERY = 5
 
+# Only redact secrets at least this long — replacing very short strings
+# would mangle unrelated text that happens to contain them.
+_MIN_REDACTABLE_SECRET_LEN = 12
+
 
 def _redact_cached_credentials(text: str) -> str:
     """Remove cached env secrets from debug text before stdout / CloudWatch."""
     out = text
     for env_key in ("GITHUB_TOKEN", "LINEAR_API_TOKEN"):
         secret = os.environ.get(env_key) or ""
-        if len(secret) >= 12:
+        if len(secret) >= _MIN_REDACTABLE_SECRET_LEN:
             out = out.replace(secret, f"<{env_key}_REDACTED>")
     return out
 
diff --git a/agent/src/telemetry.py b/agent/src/telemetry.py
@@ -30,12 +30,15 @@ def get_disk_usage(path: str = AGENT_WORKSPACE) -> float:
         return 0
 
 
+_BYTES_PER_UNIT = 1024
+
+
 def format_bytes(size: float) -> str:
     """Human-readable byte size."""
     for unit in ("B", "KB", "MB", "GB"):
-        if abs(size) < 1024:
+        if abs(size) < _BYTES_PER_UNIT:
             return f"{size:.1f} {unit}"
-        size /= 1024
+        size /= _BYTES_PER_UNIT
     return f"{size:.1f} TB"
 
 
diff --git a/cdk/eslint.config.mjs b/cdk/eslint.config.mjs
@@ -156,6 +156,29 @@ export default [
       }],
 
       // TypeScript rules
+      // AI007 guard (#258): inline numeric literals should be named constants.
+      // Values shared across Python/TypeScript belong in contracts/constants.json
+      // (see contracts/constants.md). Advisory ('warn') until the existing
+      // baseline is cleaned up, then this becomes 'error' like in cli/.
+      '@typescript-eslint/no-magic-numbers': ['warn', {
+        ignore: [
+          // Identity / trivial arithmetic
+          -1, 0, 1, 2,
+          // Radix, percentages, and well-known unit conversions
+          10, 24, 60, 100, 1000, 1024, 3600, 86400,
+          // HTTP status codes (600 = exclusive upper bound for 5xx checks)
+          200, 201, 202, 204, 301, 302, 304,
+          400, 401, 403, 404, 409, 422, 429,
+          500, 502, 503, 504, 600,
+        ],
+        ignoreEnums: true,
+        ignoreNumericLiteralTypes: true,
+        ignoreReadonlyClassProperties: true,
+        ignoreTypeIndexes: true,
+        ignoreDefaultValues: true,
+        ignoreClassFieldInitialValues: true,
+        ignoreArrayIndexes: true,
+      }],
       '@typescript-eslint/no-require-imports': 'error',
       '@typescript-eslint/no-shadow': 'error',
       '@typescript-eslint/no-floating-promises': ['error'],
@@ -219,4 +242,12 @@ export default [
       'jest/no-focused-tests': 'error',
     },
   },
+
+  // Override: tests legitimately use inline literals (fixtures, assertions)
+  {
+    files: ['test/**/*.ts'],
+    rules: {
+      '@typescript-eslint/no-magic-numbers': 'off',
+    },
+  },
 ];
diff --git a/cdk/src/handlers/shared/types.ts b/cdk/src/handlers/shared/types.ts
@@ -1078,3 +1078,12 @@ export const APPROVAL_TIMEOUT_S_DEFAULT = sharedConstants.approval_timeout_s.def
 export const APPROVAL_GATE_CAP_MIN = sharedConstants.approval_gate_cap.min;
 export const APPROVAL_GATE_CAP_MAX = sharedConstants.approval_gate_cap.max;
 export const APPROVAL_GATE_CAP_DEFAULT = sharedConstants.approval_gate_cap.default;
+
+/** Minimum allowed `max_budget_usd` (1 cent). The CLI pre-validates with the
+ *  same bound (`bgagent submit --max-budget`), so it lives in
+ *  ``contracts/constants.json`` rather than as a local literal (#258). */
+export const MAX_BUDGET_USD_MIN = sharedConstants.max_budget_usd.min;
+
+/** Maximum allowed `max_budget_usd` ($100).
+ *  Sourced from ``contracts/constants.json`` (#258). */
+export const MAX_BUDGET_USD_MAX = sharedConstants.max_budget_usd.max;
diff --git a/cdk/src/handlers/shared/validation.ts b/cdk/src/handlers/shared/validation.ts
@@ -24,6 +24,8 @@ import {
   type InlineAttachment,
   type PresignedAttachment,
   type UrlAttachment,
+  MAX_BUDGET_USD_MIN,
+  MAX_BUDGET_USD_MAX,
 } from './types';
 import { type WorkflowRequiredInputs } from './workflows';
 import { TaskStatus } from '../../constructs/task-status';
@@ -36,10 +38,6 @@ export const MIN_MAX_TURNS = 1;
 export const MAX_MAX_TURNS = 500;
 /** Maximum allowed length for task_description. */
 export const MAX_TASK_DESCRIPTION_LENGTH = 10_000;
-/** Minimum allowed value for max_budget_usd (1 cent). */
-export const MIN_MAX_BUDGET_USD = 0.01;
-/** Maximum allowed value for max_budget_usd ($100). */
-export const MAX_MAX_BUDGET_USD = 100;
 
 const REPO_PATTERN = /^[a-zA-Z0-9._-]+\/[a-zA-Z0-9._-]+$/;
 const IDEMPOTENCY_KEY_PATTERN = /^[a-zA-Z0-9_-]{1,128}$/;
@@ -223,7 +221,7 @@ export function validateMaxTurns(value: unknown): number | null | undefined {
 export function validateMaxBudgetUsd(value: unknown): number | null | undefined {
   if (value === undefined || value === null) return undefined;
   if (typeof value !== 'number') return null;
-  if (value < MIN_MAX_BUDGET_USD || value > MAX_MAX_BUDGET_USD) return null;
+  if (value < MAX_BUDGET_USD_MIN || value > MAX_BUDGET_USD_MAX) return null;
   return value;
 }
 
diff --git a/cli/eslint.config.mjs b/cli/eslint.config.mjs
@@ -153,6 +153,28 @@ export default [
       }],
 
       // TypeScript rules
+      // AI007 guard (#258): inline numeric literals should be named constants.
+      // Values shared across Python/TypeScript belong in contracts/constants.json
+      // (see contracts/constants.md).
+      '@typescript-eslint/no-magic-numbers': ['error', {
+        ignore: [
+          // Identity / trivial arithmetic
+          -1, 0, 1, 2,
+          // Radix, percentages, and well-known unit conversions
+          10, 24, 60, 100, 1000, 1024, 3600, 86400,
+          // HTTP status codes (600 = exclusive upper bound for 5xx checks)
+          200, 201, 202, 204, 301, 302, 304,
+          400, 401, 403, 404, 409, 422, 429,
+          500, 502, 503, 504, 600,
+        ],
+        ignoreEnums: true,
+        ignoreNumericLiteralTypes: true,
+        ignoreReadonlyClassProperties: true,
+        ignoreTypeIndexes: true,
+        ignoreDefaultValues: true,
+        ignoreClassFieldInitialValues: true,
+        ignoreArrayIndexes: true,
+      }],
       '@typescript-eslint/no-require-imports': 'error',
       '@typescript-eslint/no-shadow': 'error',
       '@typescript-eslint/no-floating-promises': ['error'],
@@ -215,6 +237,14 @@ export default [
     },
   },
 
+  // Override: tests and build tooling legitimately use inline literals
+  {
+    files: ['test/**/*.ts', 'build-tools/**/*.ts'],
+    rules: {
+      '@typescript-eslint/no-magic-numbers': 'off',
+    },
+  },
+
   // Override: shebang files can't have the license header at line 1
   {
     files: ['src/bin/**/*.ts'],
diff --git a/cli/src/auth.ts b/cli/src/auth.ts
@@ -27,7 +27,8 @@ import { debug } from './debug';
 import { CliError } from './errors';
 import { Credentials } from './types';
 
-const TOKEN_REFRESH_BUFFER_MS = 5 * 60 * 1000; // 5 minutes
+const TOKEN_REFRESH_BUFFER_MINUTES = 5;
+const TOKEN_REFRESH_BUFFER_MS = TOKEN_REFRESH_BUFFER_MINUTES * 60 * 1000;
 
 /** Authenticate with username/password and cache tokens. */
 export async function login(username: string, password: string): Promise<void> {
diff --git a/cli/src/bin/bgagent.ts b/cli/src/bin/bgagent.ts
@@ -111,8 +111,9 @@ if (require.main === module) {
       // keep-alive timeout. Observed in Scenarios 6 and 7-extended
       // deploy validation where ``bgagent watch`` had to be ``pkill``-ed
       // after the task reached COMPLETED.
+      const EXIT_FLUSH_DELAY_MS = 50;
       setTimeout(() => {
         process.exit(process.exitCode ?? 0);
-      }, 50).unref();
+      }, EXIT_FLUSH_DELAY_MS).unref();
     });
 }
diff --git a/cli/src/commands/admin.ts b/cli/src/commands/admin.ts
@@ -46,8 +46,9 @@ export function generateTempPassword(): string {
   const pickFrom = (set: string): string => set[crypto.randomInt(set.length)];
 
   // One required char from each class, then 14 more random chars (>= 12 total).
+  const RANDOM_FILL_CHARS = 14;
   const chars: string[] = [pickFrom(upper), pickFrom(lower), pickFrom(digit), pickFrom(symbol)];
-  for (let i = 0; i < 14; i += 1) {
+  for (let i = 0; i < RANDOM_FILL_CHARS; i += 1) {
     chars.push(pickFrom(all));
   }
 
@@ -209,7 +210,8 @@ function isLikelyEmail(value: string): boolean {
 }
 
 function printInviteSummary(email: string, tempPassword: string, bundle: string): void {
-  const bar = '─'.repeat(64);
+  const SUMMARY_BAR_WIDTH = 64;
+  const bar = '─'.repeat(SUMMARY_BAR_WIDTH);
   console.log();
   console.log(`✓ Created Cognito user ${email}`);
   console.log('✓ Set permanent password (no first-login change required)');
diff --git a/cli/src/commands/github.ts b/cli/src/commands/github.ts
@@ -27,6 +27,9 @@ import { Command } from 'commander';
 import { loadConfig } from '../config';
 import { CliError } from '../errors';
 
+/** Width of the `═` banner rules printed around webhook-info output. */
+const BANNER_WIDTH = 72;
+
 export function makeGithubCommand(): Command {
   const github = new Command('github')
     .description('Manage GitHub integration (deployment-status webhook for preview-deploy screenshots)');
@@ -57,7 +60,7 @@ export function makeGithubCommand(): Command {
           );
         }
 
-        const bar = '═'.repeat(72);
+        const bar = '═'.repeat(BANNER_WIDTH);
         console.log(bar);
         console.log('GitHub webhook configuration (preview-deploy screenshot pipeline)');
         console.log(bar);
diff --git a/cli/src/commands/linear.ts b/cli/src/commands/linear.ts
@@ -52,6 +52,9 @@ const DEFAULT_LABEL_FILTER = 'bgagent';
 /** Standard RFC 4122 UUID — Linear's `projects.nodes[].id` matches this shape. */
 const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
 
+/** Width of the `═` banner bars in printed setup output. */
+const BANNER_WIDTH = 72;
+
 /**
  * Render the printable Linear OAuth app config. Standalone export so
  * `bgagent linear setup` can call it inline (Phase 2.0b setup wizard
@@ -81,7 +84,7 @@ export function renderLinearAppTemplate(opts: LinearAppTemplateOptions = {}): st
   // setup interactively from their machine.
   const callbackUrl = opts.awsCallbackUrl ?? 'http://localhost:8080/oauth/callback';
 
-  const bar = '═'.repeat(72);
+  const bar = '═'.repeat(BANNER_WIDTH);
   return [
     bar,
     'Linear OAuth app template',
@@ -197,7 +200,8 @@ function randomState(): string {
   // uses of this command file.
   // eslint-disable-next-line @typescript-eslint/no-require-imports
   const { randomBytes } = require('crypto') as typeof import('crypto');
-  return randomBytes(32).toString('base64url');
+  const STATE_BYTES = 32;
+  return randomBytes(STATE_BYTES).toString('base64url');
 }
 
 /**
@@ -357,7 +361,7 @@ export function makeLinearCommand(): Command {
           );
         }
         const webhookUrl = `${config.api_url.replace(/\/+$/, '')}/linear/webhook`;
-        const bar = '═'.repeat(72);
+        const bar = '═'.repeat(BANNER_WIDTH);
         console.log(bar);
         console.log('Linear webhook configuration');
         console.log(bar);
@@ -1752,7 +1756,8 @@ async function runSelfLinkPicker(args: {
 export const INVITE_CODE_ALPHABET = 'abcdefghjkmnpqrstuvwxyz23456789';
 
 export function generateInviteCode(): string {
-  const bytes = new Uint8Array(8);
+  const INVITE_CODE_RANDOM_BYTES = 8;
+  const bytes = new Uint8Array(INVITE_CODE_RANDOM_BYTES);
   crypto.getRandomValues(bytes);
   let out = 'link-';
   for (const b of bytes) {
@@ -1876,8 +1881,9 @@ function extractCognitoSub(): string {
   if (!creds?.id_token) {
     throw new Error('not authenticated — run `bgagent login`');
   }
+  const JWT_SEGMENTS = 3; // header.payload.signature
   const parts = creds.id_token.split('.');
-  if (parts.length !== 3) {
+  if (parts.length !== JWT_SEGMENTS) {
     throw new Error('malformed id_token in ~/.bgagent/credentials.json');
   }
   const payload = JSON.parse(Buffer.from(parts[1], 'base64url').toString('utf-8')) as { sub?: string };
diff --git a/cli/src/commands/submit.ts b/cli/src/commands/submit.ts
@@ -33,6 +33,8 @@ import {
   CreateTaskRequest,
   INITIAL_APPROVALS_MAX_ENTRIES,
   INITIAL_APPROVALS_MAX_ENTRY_LENGTH,
+  MAX_BUDGET_USD_MAX,
+  MAX_BUDGET_USD_MIN,
 } from '../types';
 import { exitCodeForStatus, waitForTask } from '../wait';
 
@@ -130,8 +132,8 @@ export function makeSubmitCommand(): Command {
         }
       }
       if (opts.maxBudget !== undefined) {
-        if (isNaN(opts.maxBudget) || opts.maxBudget < 0.01 || opts.maxBudget > 100) {
-          throw new CliError('--max-budget must be a number between 0.01 and 100.');
+        if (isNaN(opts.maxBudget) || opts.maxBudget < MAX_BUDGET_USD_MIN || opts.maxBudget > MAX_BUDGET_USD_MAX) {
+          throw new CliError(`--max-budget must be a number between ${MAX_BUDGET_USD_MIN} and ${MAX_BUDGET_USD_MAX}.`);
         }
       }
       if (opts.approvalTimeout !== undefined) {
diff --git a/cli/src/commands/watch.ts b/cli/src/commands/watch.ts
diff --git a/cli/src/format.ts b/cli/src/format.ts
diff --git a/cli/src/linear-oauth.ts b/cli/src/linear-oauth.ts
diff --git a/cli/src/oauth-callback-server.ts b/cli/src/oauth-callback-server.ts
diff --git a/cli/src/types.ts b/cli/src/types.ts
diff --git a/contracts/constants.json b/contracts/constants.json
diff --git a/contracts/constants.md b/contracts/constants.md
