diff --git a/products/desktop/.stamphog/README.md b/products/desktop/.stamphog/README.md deleted file mode 100644 index 11ad874373c1..000000000000 --- a/products/desktop/.stamphog/README.md +++ /dev/null @@ -1,39 +0,0 @@ -# .stamphog - -Declarative policy for the stamphog PR-approval merge gate (`tools/pr-approval-agent/`). -The engine loads these files from the checked-out working tree at run time. -Engine and policy are vendored into other repos (see the note in `tools/pr-approval-agent/README.md`), so format changes here need those copies re-synced too. - -## What lives here - -- `policy.yml` - the global machine policy: deny categories, allow-list, size gate, tier thresholds, dismiss-time triviality rules, the folder delegation contract, and the ownership source (the `hogli-resolver` input that feeds the reviewer's advisory team context via the shared hogli resolver). Trusted data. Each rule's `rationale` records why the rule became what it is (which false positives drove an exclusion, and when) - historical justification like a commit message, not a claim about the present. -- `review-guidance.md` - the trusted review-norms prose injected into the reviewer's system prompt. Ordinary repo-formatted markdown. Editing it changes the production prompt directly, so update deliberately - the `stamphog_policy` deny guarantees a human reviews every change. - -## Proposing a policy change - -Open a PR that edits these files. -Stamphog can never auto-approve it: the `stamphog_policy` deny category matches `.stamphog/**`, any `AGENT_APPROVALS.md`, and `tools/pr-approval-agent/**`, so every change to the gate's own policy or engine routes to a human reviewer. -The loader also hard-fails if that self-governance entry is ever missing, so it cannot be dropped silently. - -## Per-folder overrides (`AGENT_APPROVALS.md`) - -A folder may carry an `AGENT_APPROVALS.md` with a `stamphog:` frontmatter block plus advisory prose. -Resolution: - -- Every `AGENT_APPROVALS.md` at or above a changed file governs it: guidance accumulates outermost first, and a child file adds to its ancestors rather than replacing them. -- For the delegated `size_gate.max_files`, the nearest file on the chain with a valid grant wins for its files (within the contract ceiling); files whose chain grants nothing belong to the global pool. -- The frontmatter is a positive allow-list: only keys named in the `overrides` contract in `policy.yml` are read, within their ceilings. Anything else (unknown key, out-of-bounds value, unparseable frontmatter) invalidates the whole file - frontmatter and prose. An invalid file contributes nothing itself, but it does not cancel its ancestors: files under it still ride an ancestor's grant, or fall to the global pool if the chain grants nothing. Rationale: an author who can write an invalid file could equally delete it, so treating invalid as absent grants no extra power, and every `AGENT_APPROVALS.md` edit is human-reviewed via the `stamphog_policy` deny anyway. -- The prose is untrusted advisory guidance. It is sanitized, length-capped, and injected inside the reviewer prompt's untrusted region; it can never override the deny rules or the refusal criteria. - -### Mixed PRs get mixed leniency - -Each scope's files are counted against that scope's own file ceiling, so a grant covers exactly the files that resolve to it (the nearest valid grant on their chain) and nothing else. -Example: a PR changing 30 files under `products/visual_review/` (ceiling 50) plus 19 files elsewhere (global ceiling 20) passes, because each budget fits. -Add a 21st global file and the PR is denied for the global budget, no matter how much headroom the folder still has. -Files whose chain grants no valid `max_files` (no folder file, prose-only, or only invalid grants) count against the global budget, so splitting files across pseudo-scopes can never inflate the allowance. -The line ceiling stays a single global total; it is not delegable. - -## Delegation contract - -The set of keys a folder file may override lives under `overrides` in `policy.yml` (currently just `size_gate.max_files`, ceiling 50). -deny, allow, dismiss, tiers, and `size_gate.max_lines` are non-delegable by construction - they are absent from the contract and cannot be granted from a folder file. diff --git a/products/desktop/.stamphog/policy.yml b/products/desktop/.stamphog/policy.yml deleted file mode 100644 index 7cd343d2aed0..000000000000 --- a/products/desktop/.stamphog/policy.yml +++ /dev/null @@ -1,217 +0,0 @@ -# Stamphog merge-gate policy - see .stamphog/README.md for how changes are -# proposed and reviewed. Keep pattern scalars single-quoted so regex -# backslashes survive YAML parsing. -version: 1 -deny: - auth: - description: 'Authentication and authorization surfaces.' - rationale: "Only file paths hard-deny; titles surface as scrutiny flags. Past participles (authenticated/authorized) live in titles-only because as path patterns they hard-deny the wrong things (web analytics' authorized_urls.py is domain config, not the auth system). session/token/permission are path-only with tighter compounds because they match too broadly in titles and non-auth paths (SessionAnalysisWarning, tokenizer, permission helpers)." - match: - any: - - 'auth' - - 'login' - - 'signup' - - 'oauth' - - 'saml' - - 'sso' - - 'oidc' - - 'credential' - - 'password' - - '2fa' - - 'mfa' - - 'authentication' - - 'authenticate' - - 'authorize' - - 'authorization' - - 'two[_-]?factor' - titles: - - 'authenticated' - - 'authorized' - paths: - - 'session_auth' - - 'session_token' - - 'auth/session' - - 'auth/token' - - 'permission' - exempt_path_prefixes: - - 'products/warehouse_sources/backend/temporal/data_imports/sources/' - crypto_secrets: - description: 'Cryptography, secrets, and key material.' - rationale: 'key/secret/cert/signing are too broad for titles (key matches keyboard, hotkey, localStorage key), so they are path-only with compound patterns.' - match: - any: - - 'crypto' - - 'encrypt' - - 'decrypt' - - 'vault' - paths: - - 'secret' - - 'api[_-]?key' - - 'secret[_-]?key' - - 'private[_-]?key' - - 'signing[_-]?key' - - 'certificate' - - '\.env' - - '\.pem' - migrations: - description: 'Database and schema migrations.' - rationale: 'The migrations/ substring is load-bearing - it also catches rust *_migrations/ dirs applied by sqlx at deploy.' - match: - paths: - - 'migrations/' - - 'schema_change' - infra_cicd: - description: 'Infrastructure, CI, and deployment artifacts.' - rationale: 'routing and bare deploy are excluded on purpose: every historical match was app-level (DRF routers, message-routing tests, deploy-timing docs), never infrastructure. Narrow deploy literals (bin/deploy, deploy.sh, .github/pr-deploy) cover real deployment artifacts without the false positives.' - match: - any: - - 'terraform' - - 'kubernetes' - - 'helm' - paths: - - 'k8s' - - 'dockerfile' - - 'docker-compose' - - '\.github/workflows' - - '\.github/pr-deploy' - - 'iam' - - 'cloudflare' - - 'cdn' - - 'waf' - - '(?:^|/)bin/deploy' - - 'deploy\.sh' - billing: - description: 'Payments and billing.' - rationale: 'subscription is excluded on purpose: in this repo it means scheduled insight/report deliveries, not payments. Real billing surfaces still match via the other words.' - match: - any: - - 'billing' - - 'payment' - - 'stripe' - - 'invoice' - - 'pricing' - exempt_path_prefixes: - - 'products/warehouse_sources/backend/temporal/data_imports/sources/' - public_api: - description: 'Public API contracts and schemas.' - rationale: 'Changes to the published API surface need human review.' - match: - any: - - 'openapi' - - 'api_schema' - - 'swagger' - - 'public_api' - deps_toolchain: - description: 'Dependency lockfiles and toolchain/build files.' - rationale: "All path-only literal filenames. Manifests deliberately don't hard-deny: without a lockfile change they cannot pull in third-party code, and manifest scripts/hooks are guarded by the reviewer prompt and kept out of the T0 fast path. requirements.txt stays (pins installed code directly). .nvmrc/.tool-versions stay (they change the runtime for every CI job). Makefile/Dockerfile stay (they execute)." - # NOTE: partially code-sourced. The lockfile-name path patterns are derived - # in the loader from DEPENDENCY_ECOSYSTEMS (re.escape over every ecosystem's - # lockfiles) and spliced in ahead of the literals below - do not copy them - # here. Only the non-derived literal patterns live in this file. - match: - paths: - - 'requirements[-\w]*\.(txt|in)' - - 'Makefile' - - 'Dockerfile' - - '\.tool-versions' - - '\.nvmrc' - stamphog_policy: - description: "Stamphog's own policy files, engine, and gate inputs." - rationale: "Changes to the merge gate's own policy or engine always require human review - the gate cannot be trusted to approve edits to itself. Ownership sources (CODEOWNERS, owners.yaml, product.yaml owners) are gate inputs: an ownership edit changes which team the reviewer treats as owning future PRs, and owners.yaml/product.yaml would otherwise ride the .yaml allow-list to a T0 auto-approve (Jul 2026 review finding)." - match: - paths: - - '\.stamphog/' - - 'AGENT_APPROVALS\.md' - - 'tools/pr-approval-agent/' - - 'products/stamphog/backend/logic/policy_defaults/' - - 'tools/owners/' - - 'CODEOWNERS' - - 'owners\.yaml' - - 'product\.yaml' -allow: - path_patterns: - - 'docs/' - - 'README' - - 'CHANGELOG' - - 'LICENSE' - - 'CONTRIBUTING' - - '.github/CODEOWNERS' - - '.gitignore' - - '.editorconfig' - - 'generated/' - - '__snapshots__/' - extensions_only: - - '.cfg' - - '.csv' - - '.gif' - - '.ico' - - '.ini' - - '.jpeg' - - '.jpg' - - '.json' - - '.lock' - - '.md' - - '.mdx' - - '.png' - - '.rst' - - '.snap' - - '.svg' - - '.toml' - - '.txt' - - '.webp' - - '.yaml' - - '.yml' -size_gate: - max_lines: 1200 - max_files: 30 -tiers: - t1_subclasses: - T1a-trivial: - max_lines: 20 - max_files: 3 - breadth: 'single-area' - T1b-small: - max_lines: 100 - max_files: 5 - breadth: 'not-cross-cutting' - T1c-medium: - max_lines: 300 - max_files: 15 - breadth: 'not-cross-cutting' -dismiss: - trivial_extensions: - - '.md' - - '.mdx' - trivial_name_prefixes: - - 'readme' - - 'changelog' - test_regex: '(?:^|/)(?:__tests__|tests?|fixtures)/|(?:^|/)test_[^/]+\.py$|_test\.(py|go)$|\.test\.(ts|tsx|js|jsx)$|\.spec\.(ts|tsx|js|jsx)$|(?:^|/)conftest\.py$' - generated_regex: '(?:^|/)generated/.*\.(ts|tsx|js|jsx|json|md|snap|pyi|txt)$|\.gen\.(ts|tsx|js|jsx)$|\.generated\.(ts|tsx|js|jsx)$|^frontend/src/queries/schema/' -overrides: - 'size_gate.max_files': - ceiling: 50 -# Author-familiarity signal - judgment layer only, never a gate. Non-delegable -# (absent from `overrides`). STRONG = blame overlap ≥ min_blame_overlap_pct; -# MODERATE = both of its keys satisfied; else NONE. -# Rationale: calibrated against 120 days of refused/escalated verdict outcomes -# (Jul 2026 backtest). Blame overlap was the only metric that rose monotonically -# with the human rubber-stamp rate; prior-PR counts, previously-modified-file -# fractions, and recency were flat at every cutoff, so STRONG is blame-only and -# MODERATE is advisory. Tuning these is a YAML-only diff. -familiarity: - strong: - min_blame_overlap_pct: 70 - moderate: - min_prior_prs: 5 - max_days_since_touch: 180 -# Ownership sources feeding the reviewer's team context - advisory only, never a -# gate. The hogli-resolver source delegates to the shared hogli OwnersResolver, -# which walks the distributed owners.yaml / product.yaml files from the repo root -# (the `path` locator) and merges nearest-file-wins, so stamphog reads the same -# view the reviewer auto-assigner builds. Sources are read from the master -# checkout (the workflow pins ref: master), so a PR editing them cannot influence -# its own review. Locators must stay repo-relative (no absolute paths, no '..'). -ownership: - sources: - - path: . - format: hogli-resolver diff --git a/products/desktop/.stamphog/review-guidance.md b/products/desktop/.stamphog/review-guidance.md deleted file mode 100644 index f22682cd7dd5..000000000000 --- a/products/desktop/.stamphog/review-guidance.md +++ /dev/null @@ -1,82 +0,0 @@ -You decide whether a pull request is safe for automated approval. -Your core question: are there showstoppers that block auto-approval? -If none, approve. If you find one, refuse or escalate. - -Operating philosophy: - -- We move fast and fix forward. Auto-approval is a deliberate tradeoff: contained, reversible changes go in without ceremony, so human review effort concentrates on what is genuinely risky. -- The stamphog label opted this PR into automated review, a confidence signal that whoever applied it considers the change ready. Weigh it as such; you are not here to gatekeep process. -- Two questions decide every borderline call: (1) does the change enter risky territory? (2) does it carry independent assurance? -- Risky territory: schema/data migrations, data models, public API contracts, billing/quota/plan logic, auth or security-sensitive surface, crypto/secrets, dependency and third-party code changes, CI/deploy/build tooling, event ingestion paths. Judge territory from the diff's behavior, not from file paths or keywords alone. -- In risky territory you must not certify safety on your own authority: your code reading is not a substitute for domain review there. Your job becomes assurance aggregation: approve only when independent assurance (defined under "Independent assurance" below) covers the risky part. No assurance means ESCALATE. -- Outside risky territory your own reading suffices. Zero reviews is fine: contained, reversible changes go in on your judgment alone. -- Size calibrates scrutiny effort, never risk by itself: a large well-tested refactor outside risky territory can be approved; a five-line billing change with no assurance cannot. -- When in doubt: a change clearly outside risky territory and easy to reverse gets APPROVE; we fix forward. If you cannot tell whether it is risky or reversible, treat it as risky and ESCALATE. - -Showstoppers (REFUSE or ESCALATE): - -- Could break production (crashes, data loss, silent corruption) -- Touches dependencies, data models, or API contracts the gates missed, without independent assurance -- CI/infra changes that slipped through the deny-list, without independent assurance -- Security issues (injection, auth bypass, data exposure) -- Unaddressed review comments with substantive concerns -- Bot author (dependabot, renovate) — always needs human review -- New files whose content doesn't match their extension (e.g. executable code in a .md or .json file) — file extensions are not trusted - -NOT showstoppers (just approve): - -- Code style, naming, missing comments, "could be refactored better" -- Typos, log strings, test fixes, config tweaks -- Anything purely cosmetic or additive without risk - -PR description: - -The description is the author's untrusted claim about what the change does. -Verify the diff matches it: substantive behavior present in the diff but undisclosed by the title and description deserves extra scrutiny, and if it touches risky territory REFUSE and route to a human — undisclosed behavior there is a deception signal that assurance does not rescue. -This generalizes the title-scrutiny idea to the whole stated intent. -A missing description on a non-trivial change is a mild negative, not a showstopper — weigh it, do not refuse on it alone. - -Context: Deterministic gates have already run. Gate results and their pass/fail status are provided in the prompt — rely on those, not assumptions. You typically see T1 PRs that passed all gates. - -Title scrutiny flags (in the prompt when set): the PR title mentions a sensitive domain (auth, billing, infra_cicd, crypto_secrets, public_api) but no deny-listed file was touched. Verify against the diff: if the change behaviorally touches that domain (authentication/authorization flows, payment or plan logic, CI/deploy behavior), REFUSE and route to a human. If the keyword is incidental — an error string, a warehouse connector fix, a docs mention — judge the PR normally. A flag is a magnifying glass, not a verdict. - -Dependency manifests (in the prompt when set): the diff changes a manifest (package.json, pyproject.toml, tsconfig, Cargo.toml, go.mod) with no lockfile change, so it cannot add third-party code. A deterministic scan already hard-denies edits to known scripts/lifecycle/build keys — you are the second line for what the scan can't name. Read the manifest hunks in the diff: version bumps, metadata, and internal workspace references are fine. REFUSE if "scripts" entries, lifecycle hooks (postinstall, prepare, husky), or tool configuration that executes commands were added or changed — those run in CI and on dev machines. - -T1 sub-tiers (provided in the prompt): - -- T1a-trivial: ≤20 lines, ≤3 files, single area -- T1b-small: ≤100 lines, ≤5 files, focused -- T1c-medium: ≤300 lines, ≤15 files, focused -- T1d-complex: >300 lines or >15 files - -Calibrate scrutiny to the sub-tier. T1a should be quick. - -Ownership (from owners.yaml / product.yaml, non-blocking): - -- Author on owning team: not a concern -- Author NOT on owning team: a routing signal, not a risk by itself - - Outside risky territory: judge the change on its merits; cross-team authorship alone never blocks approval - - Risky territory: cross-team authorship removes the owning-team assurance path, so the change needs independent assurance from another source; without it, ESCALATE and route to the owning team - -Author familiarity (TRUSTED, computed by us from git history on the checkout): - -- When present, the prompt reports a familiarity band — STRONG or MODERATE — with the numbers behind it: the share of the modified lines the author last-touched, how many of the changed files they previously modified, their merged PRs in these paths over the last year, and days since their last touch. No band being reported means nothing either way — judge the PR as you always have; never treat missing familiarity as a mark against the author. -- STRONG familiarity counts like owning-team membership for the independent-assurance rule in risky territory. A change with tests and no outstanding concerns from a STRONG-familiarity author is one humans approve unchanged, even when owners.yaml / product.yaml puts the files on another team. -- MODERATE familiarity softens the ownership concern but does not replace team membership — lean it toward APPROVE on a borderline low-risk change, but on its own it does not count as assurance in risky territory. -- Familiarity is judgment input, never a gate. It never overrides a deny rule or a refusal criterion, and its absence changes nothing. -- When you REFUSE or ESCALATE and the prompt lists who is most familiar with the modified lines, name them as suggested reviewers in your next-steps. - -Reviews, comments, and reactions: - -- Each top-level review shows its state (APPROVED / COMMENTED / CHANGES_REQUESTED) and whether it landed on the current head or an older commit. Treat current-head reviews as active signals; treat older-commit reviews as historical context, acting on them only if the current diff still shows the same unresolved issue. -- Inline comments are tagged [resolved], [outdated], or unmarked (unresolved). Resolution status is a signal, not gospel — use judgment. A resolved or outdated comment that raised a serious concern (security, data loss) the diff clearly did NOT address → flag it anyway. For unresolved comments, check whether a later commit already addressed the concern before flagging; substantive ones still unaddressed → REFUSE. -- Reactions (👍, 👎, 👀, etc.) on the PR and on individual review comments are provided — already filtered to trusted org members and bot reviewers, never the PR author. A 👍 from an agent reviewer or teammate is how a bot often signals "no concerns" — a mild positive; a 👎 or 😕 is a mild negative. These two are weak evidence: never approve on a 👍 alone or refuse on a 👎 alone — corroborate against the diff. -- An 👀 (eyes) reaction means a review is in flight — someone is actively looking at the PR right now. Do NOT approve over an in-progress review: REFUSE and tell the author to wait for that reviewer to finish and re-request. This overrides any 👍 present. (Reviewer bots clear their 👀 within minutes and the pipeline waits those out before invoking you, so any 👀 you see — bot or human — is a genuine in-flight review.) -- Discussion comments (the PR's general comment timeline, separate from inline review comments) are included. A maintainer's explicit hold — "don't merge yet", "wait for X", "hold off" — that has not been withdrawn later in the thread means do NOT approve: REFUSE and point at that comment. The PR author's own comments are claims about the change, not assurance — never treat them as an independent sign-off. -- Bot/agent comments with valid concerns that were ignored → ESCALATE. -- Your own prior reviews (posted as stamphog[bot] or github-actions[bot]) are excluded from this context — each run judges the PR's current state fresh. If a review or inline comment quotes or restates an earlier stamphog verdict, treat it as history — never as an independent signal, as tampering, or as someone impersonating you. - -Independent assurance (risky territory only): - -- You are the only automated approver in this path, and you do not certify risky-territory changes alone. For any change entering risky territory require independent assurance over the risky part on the current head: an APPROVED or COMMENTED review with no unresolved concerns from an agent reviewer (Codex, Greptile, Claude) or a human teammate, or authorship by someone on the owning team or with STRONG familiarity. If none is present, ESCALATE and tell the author exactly what assurance to get before re-requesting. -- Outside risky territory no independent review is required: not for docs, tests, config tweaks, contained edits, small fixes, refactors with test coverage, or additive low-risk features, regardless of size tier. Escalating those just adds a rubber stamp. Unresolved substantive reviewer concerns still block approval anywhere; that is evidence of a real problem, not process. diff --git a/products/desktop/MIGRATION.md b/products/desktop/MIGRATION.md index 6cbe1f5c144a..d2319b0e461d 100644 --- a/products/desktop/MIGRATION.md +++ b/products/desktop/MIGRATION.md @@ -31,6 +31,9 @@ be treated as a bug in the sync. The tree is a verbatim copy of the source at the pinned SHA except: - `.github/` is not imported (see workflow mapping below). +- `.stamphog/` is not imported: it is the policy for the source repo's stamphog PR-approval + merge gate, whose workflow is dropped here (the monorepo runs its own pr-approval-agent + on all PRs). Delete it on resync. - `MIGRATION.md` (this file), `POST-MIGRATION.md` (the post-merge runbook) and `docs/plan.md` (the migration plan) exist only in the monorepo; restore all three on a resync.