Add PR #675 GLM review, ignore harness scratch

[timujinne]

Summary

Housekeeping for two directories that were sitting uncommitted in the core checkout.

Changes

.gitignore

• Ignore /.pi-subagents/ — mission JSON and run transcripts written by the external
  subagent harness. Its reports are written outside the repository, so the directory is
  scratch. Root-anchored with a trailing slash, grouped with the other local-tooling
  entries (/.claude, /skills-lock.json, .mcp.json). Nothing from it is committed.

dev_docs/pull_requests/2026/

• Add the GLM reviewer pass on PR Guard the atomic custom_fields merge/delete against a NULL column #675 (COALESCE guard for the atomic custom_fields
  merge/delete) as 675-fix-custom-fields-atomic-merge/GLM_REVIEW.md. It was left
  untracked in a second, branch-name mismatched directory; it is now filed under the
  PR's existing directory, whose slug matches the head branch, per the
  one-directory-per-PR convention in CLAUDE.md.
• The document is complementary to the CLAUDE_REVIEW.md already committed for the same
  PR: it verifies the SQL semantics of both COALESCE fragments against a live Postgres,
  while the existing review covers a separate finding in Notifications.Prefs.write/2.

Test Plan

• No Elixir source in the diff — only .gitignore and one markdown document
• git check-ignore -v .pi-subagents/ resolves to the new rule; the path is absent
  from git ls-files and from history
• mix.exs@version and CHANGELOG.md untouched
• Full mix quality not run: the dialyzer PLT rebuild is OOM-killed under this
  container's 4 GiB cgroup limit. mix compile --warnings-as-errors and mix docs
  passed; credo ran. No check in the gate applies to these two files.

[timujinne]

Review — PR #698: ignore subagent harness scratch, file the #675 GLM review

Scope:.gitignore (+3 lines) and a new dev_docs/pull_requests/2026/675-fix-custom-fields-atomic-merge/GLM_REVIEW.md (+44 lines). The net diff against upstream/main is exactly these two files (git diff upstream/main...HEAD → 2 files, +47). The branch's merge commit (69af4596) is a pure upstream sync — it brings in commits already at base and contributes nothing to the net diff, so reviewing the two files is reviewing the whole PR.

Verdict: PASS. Clean hygiene PR, honest about its scope, follows the documented convention, and introduces no risk I can substantiate.

Ignore rule — /.pi-subagents/

Verified correct on every axis:

• Classification is right. The directory holds only external-harness scratch — missions/*.json (UUID-named mission specs) and artifacts/* ({hash}_{role}_0_{input,output,meta,transcript} files from scout/reviewer runs). The only references to pi-subagents anywhere in the repo are self-referential paths inside the directory itself; no .ex/.exs/.md/CI/config in the project reads or produces it. Nothing under it is tracked (git ls-files .pi-subagents/ → empty).
• Anchored and scoped correctly. Leading / pins it to the repo root; trailing / restricts it to a directory. git check-ignore confirms it matches .pi-subagents/, .pi-subagents/missions, .pi-subagents/missions/foo.json, and does not over-match pi-subagents, foo/.pi-subagents, or dev_docs/.pi-subagents (all report NOT ignored). No legitimate path is at risk.
• Placed sensibly. It sits next to /.claude — the closest analog (an agent/tooling scratch entry) — under its own comment, in the block of agent/config scratch entries. (That block groups /.fetch, /.claude, /.pi-subagents/, /skills-lock.json, .mcp.json without internal blank-line separation; the new entry is slightly more sectioned than its neighbors, purely cosmetic.)

A minor point worth noting only as a benefit: one mission JSON in the scratch dir contains a transient API authentication-error string. Ignoring the directory means such diagnostics never get committed — exactly the right outcome.

Review document — placement and legitimacy

• Genuinely about #675, not misfiled. The doc reviews the COALESCE(u.custom_fields, '{}'::jsonb) || type(^additions, :map) merge and the … - ?::text delete on custom_fields. Those fragments exist in current code at auth.ex:1826 and auth.ex:2230 (inside merge_user_custom_fields/3 at :1810 and delete_user_custom_field/3 at :2221). The review's cited line numbers (:1704, :2080) are from the 2026-07-31 reviewed revision and have drifted ~120 lines as later upstream merges shifted the file — normal for a point-in-time review artifact, and a reason a future reader shouldn't treat those numbers as authoritative against HEAD, but not a defect of this PR.
• Not a duplicate of CLAUDE_REVIEW.md. The two are independent reviews by different models with different findings: GLM is a focused spec-compliance PASS with one NITPICK (NULL→%{} semantics on delete); CLAUDE_REVIEW's substantive output is a follow-up BUG - MEDIUM on Notifications.Prefs.write/2 (which GLM never mentions). Co-locating them adds a second perspective, not repetition.
• Directory choice matches the documented convention. CLAUDE.md specifies dev_docs/pull_requests/{year}/{pr_number}-{slug}/{AGENT}_REVIEW.md — i.e. one directory per PR, with multiple {AGENT}_REVIEW.md files coexisting. Putting GLM_REVIEW.md beside CLAUDE_REVIEW.md in 675-fix-custom-fields-atomic-merge follows that exactly. The counter-examples cited (#668, #680) violate the convention: each splits its reviews across two slug directories (e.g. #668 has GLM_REVIEW.md in …-tls-options-and-queue-hook/ but CLAUDE_REVIEW.md in …-transport-settings-queue-hook/; #680 does the same). This PR is the correct layout; those are the drift the commit message describes and declines to repeat.
• No secrets.grep for token/secret/key/password patterns in the new doc returns nothing.

Project-rule compliance

• Commit message starts with Add ✓
• @version in mix.exs and CHANGELOG.md untouched (no code files in the diff at all) ✓
• No AI/Claude attribution in the commit. The doc's header ("Model: glm-5.2 via the z.ai endpoint, reviewer persona") documents which model produced the review — appropriate metadata for an {AGENT}_REVIEW.md, not the forbidden commit/PR attribution.

Verification runs

The diff contains no .ex/.exs/.heex file, so there is no code surface to validate; the green runs confirm nothing was incidentally broken:

• mix compile --warnings-as-errors → exit 0
• mix format --check-formatted → exit 0
• mix credo --strict → 0 issues, 10334 mods/funs across 725 files
• mix dialyzer → not completed (SIGKILL/137 during PLT update under the container's 4 GiB cgroup limit). Environment limit, not a code result; and with no Elixir file in the diff, there is no dialyzer-visible change to assess.

Substantiable risk

None. No action required before merge.

---

Update — posted after the review ran.mix dialyzer has since completed on this branch. It had been failing with SIGKILL under the 4 GiB container cap; re-run with the dev servers stopped and schedulers capped (ERL_FLAGS="+S 4:4"), it finished in 2m57s: done (passed successfully), exit 0.

Full gate on the PR head, all green: mix compile --warnings-as-errors · mix format --check-formatted · mix credo --strict (0 issues, 10334 mods/funs, 725 files) · mix dialyzer (0 warnings).