Add comprehensive missing attribution test suite#915
Closed
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
jwiegley
force-pushed
the
johnw/missing-notes
branch
from
f315e33 to
4dbbd4b
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jwiegley
force-pushed
the
johnw/missing-notes
branch
from
dd94f99 to
b274ec6
Compare
Collaborator
Author
|
Addressed Devin Review's finding about Fixed in 7a37d84: moved both calls from |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
jwiegley
force-pushed
the
johnw/missing-notes
branch
from
5c1556e to
0d323e1
Compare
jwiegley
force-pushed
the
johnw/missing-notes
branch
from
0d323e1 to
0120724
Compare
jwiegley
force-pushed
the
johnw/missing-notes
branch
2 times, most recently
from
9534ff5 to
989e890
Compare
Document 18 failure hypotheses (H1-H18) covering every identified scenario where AI attribution can be silently lost: JSONL corruption, SHA mismatches, partial checkpoints, binary files, merge commits, INITIAL file corruption, and multi-agent conflicts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
27 integration tests covering 18 failure hypotheses for silent AI attribution loss. Tests exercise JSONL corruption, SHA mismatches, partial checkpoints, binary files, merge commits, INITIAL file corruption, multi-agent conflicts, and forward-compatibility edge cases. All tests pass (26 ok, 1 ignored observational). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Three missing_attribution tests (H6d, H7a, H18) intentionally create corrupt checkpoint data to document failure modes. In daemon mode, the daemon reports errors when processing these, causing wait_for_daemon_completion_sessions to panic before the test can check the outcome. Add allow_daemon_errors flag to TestRepo so these tests can indicate that daemon errors are expected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The allow_daemon_errors flag was only checked in wait_for_daemon_completion_sessions but not in the structurally identical wait_for_daemon_completion_count or wait_for_daemon_total_completion_count. This could cause tests that set allow_daemon_errors(true) to still panic if their execution path went through the other wait functions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sanitize_git_env_for_daemon() and disable_trace2_for_daemon_process() use unsafe env::remove_var/set_var which are not thread-safe on POSIX. They were being called from run_daemon() inside runtime.block_on(), but Builder::new_multi_thread().build() already spawns worker threads. Move these calls to handle_run() before the runtime is constructed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…h analysis Make run_daemon pub(crate) since it's only called from handle_run internally. Add CodeQL config to exclude test directory from analysis, suppressing 51 false-positive rust/path-injection alerts from TempDir paths in integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etup) The paths-ignore option in codeql-config.yml is only supported with advanced setup (workflow file), not GitHub's default CodeQL setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…erification Replace set_contents_no_stage() with direct fs::write() + explicit git-ai checkpoint calls across all 27 test scenarios (H1-H18). Add assert_lines_and_blame() assertions to every test, verifying per-line authorship attribution rather than merely checking that attestation maps are non-empty. This catches subtle attribution bugs that the previous existence-only checks would miss. Tests that use custom agent names not in AI_AUTHOR_NAMES (H11, H13) use direct blame output verification instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jwiegley
force-pushed
the
johnw/missing-notes
branch
from
989e890 to
17d10f5
Compare
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
3 participants
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.
Summary
Test plan
cargo test --test integration missing_attribution -- --test-threads=1passes (26 ok, 1 ignored)cargo fmt -- --checkpassescargo clippy --all-targets -- -D warningspasses🤖 Generated with Claude Code