Repository navigation
Verify the full schema contract and harden commit and CLI inputs - #28
Merged
Merged
Conversation
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
There was a problem hiding this comment.
💡 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".
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This carries forward the parts of #10 that
maindoesn't have yet and that don't depend on the push-hook design. It's split out so it can land on its own.mainalready 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_contractchecks 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: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 absoluteGIT_DIR, every registered path resolved to the invoking repository and caused false duplicate-identity errors. Real pushes happen to setGIT_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-commitwins over the worktree'sHEAD. 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 addalso waits for a busy database instead of failing.The README is updated for the contract, commit specs, and
--last-commit.Tests
tests/test_hardening.pyadds 13 tests. Against currentmain, 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 passedpyright: 0 errorsruff check(the pinned 0.16.9): no findings🤖 Generated with Claude Code
https://claude.ai/code/session_01Q8BpCkArA34hB1aSyVcnZp
Generated by Claude Code