Conversation
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR implements Substack Notes publication history tracking in D1 to prevent duplicate posts. The implementation is well-structured with comprehensive test coverage. However, there are critical issues that must be fixed before merge:
Critical Issues (Must Fix)
- Database update verification missing:
markNotePublishedandmarkNoteUnknowndo not verify that database updates succeeded, creating potential for inconsistent state - Performance regression: Missing index on
content_hashcolumn causes full table scans on duplicate checks, degrading performance with table growth
Positive Aspects
- Well-designed duplicate detection using content normalization and SHA-256 hashing
- Comprehensive test coverage with FakeD1 implementation
- Proper error handling with custom DuplicateNoteError
- Clean separation of concerns with note-history module
Please address the critical issues identified in the inline comments.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| export async function markNoteUnknown( | ||
| db: D1Database, | ||
| reservation: NoteReservation, | ||
| now = new Date(), | ||
| ): Promise<void> { | ||
| await db | ||
| .prepare( | ||
| `UPDATE note_publications | ||
| SET status = 'unknown', updated_at = ? | ||
| WHERE attempt_id = ? AND status = 'pending'`, | ||
| ) | ||
| .bind(now.toISOString(), reservation.attemptId) | ||
| .run(); | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: markNoteUnknown does not verify that database update succeeded. If the WHERE clause matches no rows, the function silently succeeds without indication of failure. This creates inconsistent state where the application believes status was updated to 'unknown' but the database remains unchanged.
| export async function markNoteUnknown( | |
| db: D1Database, | |
| reservation: NoteReservation, | |
| now = new Date(), | |
| ): Promise<void> { | |
| await db | |
| .prepare( | |
| `UPDATE note_publications | |
| SET status = 'unknown', updated_at = ? | |
| WHERE attempt_id = ? AND status = 'pending'`, | |
| ) | |
| .bind(now.toISOString(), reservation.attemptId) | |
| .run(); | |
| } | |
| export async function markNoteUnknown( | |
| db: D1Database, | |
| reservation: NoteReservation, | |
| now = new Date(), | |
| ): Promise<void> { | |
| const result = await db | |
| .prepare( | |
| `UPDATE note_publications | |
| SET status = 'unknown', updated_at = ? | |
| WHERE attempt_id = ? AND status = 'pending'`, | |
| ) | |
| .bind(now.toISOString(), reservation.attemptId) | |
| .run(); | |
| if ((result.meta.changes ?? 0) === 0) { | |
| throw new Error(`Failed to mark note as unknown: attempt_id ${reservation.attemptId} not found or not in pending status`); | |
| } | |
| } |
| export async function markNotePublished( | ||
| db: D1Database, | ||
| reservation: NoteReservation, | ||
| noteId: string | null, | ||
| noteUrl: string | null, | ||
| now = new Date(), | ||
| ): Promise<void> { | ||
| const publishedAt = now.toISOString(); | ||
| await db | ||
| .prepare( | ||
| `UPDATE note_publications | ||
| SET status = 'published', note_id = ?, note_url = ?, published_at = ?, updated_at = ? | ||
| WHERE attempt_id = ?`, | ||
| ) | ||
| .bind(noteId, noteUrl, publishedAt, publishedAt, reservation.attemptId) | ||
| .run(); | ||
| } |
There was a problem hiding this comment.
🛑 Logic Error: markNotePublished and markNoteUnknown do not verify that database updates succeeded. If the WHERE clause matches no rows (e.g., attemptId doesn't exist or was already updated), the functions silently succeed without indication. This creates inconsistent state where the application believes the operation succeeded but the database was not modified.
| export async function markNotePublished( | |
| db: D1Database, | |
| reservation: NoteReservation, | |
| noteId: string | null, | |
| noteUrl: string | null, | |
| now = new Date(), | |
| ): Promise<void> { | |
| const publishedAt = now.toISOString(); | |
| await db | |
| .prepare( | |
| `UPDATE note_publications | |
| SET status = 'published', note_id = ?, note_url = ?, published_at = ?, updated_at = ? | |
| WHERE attempt_id = ?`, | |
| ) | |
| .bind(noteId, noteUrl, publishedAt, publishedAt, reservation.attemptId) | |
| .run(); | |
| } | |
| export async function markNotePublished( | |
| db: D1Database, | |
| reservation: NoteReservation, | |
| noteId: string | null, | |
| noteUrl: string | null, | |
| now = new Date(), | |
| ): Promise<void> { | |
| const publishedAt = now.toISOString(); | |
| const result = await db | |
| .prepare( | |
| `UPDATE note_publications | |
| SET status = 'published', note_id = ?, note_url = ?, published_at = ?, updated_at = ? | |
| WHERE attempt_id = ?`, | |
| ) | |
| .bind(noteId, noteUrl, publishedAt, publishedAt, reservation.attemptId) | |
| .run(); | |
| if ((result.meta.changes ?? 0) === 0) { | |
| throw new Error(`Failed to mark note as published: attempt_id ${reservation.attemptId} not found`); | |
| } | |
| } |
| CREATE INDEX IF NOT EXISTS idx_note_publications_attempted_at | ||
| ON note_publications(attempted_at DESC); |
There was a problem hiding this comment.
🛑 Performance Regression: Missing index on content_hash column causes full table scan on every duplicate check. The reserveNote function queries WHERE content_hash = ? without an index, resulting in O(n) lookup time instead of O(log n). This degrades performance proportionally with table size. Add index for the content_hash column.
| CREATE INDEX IF NOT EXISTS idx_note_publications_attempted_at | |
| ON note_publications(attempted_at DESC); | |
| CREATE INDEX IF NOT EXISTS idx_note_publications_content_hash | |
| ON note_publications(content_hash); | |
| CREATE INDEX IF NOT EXISTS idx_note_publications_attempted_at | |
| ON note_publications(attempted_at DESC); |
|
GPT Work定期実行の文体・禁止話題・重複回避・失敗時停止ルールを docs/gpt-work-note-schedule-prompt.md に追加しました（commit: f667cd2）。D1本番migrationとWorkerデプロイも完了し、公開済み直近3件を初期履歴として登録済みです。 |
|
訂正: GPT Work定期実行の文体・禁止話題・重複回避・失敗時停止ルールを |
|
OAuth接続の固定名CSRF Cookie衝突をc11cb5aで修正し、本番Worker 7259148c-3362-42a8-9ffa-f4312cfb044dへ反映しました。Node 5/5、Python 7/7、type-check、dry-runに成功。本番で並行2認証フローが両方GitHubへ302遷移することを確認しています。残りはユーザー承認後のlist_note_history読み取り確認です。 |
CSRF再発の診断・キャッシュ防止を本番反映
実ブラウザのCookie送信と接続成功は未確認。再発原因はまだ特定できておらず、根本解決とは扱わない。次回Connectで新しいエラーコードを確認し、Cookie未送信と値不一致を切り分ける。定期プロンプト更新と履歴MCP接続確認も未完了。 |
1 participant
Closes #10
変更概要
検証結果
リスク
Cookie接続診断（2026-09-16）
OAuth戻り先の遷移修正
診断ページは本番反映済み。ChromeでGET/POST両Cookieの到達を確認しました。一方、最初の認証POSTとGitHub callbackは302成功でも画面が戻らず、再送信時に消費済みCSRF Cookieが欠落しました。フォームのCSPに登録済みcallback originを含め、ChatGPTへ戻る遷移を許可します。URLのパス・クエリはCSPへ含めません。
本番接続復旧の確認
2026-09-16、Worker 6da579c2-8182-421b-b8cb-5accc0d7d0edへCSP修正を反映。ChromeでChatGPTへの戻り・接続完了・12個のActionsを確認しました。TC-007の証跡はa540e05。下書き・履歴の実取得と定期タスクの更新/再開は未実施です。