Attestation commit code review feedback from #8794

src/github/attestationCommit.ts

@@ -20,7 +20,7 @@ const DEFAULT_ATTESTATION_COMMIT_MESSAGE = 'Attestation commit';
  *
  * Accepts any of the following (checked across local + global git config):
  *  - `user.signingkey` is set, OR
- *  - `commit.gpgsign` is `true` (git will pick a default signing identity), OR
+ *  - `commit.gpgsign` is a truthy git boolean (`true`/`1`/`yes`/`on`), OR
  *  - `gpg.format` is set to `ssh` or `x509` (the user is explicitly opting in
  *    to a non-default signing format).
  */
@@ -48,7 +48,8 @@ async function hasCommitSigningConfigured(repository: Repository): Promise<boole
 	if (signingKey) {
 		return true;
 	}
-	if (commitGpgSign?.toLowerCase() === 'true') {
+	// `commit.gpgsign` is a git boolean: true/1/yes/on (case-insensitive) are all truthy.
+	if (commitGpgSign && ['true', '1', 'yes', 'on'].includes(commitGpgSign.toLowerCase())) {
 		return true;
 	}
 	if (gpgFormat && ['ssh', 'x509'].includes(gpgFormat.toLowerCase())) {

src/github/utils.ts

@@ -1738,20 +1738,10 @@ export function insertNewCommitsSinceReview(
 	}
 
 	if (insertIndex === -1) {
-		// No post-review commits - place the marker right after the commit
-		// that the latest review actually covered.
-		for (let i = 0; i < timelineEvents.length; i++) {
-			if (
-				timelineEvents[i].event === Common.EventType.Committed &&
-				(timelineEvents[i] as Common.CommitEvent).sha === latestReviewCommitOid
-			) {
-				insertIndex = i + 1;
-				break;
-			}
-		}
-	}
-
-	if (insertIndex === -1) {
+		// No commits occurred after the user's most recent review - skip the
+		// marker entirely. Placing it before the review event (e.g. next to a
+		// pre-review attestation commit) would contradict its "New changes
+		// since your last Review" label.
 		return;
 	}
 

src/issues/userCompletionProvider.ts

@@ -167,7 +167,7 @@ export class UserCompletionProvider implements vscode.CompletionItemProvider {
 	}
 
 	private cachedPrUsers: UserCompletion[] = [];
-	private cachedPrTimelineEvents: TimelineEvent[] | undefined = [];
+	private cachedPrTimelineEvents: TimelineEvent[] | undefined;
 	private cachedForPrNumber: number | undefined;
 	private async getCommentSpecificSuggestions(
 		alreadyIncludedUsers: Map<string, IAccount>,

src/test/github/utils.test.ts

@@ -144,7 +144,7 @@ describe('utils', () => {
 			assert.strictEqual((events[3] as CommitEvent).sha, HEAD_SHA);
 		});
 
-		it('does NOT move a commit pushed just BEFORE the user\'s review (e.g. an attestation commit)', () => {
+		it('does NOT insert a marker when the only diverging commit was pushed BEFORE the user\'s review (e.g. an attestation commit)', () => {
 			const reviewTime = new Date('2024-01-01T12:00:00Z');
 			const attestationCommitTime = new Date('2024-01-01T11:59:00Z'); // 1 minute before the review
 			const events: TimelineEvent[] = [
@@ -155,15 +155,14 @@ describe('utils', () => {
 
 			insertNewCommitsSinceReview(events, LATEST_REVIEW_SHA, CURRENT_USER, makeHead(HEAD_SHA));
 
-			// Expected: pre-review commit stays in place (not under NewCommitsSinceReview),
-			// and the marker is still inserted after the review for any later commits (none here).
-			assert.strictEqual(events.length, 4);
+			// Expected: no marker is inserted because there are no commits after the review.
+			// The pre-review attestation commit stays in its chronological place.
+			assert.strictEqual(events.length, 3);
 			assert.strictEqual(events[0].event, EventType.Committed);
 			assert.strictEqual((events[0] as CommitEvent).sha, LATEST_REVIEW_SHA);
-			assert.strictEqual(events[1].event, EventType.NewCommitsSinceReview);
-			assert.strictEqual(events[2].event, EventType.Committed);
-			assert.strictEqual((events[2] as CommitEvent).sha, HEAD_SHA);
-			assert.strictEqual(events[3].event, EventType.Reviewed);
+			assert.strictEqual(events[1].event, EventType.Committed);
+			assert.strictEqual((events[1] as CommitEvent).sha, HEAD_SHA);
+			assert.strictEqual(events[2].event, EventType.Reviewed);
 		});
 
 		it('moves only post-review commits when both pre- and post-review commits exist', () => {