refactor(log-guard): isolate the canonical logs schema check (split S12 L1/3)

[lidge-jun]

Summary

• Pure move: the canonical Codex logs schema definition and its exact-match comparison (CURRENT_LOG_SCHEMA, the table/index DDL, SQL normalization helpers, hasCurrentLogsSchema/hasCurrentLogsTable — src/codex/log-guard/inspect.ts:18–67, 127–135, 246–320) move verbatim to src/codex/log-guard/inspect-schema.ts (137 lines; single bun:sqlite type import). inspect.ts keeps the file-identity memo cache, metrics and inspectCodexLogs (392 lines) and re-exports hasCurrentLogsSchema, so all 8 previously exported names stay importable; 5 importers unchanged.
• Why: 524-line file over the 400-line module limit; the schema check is a dependency-free leaf. Zero behavior change — the inspection cache stays single-owned in the residual.
• Plan and evidence: devlog/_plan/260905_now_split_train/370_codex_log_guard_inspect.md; rules 003_parent_decisions.md (PURE-MOVE-SIZE-01).

Stack (S12 log-guard; merge bottom-up):

#	PR	Branch	Base	Review focus
3	TBD	codex/split-codex-log-guard-maintenance	codex/split-codex-log-guard-inspect	maintenance
2	TBD	codex/split-codex-log-guard-protection	codex/split-codex-log-guard-inspect	protection
1	this PR	codex/split-codex-log-guard-inspect ← you are here	dev	schema leaf

Base: dev; layers 2 and 3 depend on this one. Review this PR's diff only (3 files, +150/−134; non-move diff: 1 leaf import, 2 facade wiring lines, 2 export modifiers, 11 test lines). Move-aware view: git diff --color-moved=dimmed-zebra dev...HEAD.

Verification

• bun run typecheck → exit 0
• Focused (codex-log-guard-inspect, -doctor, -protection, -maintenance) → 48 pass / 0 fail
• tests/lab/core-lab-boundary.test.ts → 17 pass / 0 fail
• Red-drive, then restored: bypassing the canonical-index comparison in the leaf fails requires every canonical Codex logs index (codex-log-guard-inspect.test.ts:314).
• bun run privacy:scan → passed; git diff --check dev...HEAD clean.
• New test: hasCurrentLogsSchema identity via facade vs leaf; leaf has no back-edge.
• Full suite on the remote CI host (lidge) at this exact SHA: recorded in the devlog doc.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (devlog unit records the layer; no user-facing change).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (read-only SQLite inspection; exact-schema gate moved byte-for-byte and driven red once).

Summary by CodeRabbit

• Refactor

  • Centralized log-schema validation to ensure log data is checked consistently against the expected table structure.
  • Improved reliability when validating log storage before write operations.

• Tests

  • Added coverage confirming shared schema validation remains consistent across inspection paths.
  • Added safeguards against circular module dependencies that could affect runtime behavior.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-05T03:27:43.140098Z	5c1a398	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 76c2d9c8-f9f5-4300-91bf-b969698956a2

📥 Commits

Reviewing files that changed from the base of the PR and between 593978d and 5c1a398.

📒 Files selected for processing (3)

• src/codex/log-guard/inspect-schema.ts
• src/codex/log-guard/inspect.ts
• tests/codex-integration/codex-log-guard-inspect.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

The PR extracts logs schema validation into a shared module. It pins columns, table SQL, and required indexes, then updates inspect.ts to re-export and use the shared predicates. An integration test verifies the export identity and prevents a dependency back-edge.

Changes

Logs schema inspection

Layer / File(s)	Summary
Pinned schema predicate src/codex/log-guard/inspect-schema.ts:1-137	Defines the expected logs columns, table SQL, and indexes. Normalizes SQLite metadata and validates column properties, table SQL, and required indexes through hasCurrentLogsSchema and hasCurrentLogsTable.
Inspection export and dependency validation src/codex/log-guard/inspect.ts:11-19, src/codex/log-guard/inspect.ts:79-82, tests/codex-integration/codex-log-guard-inspect.test.ts:496-506	Re-exports the shared schema predicate and imports its types and helper. The integration test verifies export identity and confirms that inspect-schema.ts does not import inspect.ts.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 5c1a3

This change centralizes logs-schema validation without changing the existing inspection export or validation behavior. No current merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: moving the canonical logs schema check into an isolated module while preserving the refactor context.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/split-codex-log-guard-inspect

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 58 / 80

이 PR은 베이스가 현재 dev입니다. S12 log-guard 스플릿의 맨 아래 잎(L1/3)입니다. 하는 일은 단순합니다. src/codex/log-guard/inspect.ts 안에 524줄로 붙어 있던 Codex logs 정규 스키마 정의와 완전 일치 검사(CURRENT_LOG_SCHEMA, 테이블/인덱스 DDL, SQL 정규화, hasCurrentLogsSchema/hasCurrentLogsTable)를 새 파일 src/codex/log-guard/inspect-schema.ts로 그대로 옮깁니다. inspect.ts는 파일 신원 메모 캐시, 메트릭, inspectCodexLogs만 남기고, hasCurrentLogsSchema를 리엑스포트해서 예전처럼 8개 public 이름이 그대로 보이게 합니다.

지금 dev HEAD(850afb2e9, package 2.43.0)에서도 inspect.ts가 아직 524줄입니다. 모듈 크기 한도(대략 400줄)를 이미 넘긴 상태라, 의존성 없는 스키마 잎을 먼저 빼는 순서는 스플릿 열차와 맞습니다. maintenance.ts와 protection.ts는 계속 ./inspect에서 hasCurrentLogsSchema를 가져오므로 import 경로를 바꿀 필요가 없습니다. 동작 변경이 없는 pure-move라면 Protect/Reclaim이 쓰는 “검사기와 쓰기 트랜잭션이 같은 스키마 판정을 본다”는 계약도 그대로 유지됩니다.

테스트는 facade와 leaf의 hasCurrentLogsSchema가 같은 함수인지, leaf가 ./inspect로 다시 끌어오지 않는지를 확인합니다. 회귀 범위를 좁히면서 순환 의존을 막는 합리적인 가드입니다. CI는 Linux 쪽 게이트·일부 샤드가 이미 통과했고 macOS/남은 샤드는 진행 중입니다.

라인 없음 - 런타임 동작 변경 의도가 없는 pure-move로 보이며, 공개 심볼 집합도 facade 리엑스포트로 유지됩니다.

src/codex/log-guard/inspect-schema.ts - hasCurrentLogsTable가 예전엔 inspect 내부 private였다가 leaf에서 export로 바뀝니다. 의도된 노출이면 괜찮지만, 외부에서 약한 경로(칼럼만 검사)로 쓰이지 않게 문서/사용처를 facade(hasCurrentLogsSchema)로 고정하는 편이 안전합니다.

src/codex/log-guard/inspect-schema.ts - CURRENT_LOG_INDEX_SQL 뒤에 빈 줄이 하나 더 있습니다. 동작과는 무관한 잡티입니다.

메인테이너의 판단이 필요한 지점

• S12 L2(protection)·L3(maintenance) 브랜치를 이 잎 위에만 쌓을지, 아니면 다른 split 열차와 병렬로 둘지
• leaf에서 hasCurrentLogsTable를 public으로 둘지, facade만 외부 API로 남길지

너의 추천
Linux CI가 모두 초록이면 이 잎(#3599)부터 합칩니다. 동작 변경이 없으니 리베이스 대신 fast-forward/스쿼시로 바로 올려도 됩니다. L2/L3는 이 SHA를 베이스로만 열면 됩니다.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Superseded by aggregate PR #3668, admin-merged into dev as 6585e6a after exact-head CI and tested-tree verification. This original PR was not individually merged; its rebased content and historical records were consolidated in #3668. The original branch is preserved. Further unimplemented debt layers remain deferred.

[lidge-jun]

Landed via #3668 at 6585e6a