diff --git a/AGENTS.md b/AGENTS.md index 4dc60a7d..9c8812d4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,11 @@ query := "SELECT * FROM bureaus WHERE id = $1" rows, err := db.QueryContext(ctx, query, id) ``` -文字列連結(`"... " + id`)は SQL インジェクション脆弱性のため禁止。 +文字列連結(`"... " + id`)は SQL インジェクション脆弱性のため禁止。golangci-lint の gosec は SeeFT の連結 SQL(`abstract.Crud` 経由や、`DB()` からのメソッドチェーン)を検出できない。lint が通っても連結が無い証明にはならないので、レビューで確かめる。 + +**INSERT した行の id をあとで使うときは、`RETURNING id` で受け取る** + +作成のあとに最新の行(`ORDER BY id DESC LIMIT 1` など)を読み直すと、同時に作られた別の行を掴む(#536)。id を使わない INSERT(`actionLogRepository.Create` など)は、これまでどおり実行するだけでよい。`user_repository.go` の `Create` などが今の書き方。 **エラーレスポンスは JSON 形式で返す** @@ -113,7 +117,7 @@ if (!mounted) return; setState(() => _data = data); ``` -dispose 後に `setState` を呼ぶと例外になるため必須。 +dispose 後に `setState` を呼ぶと例外になるため必須。失敗の分岐(`else`・`catch`)で `await` のあとに `setState` や `ScaffoldMessenger.of(context)` を呼ぶときも同じ。`flutter analyze` の `use_build_context_synchronously` が0件でも、失敗の分岐が抜けていることがある(PR #383)。 **ログは `logger`、`print` 禁止** @@ -192,7 +196,8 @@ try { ### Always Do - 新規 SQL はプレースホルダで書く - 設定値(API URL・シークレット)は環境変数 / `PropertiesService`(GASのみ)から取得 -- Flutter で非同期またぎ後の `setState()` 前に `mounted` チェック +- INSERT した行の id をあとで使うときは、`RETURNING id` で受け取る +- Flutter で非同期またぎ後の `setState()` 前に `mounted` チェック(失敗の分岐も) - GAS で `LockService` 取得後は `finally` で `releaseLock()` - 空リストは `[]Type{}` を返す - 機能の採否や設計・運用の方針を決めたら `docs/decisions/` に ADR を書く @@ -200,11 +205,12 @@ try { ### Ask First - 新規ライブラリの導入(特に Flutter の状態管理系) - 既存 entity の JSON キー命名変更(mobile / gas に影響) -- API レスポンス形式の変更 +- API レスポンス形式の変更(キーを変えるなら、クライアントは新旧どちらの形でも受けられるようにする。マージしても本番の反映までは古い API が動き続けるため。#479) - DB スキーマ変更(マイグレーション) ### Never Do - SQL を文字列連結で組み立てる +- 適用済みの migration のファイルを消す・書き換える(消すと、`schema_migrations` に記録された版のファイルが見つからず migrate が止まる。書き換えても、適用済みの DB には反映されない) - API URL やシークレットをハードコードする - `package:http` を `mobile/lib/utils/api.dart` 以外で import する - `print()` を新規コードで使う(`mobile/lib/`) @@ -219,3 +225,4 @@ try { - **JSON キー命名**: 古い entity は camelCase、新しい entity は snake_case。クライアント影響のため既存維持 - **エラーレスポンス**: 古い controller は `return err`、新しいものは map 形式 - **空リスト返却**: 一部 UseCase が `nil` を返す箇所あり(順次 `[]Type{}` へ) +- **作成後の最新行の読み直し**: place・task・review の UseCase が、作成後に `FindNewRecord` で最新の行を読み直している(新規コードは `RETURNING id`) diff --git a/docs/development/workflow.md b/docs/development/workflow.md index 82ace225..3e6e1ffd 100644 --- a/docs/development/workflow.md +++ b/docs/development/workflow.md @@ -51,6 +51,7 @@ SeeFT で、課題に気づいてから本番に反映するまでの仕事の - 名前は `種類/名前/issue番号/内容` です。種類は `feat`(機能)・`fix`(修正)・`docs`(文書)です。例:`feat/{名前}/123/show-break-card`。 - **main は使っていません。** 2025 年 8 月から更新されておらず、本番は develop を動かしています([deploy.md](../operations/deploy.md))。 - **PoC(作ってみないと分からないもの)は、試作用のブランチで自由に試します**。形が見えないうちに develop 向けのブランチで書くと、本番と同じ品質を早い段階から求められすぎて、試作が進まないためです。完成形が見えたら、develop から新しいブランチを切り、要るファイルだけを `git checkout <試作用のブランチ> -- <パス>` で持ち込んで PR にします。試行錯誤のコミットは develop の履歴に入れません。持ち込む時期の目安は、本番に入れると確信できたとき、または試作用のブランチが2か月を超えたときです。 +- **git の追跡から外したいファイルは、ほかの人も同じものを作るかで置き場所を決めます。** 誰が作業しても出る生成物(マニュアルの変換の出力など)は、理由のコメントを付けて `.gitignore` に書きます。自分だけの作業ファイルは、手元の `.git/info/exclude` に書きます。`.git/info/exclude` はリポジトリに入らないので、みんなが作る生成物をここに書くと、ほかの人の手元では追跡されていないファイルとして溜まり続けます(PR #491 で `.gitignore` に移しました)。 ## 4. コミット @@ -72,9 +73,15 @@ SeeFT で、課題に気づいてから本番に反映するまでの仕事の - テンプレート(`.github/pull_request_template.md`)に沿って書きます。「テスト項目」には、自分で確かめたことと、確かめていないことを分けて書くと、レビューする人が見る場所を決めやすくなります。 - PR を出すと CI が走ります。何が走るかは [onboarding.md の11節](onboarding.md) を見てください。ADR の前提の点検(`docs-refcheck`)は、すべての PR で走ります。 +- **新しい lint のルールを入れるときは、ルールを入れる変更と、既存の違反を直す変更を分けます。** 設定が妥当かの確認と、大量の修正の確認を1つの PR でやると、レビューが追いつかないためです。違反を直す変更はルールごとに issue に分けます。自動で直せるものと手で直すものも、危なさと要る知識が違うので混ぜません。 + - api(Go)は、CI の `go-lint` が PR で新しく増えた違反だけを見るので、設定だけの PR を先に出せます。 + - mobile は、CI の `flutter-lint` が既存の違反も含めて、info の指摘1件で落ちます。設定だけの PR は通らないので、先にルールごとの PR で違反を直し、最後にルールを有効にする PR を出します。 + - 45th で mobile に flutter_lints を入れたときは、まだ `flutter-lint` が無かったので、設定の PR(#280)を先に入れました。242 件の違反はルールごとの子 issue(#286 の下)で直し、違反がなくなってから CI を足しました(#386)。 ## 7. レビュー +人が見る前に、機械で拾えるものは機械で拾います。フォーマッタ → linter(CI の `go-lint`・`flutter-lint`)→ AI のレビュー(CodeRabbit。`AGENTS.md` の規約も読む)→ 人、の順に重ねています。45th は、書く人とレビューする人がほぼ同じ1人で、人のレビューだけに頼れなかったためです。フォーマッタは方針にはありますが、CI にはまだ入っていません。 + ### 人のレビュー develop には、**承認1件と、PR 上の会話がすべて解決していること**がマージの条件になっています。 @@ -90,9 +97,13 @@ develop には、**承認1件と、PR 上の会話がすべて解決している PR には AI のレビュー(CodeRabbit)が付きます。 - 指摘は参考です。スコープ外のものは、理由を書いて見送って構いません。 +- **指摘は「その PR が持ち込んだか」と「実害があるか」の2つで振り分けます。** CodeRabbit は PR で触った行を見るので、元からあった問題も、その PR が作ったように見えます。実害は、動かしたときの不具合・ビルドが壊れること・情報が漏れることのどれかです。 + - その PR が持ち込んだもの:その PR で直す + - 元からあって、実害があるもの:PR のスコープ外として、別の issue に切る + - 元からあって、見た目や書き方の揃え方だけのもの:スコープ外だと返信して、スレッドを閉じる - 無料枠のため、**レビューは1時間に1回まで**です。枠を超えると自動では走らず、PR の要約コメントに「Review limit reached」と出ます。枠が戻ってから、PR に `@coderabbitai review` とコメントすると頼めます。 - **指摘を直したあとの再レビューは、必要なときだけ頼みます**。直し方が提案どおりで、境目のケースをテストで確かめてあるなら、頼みません。1時間に1回の枠は、まだ誰も見ていない PR のために取っておきます。直し方が提案から大きく外れたとき、ほかの場所にも手を入れたとき、テストで確かめられていないときに頼みます。 -- CodeRabbit は、自分の指摘が直ったと判断すると、スレッドを自分で「Resolve」します。PR の要約コメントの「Merge Risk」は、最後にレビューできたコミットの時点の評価です(「up to `xxxxx`」の部分)。 +- CodeRabbit は、自分の指摘が直ったと判断すると、スレッドを自分で「Resolve」します。手で閉じる前に、もう閉じられていないかを見てください。CodeRabbit のスレッドに返信すると CodeRabbit が自動で返信してくるので、見送る理由の返信は1回にまとめます。PR の要約コメントの「Merge Risk」は、最後にレビューできたコミットの時点の評価です(「up to `xxxxx`」の部分)。 ## 8. マージ @@ -127,6 +138,7 @@ PR には AI のレビュー(CodeRabbit)が付きます。 - **割り振りは DM ではなく、チームのチャンネルに投稿します**。記録が残り、同じ種類のタスクを持つほかのメンバーも、その説明を参考にできるためです。相手へのメンションと issue のリンクに、お手本や設計のリンクを添えます。最後に「質問があれば対面か通話の時間を取ります。なければ次の MT か作業会で」と書き添えます。 - 対面や通話は、相手が望んだときに取ります。先に日程を押さえると、相手の負担だけが増えます。 +- **同じ種類のタスクを何人かに振るときは、1つずつ issue に分け、先にお手本の PR と設計の文書を用意します。** 各自が同じ形で書けるので、レビューも揃えやすくなります。45th のテストでは、親の issue(#404)の下に1関数ずつ子の issue を切り、お手本の PR(#419)と [設計の文書](test-design/phase1-pure-functions.md) を先に出してから割り振りました。 ### 作業を並べる