Skip to content

Verify the full schema contract and harden commit and CLI inputs - #28

Merged
Deep-Sixed merged 1 commit into
mainfrom
claude/fervent-darwin-65ds9y-hardening
Sep 29, 2026
Merged

Deep-Sixed merged 1 commit into
mainfrom
claude/fervent-darwin-65ds9y-hardening

Conversation

@Deep-Sixed

Copy link
Copy Markdown
Owner

This carries forward the parts of #10 that main doesn't have yet and that don't depend on the push-hook design. It's split out so it can land on its own. main already covers #10's self-approval, worktree-continuation and scope fixes (#12, #19). #10's ref-transaction boundary will come separately, rebuilt on #18's hook.

Changes

  • verify_contract checks the whole schema. It replays the bundled migrations into an in-memory SQLite database and requires every table, index and trigger in the store to match (ignoring formatting). It rejects:

    • missing or altered objects;
    • indexes or triggers that shouldn't be there;
    • stores migrated by a newer QuorumGit.

    Move approval authority to the repository and continue retained worktrees #12's check of governance triggers by name still runs first, because its message is clearer.

  • Repository identity no longer follows the hook's GIT_DIR. git_common_dir() drops Git's repository-selecting environment variables. With an absolute GIT_DIR, every registered path resolved to the invoking repository and caused false duplicate-identity errors. Real pushes happen to set GIT_DIR=., which hid the bug.

  • Commit arguments accept any commit spec. verify_commit() resolves abbreviated OIDs, refs and uppercase hex to the full OID, and checkpoints and handoffs store that. Before, a short SHA passed the Git check and then crashed with a traceback on the database's commit-ID check.

  • handoff create --last-commit wins over the worktree's HEAD. Before, it was silently ignored whenever the claim had a worktree.

  • Clean errors instead of tracebacks: a duplicate agent add, a repeated --protected-ref, and malformed operation JSON now give normal errors. task add also waits for a busy database instead of failing.

The README is updated for the contract, commit specs, and --last-commit.

Tests

tests/test_hardening.py adds 13 tests. Against current main, 11 of them fail for the intended reason. The other 2 guard behaviour that must not change: formatting-only schema differences are accepted, and a claim without a worktree must still pass --last-commit.

Validation

  • pytest tests -q: 193 passed
  • pyright: 0 errors
  • ruff check (the pinned 0.16.9): no findings

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp


Generated by Claude Code

Carries forward the parts of PR #10 that main does not have yet and that
do not depend on the push-hook design:

- verify_contract() replays the bundled migrations in memory and
  requires every table, index, and trigger to match the store (ignoring
  formatting). It rejects missing or altered objects, extra indexes or
  triggers, and stores migrated by a newer version; the named-trigger
  check stays first for its clearer message.
- git_common_dir() no longer inherits the hook's repository-selecting
  Git environment, which resolved every registered path to the invoking
  repository whenever GIT_DIR was absolute.
- verify_commit() resolves any commit spec (abbreviated OID, ref,
  uppercase) and returns the full OID; checkpoints and handoffs store
  it instead of failing a CHECK constraint with a traceback.
- handoff create uses an explicit --last-commit over the worktree HEAD.
- Duplicate agents, duplicate --protected-ref values, and malformed
  operation JSON are clean errors; task creation takes the write lock.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d889864d2f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/quorumgit/store.py Resolved
@Deep-Sixed
Deep-Sixed merged commit 0514a69 into main Sep 29, 2026
7 checks passed
Deep-Sixed pushed a commit that referenced this pull request Sep 29, 2026
Review on #28: the schema fingerprint only ignored whitespace around
brackets and commas, so an object recreated with lowercase keywords or
different operator spacing (role<>'' versus role <> '') was reported as
altered and every command refused the store. SQL is now compared as a
token sequence: keyword and bare-name case, whitespace, and comments are
ignored, while quoted literals and quoted identifiers stay exact.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp
Deep-Sixed pushed a commit that referenced this pull request Sep 29, 2026
…ansaction binding

#28 is this branch's findings 5, 6, 8 and 9 split out and merged on
their own, so registry.py and store.py now match main exactly; the
auto-merge had duplicated path_scoped_git_env, the `env=` argument to
git_common_dir, and SCHEMA_MIGRATIONS_DDL, which are removed. README
hunks take main's later wording. The named-trigger contract test here
now expects main's "Missing required governance triggers" message; main
has no other test of that path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014R4wfmENrMnt36KN1Kj6Fx
Deep-Sixed pushed a commit that referenced this pull request Sep 30, 2026
No conflicts. Full suite passes on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp
Deep-Sixed pushed a commit that referenced this pull request Sep 30, 2026
#30 records each validated ref update's paths at pre-receive and replays
them at the reference transaction's `prepared` stage. This PR adds a
second list, scope_paths, measured against the mainline, so both are now
recorded and replayed: migration 007 adds ref_updates.scope_paths, and
evaluate_ref_update() computes whichever list it is not given. An update
validated before 007 (NULL) recomputes its scope paths.

test_prepared_uses_the_paths_pre_receive_derived pushes main and a claimed
branch together; it fails if `prepared` recomputes scope_paths, because
main landing first moves the mainline baseline.

README keeps main's "hooks" wording and this PR's mainline-baseline note.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp
Deep-Sixed pushed a commit that referenced this pull request Sep 30, 2026
Renumber 006_operation_policies.sql to 008, since 006 and 007 now hold
the ref-update transaction tables. Check the required views by name
before the schema fingerprint, as tables and triggers already are, so a
dropped policy view is reported as such.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp
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