Skip to content

Kernel: a dependency update needs no model review - #70

Merged
don-petry merged 3 commits into
mainfrom
kernel/dependency-update-no-model-review
Oct 15, 2026
Merged

don-petry merged 3 commits into
mainfrom
kernel/dependency-update-no-model-review

Conversation

@don-petry

@don-petry don-petry commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Closes the fourth gap of the epic "Epic: nine gaps in the Kernel, found in review": a pull request of class dependency with green checks waited for a model review, and one was admitted.

Stacks on "Kernel: an unknown merge state defers" (#68), which stacks on "Kernel: a Pause stops approvals and merges, not only Runs" (#67). It will be rebased onto main once they merge.

What changes

  • kernel/derive.py: one function, required_model_lenses(facts, config), says which model Lenses an item needs, and the Kernel uses it where it waits for reviews. A dependency update needs none.

  • A dependency update is a pull request of a class named in required_lenses.model_not_required_for whose head was pushed by the identity that opened it. The class is worked out from who opened it, and the last pusher is GitHub's record of the push, not a commit's free-text author. Anyone else's push, including the author Role's fix after a failed check, puts their content in it, and it needs pr-review again. No new fact is needed.

  • Following the architect's ruling on the issue, a Finding a model Lens has already raised gates whatever the class: gating_threads reads every configured model Lens. A bot that recreates its branch after someone else's push therefore cannot drop a thread pr-review opened, a security Finding among them.

  • config/defaults.json and schemas/config.schema.json: the new key, as data (§4.2). The schema allows only dependency; examples/invalid/config.a-change-by-a-person-skips-the-model-review.json lists maintainer and is rejected.

  • required_lenses.deterministic now needs at least one entry, with the counter-example examples/invalid/config.no-deterministic-lens.json. This story makes an empty list dangerous: it would approve a dependency update with no check at all. This is criterion 3 of "Kernel: two smaller gaps in what it is given" (Kernel: two smaller gaps in what it is given #39), and it lands here.

  • tests/test_decide.py: test_a_dependency_update_needs_no_model_review loses @pending, leaves NotYetHeld and asserts both sides. Seven tests join it:

    • every other class needs a model review;
    • a push by someone else needs one too;
    • each deterministic Lens failing sends the update back, and pending makes it wait; secret-scan and code-scanning judge security;
    • a neutral deterministic check does not pass it;
    • a writer's thread still gates, and a stranger's does not;
    • a model Lens's thread gates, including after the bot recreates the branch, with the content changed or unchanged;
    • the order: a Hold, an unknown merge state, a conflict, a Pause and a standing approval still bind.
  • tools/mutants.py: seven mutants.

    • "a dependency update waits for a model review"
    • "a model Lens's thread does not gate a dependency update"
    • "a dependency bot's pull request someone else pushed to skips the model review"
    • "a class the configuration does not name skips the model review"
    • "a dependency update converges before its checks are read"
    • "a dependency update converges before its threads are read"
    • "a neutral deterministic check passes"

    33 of 33 killed.

  • Documents:

    • Decision 0012 loses "not held yet" and gains rows for each boundary.
    • One of those rows, waiting on adapters/, says last_pusher must be the identity GitHub records as pushing the head, never a commit's author or committer. It also says class dependency is given only for an exact App identity, and a fork is always outside.
    • The design's §3.3 says what last_pusher means. §4.2, the dependency-update row of §4.6, the glossary's Class entry and config/AGENTS.md say how a dependency update is recognised.

Acceptance criteria (from "Kernel: a dependency update needs no model review")

  • test_a_dependency_update_needs_no_model_review passes with @pending removed.
  • One function says which model Lenses an item's class requires, and is used where the Kernel waits for reviews. A Finding a model Lens has raised gates whatever the class. Each has a test and a mutant.
  • A person's thread still gates a dependency update.
  • No document says not held yet beside this test.

Left open, on purpose

  • Code the bot writes itself (Renovate post-upgrade tasks, vendored code) is still a dependency update. Catching it needs the changed_paths fact of §3.3 and a set of manifest paths in the configuration.
  • That a bot never becomes the last pusher over someone else's surviving commits is bot behaviour, to settle by trial (§6).
  • A same-named check from another App, or a re-run, can hide a failure, because the last entry wins. That belongs to "Kernel: a Lens is read only from the App that reports it" (Kernel: a Lens is read only from the App that reports it #60).
  • _wanted_run reads model[0], which is safe today with one model Lens.
  • What a neutral conclusion means is not changed here; it is pinned only as not passing a dependency update. That is "What each check conclusion means" (Kernel: what each check conclusion means #35).
  • The simulator's world is always of class maintainer, so the tests carry this rule. sim/world.py is shared with other stories in the epic.

Checks and reviews

  • make verify and python3 -m tools.mutants pass on a git archive copy, which avoids the nested worktrees of "Verify passes in a checkout that holds other worktrees" (Verify passes in a checkout that holds other worktrees #64).
  • The new tests were written before the code. The ones that depend on the rule failed first: needing no model review, a writer's thread, the order, and a model Lens's thread gating after the ruling. The others passed throughout, because they pin existing behaviour: the other classes, someone else's push, each deterministic Lens, and neutral. The neutral mutant survived every test until its test was added.
  • The second model family's read is the Gemini code review and the other bot reviewers on this pull request, as the Maintainer directed. Not merged or enqueued; the Maintainer merges.

Closes #32

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@don-petry
don-petry requested a review from a team as a code owner October 15, 2026 00:27
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-15T00:27:13.761656Z d9dd2cb PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: combsmith/software-factory/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 08e504e6-e1f5-4258-be5b-e9c8212989a9
📥 Commits

Reviewing files that changed from the base of the PR and between a411ca9 and dba2a38.

📒 Files selected for processing: 11
  • config/AGENTS.md
  • config/defaults.json
  • docs/architecture/software-factory-design.md
  • docs/decisions/0012-lenses-report-findings-and-the-kernel-decides-convergence.md
  • docs/glossary.md
  • examples/invalid/config.a-change-by-a-person-skips-the-model-review.json
  • examples/invalid/config.no-deterministic-lens.json
  • kernel/derive.py
  • schemas/config.schema.json
  • tests/test_decide.py
  • tools/mutants.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@don-petry
don-petry force-pushed the kernel/unknown-merge-state-defers branch from d55d3ed to 37d3052 Compare October 15, 2026 00:39
@don-petry
don-petry force-pushed the kernel/dependency-update-no-model-review branch from d9dd2cb to 8a600b3 Compare October 15, 2026 00:42
don-petry and others added 3 commits October 14, 2026 19:45
A pull request of class `dependency` with green checks waited for a
model review, and one was admitted. One function, required_model_lenses,
now says which model Lenses an item needs, and both places that read
them use it: where the Kernel waits for reviews, and where it reads
gating threads. A dependency update needs none.

A dependency update is a pull request of a class the configuration names
in `model_not_required_for`, whose head was pushed by the identity that
opened it. The class is worked out from who opened it; anyone else's
push, the author Role's fix among them, puts content of their own in it,
and it needs the model review again. The schema lets the configuration
name only `dependency`, and a counter-example names `maintainer`.

test_a_dependency_update_needs_no_model_review leaves NotYetHeld and
asserts both sides. Tests join it for every other class, for a push by
someone else, for each deterministic Lens failing or pending (secret-scan
and code-scanning judge security), for a person's thread, for a model
Lens's thread, and for the order: a Hold, an unknown merge state, a
conflict, a Pause and a standing approval still bind. Each place the
rule acts has a mutant, and so does each place it could be read too
early.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The architect's ruling on the open question: a dependency update needs
no model review, and a Finding a model Lens has already raised still
gates it, whatever its class. Without that, a bot recreating its branch
after someone else's push dropped the thread pr-review had opened on it,
security Findings among them. gating_threads reads every configured
model Lens again; required_model_lenses stays where the Kernel waits
for reviews. test_a_model_lens_thread_gates_a_dependency_update replaces
the test that said otherwise, and covers the recreate with the content
changed and unchanged; its mutant is inverted to match.

This story makes an empty list of deterministic Lenses dangerous: it
would approve a dependency update with no check at all. The schema now
asks for at least one, with a counter-example. A neutral deterministic
check is pinned as not passing a dependency update, with a mutant that
lets it pass; what each conclusion means is left to its own story.

Decision 0012 gains a row, waiting on adapters, for what the port must
hold: last_pusher is who GitHub records as pushing the head, and class
dependency is given only for an exact App identity. Design §3.3 says
what last_pusher means.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The two configuration counter-examples were drawn before `max_unmerged_closes` joined the
limits on main, so each was wrong in two ways. Each is now the configuration on main with its
one named fault.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry force-pushed the kernel/dependency-update-no-model-review branch from 8a600b3 to dba2a38 Compare October 15, 2026 00:48
@don-petry
don-petry changed the base branch from kernel/unknown-merge-state-defers to main October 15, 2026 00:48
@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@don-petry
don-petry added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit 96c6f46 Oct 15, 2026
12 of 13 checks passed
@cubic-dev-ai cubic-dev-ai Bot mentioned this pull request Oct 15, 2026
4 tasks done
@don-petry
don-petry deleted the kernel/dependency-update-no-model-review branch October 20, 2026 17:22
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.

Kernel: a dependency update needs no model review

1 participant