Skip to content

Deploys on merge: a merge that deploys is a release - #105

Merged
hhff merged 9 commits into
mainfrom
deploys-on-merge
Nov 6, 2026
Merged

hhff merged 9 commits into
mainfrom
deploys-on-merge

Conversation

@hhff

@hhff hhff commented Nov 5, 2026

Copy link
Copy Markdown
Member

TL;DR

  • settings release takes "merge": production deploys whenever main changes (Heroku's automatic deploys, a deploy-on-push workflow). Also { "on": "merge", "paths": ["app/**"] } when only some paths deploy, and { "branch": <main_branch> }.
  • on such a project, a merge that changes what deploys is a release. It needs release-production's authority: the class on rung 3, or a release-production decision naming that ticket (decide --class release-production --refs <ticket> [--approved <qid>], then merge <ticket> --decision <d-id>).
  • the decision is checked like an act's: it isn't corrected, it hasn't lapsed after a rung drop, and it names this target. The merge logs a release-production act carrying it.
  • plan merges and record-keeping chores change only the kit's folder, so they don't count as releases.

Why

Merging and releasing were separate decisions. On an app that deploys on merge, every merge skipped your trust setting for release-production. This closes that gap for every ticket.

Docs

  • constitution (who decides: releasing)
  • quality.md §5
  • supervisor.md (merging is releasing)

Tests

The forge-git merge test sets release: "merge":

  • a deploying merge is refused below rung 3
  • an answered release question, then a decision, then merge --decision lands it
  • the release act carries the decision and is irreversible

The suite passes, except one tmux-session test that also fails on main in this environment.


Generated by Claude Code

… is a release and needs release-production's authority

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
…corrected, not lapsed, naming this target); the release act carries it, not the merge

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T5z3hZtpGaQUGsTaAMQrAg
@hhff

hhff commented Nov 5, 2026

Copy link
Copy Markdown
Member Author

Adversarial review (upstream loop): #105 CHANGES, #106 CHANGES

A fresh reviewer reproduced each blocker on Node 18.20.4, in a throwaway project with forge git and a local bare origin. Line numbers are in devteam.mjs on monitoring-uptime.

Blockers

  1. (Monitoring 1: uptime and response time #106) The supervisor can change any owner setting with no owner involved (:2388, :2394).
    • merge accepts --approved if the question's class is any one of the classes the PR opens. A monitors change adds monitor-change, which is a ladder class, not owner_only, so answer --from-chat accepts it.
    • Reproduced: one chore changed monitors, agent_command, gates: {} and secret_commands. The supervisor asked a monitor-change question, answered it --from-chat, and merge --chore … --approved <q> merged all four keys to main.
    • Fix: every changed owner_only class needs its own answer (allow several --approved). A ladder class's answer never covers an owner_only key. Settings PRs refuse channel: 'chat' answers, or monitor-change joins the reserved set.
  2. (Deploys on merge: a merge that deploys is a release #105, Monitoring 1: uptime and response time #106) A hand-written decision line can reuse an owner answer that was already used (decisionAuthorityGap :3749, used by merge :2388, :2411).
    • It checks only that approved_by is an answered question in the class: not that it's unused, nor that it was about this ticket or its approves. CLI decide refuses reuse; a hand-written line skips that.
    • Reproduced: a release answer for ticket-A, reused by an appended decision with refs: ["chore-app"], passed merge --decision. log-check was silent.
    • Note: this function is on main too. The upstream loop is fixing it there in its own PR (the earliest decision or act citing an answer wins; the question's ticket/approves must overlap the decision's refs). Merge main in once it lands.

Should-fix

  1. (both) One decision can be carried out more than once (a monitor-change decision merged twice on two branches). Refuse a decision when an act already carries it for the same target.
  2. (Monitoring 1: uptime and response time #106) A monitor's name can be overridden by m.name or monitor_defaults.name (:6633). Polls then write outside run/; reproduced with ../../../../../ESCAPED-MON. Spread m first, set name last, and refuse a name field.
  3. (Monitoring 1: uptime and response time #106) A new monitor whose site is already down never goes down (:6735, :6731). While unknown, each poll re-seeds fails: 0. Seed only when state.json is missing.
  4. (Monitoring 1: uptime and response time #106) Redirects count as up (:6672; r.status < 400 passes 302), which contradicts the PR and supervisor.md.
  5. (Monitoring 1: uptime and response time #106) Secrets in monitor URLs reach samples, state, the monitor status output, the down observation, the monitor-down finding and wait's ALERT line unredacted (userinfo, and ?token=). Redact userinfo and the query everywhere a URL is shown; refuse credentials in settings.

Nits

  1. "Logged as an act of the class" doesn't hold under --approved (:2396).
  2. NODE_USE_ENV_PROXY does nothing on Node 18, 20 or 22; tlsDaysLeft ignores proxies and sets servername for IP hosts; a poll killed at 60 s leaves its lock for 2 min.
  3. (Deploys on merge: a merge that deploys is a release #105) Deploy-relevant files outside paths (Procfile, package.json, Dockerfile); {paths} without on: "merge" is silently a no-op; plan-undefined in targets.

What held

  • --no-renames catches renames out of deployed paths, and --force, --chore log and plan merges don't bypass the release check.
  • Correction, lapse-after-rung-drop and target checks work, and a project without release: "merge" is unaffected.
  • Rungs rise only on owner-answered trust questions.
  • Monitors run only kind: "http", with no headers or commands, and only in wait / monitor poll.
  • Response bodies never reach prompts or the log.
  • None of the shipped tests cover findings 1–7.

hhff and others added 2 commits November 6, 2026 06:08
…e ticket's repo, and the review's #105 findings

- Resolved against main's multi-repo merge: the release check reads the ticket's repo's release
  setting and diff (R), and the release act carries repo like the merge act.
- A release decision is carried out once per target: merge --decision refuses one an act already
  carries for that ticket or chore (review should-fix 3).
- settings release is validated: paths without "on": "merge", unknown keys, or other shapes fail
  validate, and merge fails closed on them (nit 10).
- With release.paths, root build manifests (Procfile, package.json and lockfiles, Dockerfile, ...)
  deploy whatever paths says (nit 10).
- No plan-undefined target when merging a ticket or chore (nit 10).
- A release-production decision is never cited by the merge act, even when nothing deploys.
- Tests: a hand-written decision reusing the release answer is refused at merge --decision (for
  another target, and a second time); a manifest outside paths deploys; a decision carried out
  twice; paths without on: merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hhff

hhff commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Upstream loop: #105 brought up to main, review findings addressed

I merged main in twice: first up to v0.20.1+, then through #126. The second merge was clean. In the first, I resolved the two conflicts in devteam.mjs and kept main's rules (#113 answerUseGap and matched ownerAnswer, #116 question_hash, #115 owner_rules, #123's kit-file check). Head is now 6f43598.

The release check after the merge

Main's merge now lands a ticket in its own repo (R). The release check runs there too:

  • it reads that repo's release (a repo link inherits home's or sets its own);
  • it diffs in that repo;
  • it leaves out that repo's kit line;
  • like the merge act, the release act carries repo.

Before this change, a ticket in another repo was diffed in home, so the release check passed without checking anything.

Findings

  • Blocker 2 (decision reuse): already fixed on main, by decisionAuthorityGap and answerUseGap, which decisionFor calls at merge --decision. New tests in the forge-git merge test:
    • a hand-written release-production decision that reuses the release answer (asked with --ticket bill-01-card) for chore-app is refused: "was asked about bill-01-card, not chore-app";
    • a second hand-written decision for bill-01-card is refused: "already approved a decision ()".
  • Should-fix 3 (a decision carried out twice): fixed for the release path. merge --decision refuses a release-production decision when an act already carries it for the same ticket or chore ("was already carried out for … decide again"). One decision that names several tickets still releases each of them once. Tested with a chore merged under a decision whose release act is already in the log. The general rule, for every command's --decision, is not in Deploys on merge: a merge that deploys is a release #105's scope and is left for main.
  • Nit 10: fixed.
    • Deploy files outside paths: with { on: "merge", paths }, the root build manifests count as deploying even when paths doesn't match them (Procfile, package.json and lockfiles, Dockerfile, app.json, heroku.yml, fly.toml, Gemfile/requirements/pyproject/go.mod/Cargo and the like). quality.md §5 says to list anything else that deploys in paths. Tested: a chore that changes Procfile with paths: ["web/**"] is a release.
    • {paths} without on: "merge": validate refuses it, along with unknown keys or other shapes. merge fails closed on an invalid release, and when release is on merge but there's no merge base. Tested.
    • plan-undefined: removed from the targets. It was not reachable before, because dispatch's target check refuses a decision that doesn't name the merge's target.
  • A release-production decision is no longer cited by the merge act, even when nothing deploys. The command prints that it was not carried out.

What held (re-checked after the merge)

  • The changed-files diff still uses --no-renames.
  • The release check doesn't depend on --force.
  • --chore log and plan merges change only the kit line, so they aren't releases. A plan or chore that changes app files is one.
  • The decision goes through decisionFor: not corrected, not lapsed after a rung drop, and its recorded authority checked against the log.
  • tag and {branch} projects are unaffected.
  • Docs: quality.md §5 and supervisor.md.

Tests

  • Full suite on Node 24 (--test-name-pattern '^devteam kit$', tmux shim kept): 49/49 pass.
  • CI at 6f43598: success.

🤖 Generated with Claude Code

…rmed release fails closed only when it could mean merge; the diff fails closed

- A release-production decision under answer or default authority releases only the targets its
  question named (--ticket, or --approves for a batch); a question naming neither releases nothing.
- A malformed release that could mean deploy-on-merge fails validate and merge; any other is a
  warning, read as not deploying on merge ({"branch": "production"} is unaffected).
- More build files deploy whatever paths says (.npmrc, .yarnrc.yml, .yarn/, Aptfile, .slugignore,
  bin/pre_compile, bin/post_compile, .profile.d/, Dockerfile.*, next.config.*, tsconfig.json,
  vite.config.*, ...); the owner lists the rest in paths.
- The release check reads the diff itself: a failed diff fails closed instead of reading as no files.
- With paths, the kit's line (and so project/log) never deploys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hhff

hhff commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Upstream loop: second review of #105 (at 6f43598) addressed in 60d3b8c

  • Blocker 1 (a hand-written "default" decision with no rung check): this is on main. It's being fixed there in A decision on a default needs rung 2, the deadline passed, and the answer unused #127. Once that lands I'll merge main into this branch again.
  • Blocker 2 (one answer releases tickets it never asked about): fixed. A release-production decision under answer or default authority now releases only the targets its question named: the question's ticket (a ticket id or chore-<slug>), or its approves for a batch (ask --approves a,b). A question that names neither releases nothing. The check is releaseAnswerGap, called from the release check in merge. supervisor.md's batch line and quality.md §5 now say this. The refusal message now says to ask --ticket <target> first. Tests:
    • ask with no ticket, then decide --refs chore-procfile: refused ("names no ticket or target");
    • a question with --ticket bill-01-card, decided with --refs bill-01-card chore-procfile: refused for chore-procfile.
  • Should-fix 3 (a malformed release blocks every merge): fixed. A malformed release that could mean deploy-on-merge ("merge", or an object with on: "merge") is a validate error, and merge fails closed on it. Any other malformed shape is a warning and reads as not deploying on merge. {"branch":"production"} is valid and unaffected. Tested both ways: {on:"merge", paths:"web/**"}, and {branch:"production", when:…}, which warns without blocking.
  • Should-fix 4 (gaps in the build-file list): fixed. I added .npmrc, .yarnrc(.yml), .yarn/**, Aptfile, .slugignore, bin/pre_compile, bin/post_compile, .profile.d/**, Dockerfile.*, .dockerignore, next.config.*, vite.config.*, tsconfig.json, npm-shrinkwrap.json, pnpm-workspace.yaml, bun.lock and uv.lock. Entries are globs now. quality.md §5 says the list can't be complete and that the owner lists anything else that deploys in paths.
  • Should-fix 5 (changedFiles fails open): fixed for the release check. The check runs the diff itself, and a failed git diff fails closed ("the diff … failed; merge can't tell what deploys"). Tested with a git shim that fails that diff. Elsewhere, changedFiles still reads a failed diff as an empty list; that's on main and not changed here.
  • Should-fix 8 (paths doesn't leave out the kit line): fixed. With paths, files in the kit's line (devteamLine(), which includes project/ and so project/log) are left out before matching, so paths: ["docs/**"] doesn't turn log chores into releases.
  • Nits 6 and 7: noted here and not changed in this PR.

Tests: the full suite on Node 24 (--test-name-pattern '^devteam kit$') passes 49/49. CI at 60d3b8c: success.

🤖 Generated with Claude Code

@hhff

hhff commented Nov 6, 2026

Copy link
Copy Markdown
Member Author

Merging (upstream loop).

  • Reviews: two independent adversarial reviews; the second re-ran every repro at 2bffaa7 and all are refused (APPROVE). Blocker 1 was fixed on main in A decision on a default needs rung 2, the deadline passed, and the answer unused #127.
  • Tests: local Node 24.14.1 full suite 129/129. CI 18/20/22/24 green.
  • Left for the owner: refusing reuse of any decision that an act already carried out (for every --decision, not just releases). Nits on mistyped "Merge" and the default-deadline check are noted for a follow-up.

@hhff
hhff merged commit 012e082 into main Nov 6, 2026
4 checks passed
hhff added a commit that referenced this pull request Nov 6, 2026
…sions runs in line

forge git now asserts release-production is below rung 3, which the decisions step's correction
leaves: a later step reads it, so decisions no longer forks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants