feat(identity): preserve authentication age across session rotation - #134
Merged
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough세션 인증 시각을 데이터베이스와 세션 흐름에 연결했습니다. Buyer-gap 레지스트리와 GitHub 증거 감사 기능을 추가했습니다. OpenCode 모델 검증, Compose 런타임 검증, CI 계약을 강화했습니다. Changes세션 인증 연령
Buyer-gap 상용 준비성 감사
OpenCode 및 Compose 검증
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequestCI
participant OpenCodeWorkflow
participant ModelCatalog
participant Compose
participant CommercialReadiness
PullRequestCI->>Compose: PostgreSQL 및 NATS 기동과 상태 확인
OpenCodeWorkflow->>ModelCatalog: 오프라인 NVIDIA 모델 검증
OpenCodeWorkflow->>Compose: Compose 설정 검증
OpenCodeWorkflow->>PullRequestCI: 검증 영수증 생성
CommercialReadiness->>CommercialReadiness: Buyer-gap 감사 실행
CommercialReadiness-->>PullRequestCI: JSON 및 Markdown 보고서 기록
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Author
|
@opencode-agent Implement PR #134 directly on exact head Implement the narrowest identity-owned GREEN path without reviving the superseded parallel
Run the new RED test first and record that it fails for the expected missing field, then implement and run identity unit tests, PostgreSQL integration with all ordered migrations, typecheck/build/root format, and any exact relevant coverage/docstring gates. If the PR head is no longer exactly the expected head before writing, stop instead of racing another writer. Commit the bounded implementation to this branch; keep the PR Draft until exact-head CI/security/review evidence is green. |
seonghobae
commented
Aug 9, 2026
seonghobae
left a comment
seonghobae
left a comment
Contributor
Author
There was a problem hiding this comment.
One migration/security condition should be part of the GREEN contract before this Draft becomes Ready: do not backfill authenticated_at from the current session row's created_at unless provenance proves that row was created by authentication rather than rotation. On protected main, rotation issues a new session with a new createdAt, so authenticated_at = created_at can make an already-old login appear freshly authenticated after migration. Prefer fail-closed unknown/null provenance for legacy rows, or reconstruct the root authentication instant from the retained rotation chain and prove it with a PostgreSQL migration regression. Add a realistic pre-migration rotated-session fixture and assert the migrated session can never gain a more recent authentication instant than its actual chain root. This matters because #55 will later use this field for high-impact export/deletion recent-auth decisions.
#131) * test(readiness): define canonical buyer-gap registry contract * feat(readiness): evaluate canonical buyer-gap evidence * feat(readiness): register canonical buyer-visible gaps * feat(readiness): attach canonical buyer-gap evidence * feat(readiness): add live buyer-gap audit entrypoint * fix(readiness): render capability and buyer gaps separately * build(readiness): verify buyer-gap audit modules * ci(readiness): reconcile canonical buyer-gap state * test(readiness): cover bounded buyer-gap collection * test(readiness): verify separated buyer-gap reporting * test(readiness): verify buyer-gap CLI boundary * test(readiness): harden buyer-gap snapshot validation * test(readiness): preserve capability maturity under buyer-gap evidence * fix(readiness): bind audit evidence to exact PR head * test(readiness): require exact-head PR audit checkout * style(readiness): format buyer-gap renderer * test(readiness): assert canonical buyer-gap exhaustion failures * test(readiness): reject coerced buyer-gap timestamps * docs(readiness): explain capability and buyer-gap report contract * test(readiness): reject non-string buyer-gap timestamps * fix(readiness): require string buyer-gap timestamps
* fix(agent): verify model and compose runtime * fix(ci): use supported Compose exec TTY flag * fix(agent): allow bounded OpenCode catalog probe runtime * test(agent): avoid secret-shaped catalog fixtures * test(agent): generate an ephemeral catalog credential * test(agent): derive non-secret catalog probe value * test(agent): exercise operational OpenCode config path
seonghobae
marked this pull request as ready for review
August 9, 2026 15:06
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/identity-service/src/auth-security.ts (1)
208-224: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win세션 계약에 설명 docstring을 추가하십시오.
SessionRecord와ActiveSession은 production declaration이지만 설명 docstring이 없습니다.authenticatedAt이 최초 인증 시각이며 회전 후에도 유지되는 UTC 값임을 계약에 기록하십시오.As per coding guidelines, “Production declarations must include explanatory docstrings sufficient for a new contributor to understand the contract without reconstructing the implementation.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/identity-service/src/auth-security.ts` around lines 208 - 224, Update the SessionRecord and ActiveSession interface declarations with explanatory docstrings documenting that authenticatedAt is the UTC timestamp of the initial authentication and remains unchanged after session rotation. Keep the contract clear for new contributors and preserve the existing field definitions.Source: Coding guidelines
🧹 Nitpick comments (11)
packages/commercial-readiness/src/buyer-gap-audit.test.mjs (2)
8-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value픽스처를
validateCapabilityManifest로 통과시키십시오.
manifest상수는 원시 객체이며schema키가 없습니다.evaluateCapabilities는manifest.capabilities만 읽으므로 지금은 동작합니다. 그러나 이 픽스처는 실제 검증된 매니페스트 형태와 다릅니다.audit.test.mjs의manifest()헬퍼처럼 검증기를 통과시키면 픽스처가 실제 계약과 계속 일치합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gap-audit.test.mjs` around lines 8 - 31, Update the manifest fixture to pass through validateCapabilityManifest, following the pattern used by the manifest() helper in audit.test.mjs, so it includes the validated schema and matches the real manifest contract before evaluateCapabilities consumes it.
43-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win레거시 형태의 단언과
unknown/resolved사례를 추가하십시오.테스트 이름은 "byte-for-byte equivalent"를 주장하지만, 단언은
capabilities,gaps, 두 개의 요약 값만 비교합니다. 핵심 하위 호환 계약, 즉buyerGapEvidence가 없을 때 신규 필드가 추가되지 않는다는 점이 검증되지 않습니다.또한
resolved와unknown이 모두 빈 배열입니다. 따라서buyer_gap_resolved와unknown_buyer_gap_states의 비영(非零) 경로가 검증되지 않습니다.💚 제안 추가 단언
assert.deepEqual(enriched.capabilities, legacy.capabilities); assert.deepEqual(enriched.gaps, legacy.gaps); + assert.equal(Object.hasOwn(legacy, 'buyer_gaps'), false); + assert.equal(Object.hasOwn(legacy, 'buyer_gap_unknown'), false); + assert.equal(Object.hasOwn(legacy, 'buyer_gap_resolved'), false); + assert.equal(Object.hasOwn(legacy.summary, 'capability_evidence_gaps'), false); assert.equal( enriched.summary.weighted_maturity_percent, legacy.summary.weighted_maturity_percent, ); assert.equal(enriched.summary.unresolved_gaps, legacy.summary.unresolved_gaps); assert.equal(enriched.summary.capability_evidence_gaps, 0); assert.equal(enriched.summary.unresolved_buyer_gaps, 1); assert.equal(enriched.summary.unknown_buyer_gap_states, 0); + assert.deepEqual(enriched.buyer_gaps, [ + { + gap_id: 'today.multi-device-sync', + issue_number: 121, + capability_ids: ['planning.durable-data'], + state: 'open', + resolution: null, + }, + ]); }); + + it('reports non-zero unknown and resolved buyer-gap dimensions', async () => { + const rootDir = await mkdtemp(join(tmpdir(), 'life-os-buyer-gap-audit-')); + await writeFile(join(rootDir, 'evidence.txt'), 'durable', 'utf8'); + + const report = await evaluate(rootDir, { + unresolved: [], + resolved: [ + { + gap_id: 'data.portability-completion', + issue_number: 55, + capability_ids: ['planning.durable-data'], + state: 'closed', + resolution: 'completed', + }, + ], + unknown: [ + { + gap_id: 'plugins.runtime-delivery', + issue_number: 130, + capability_ids: ['planning.durable-data'], + state: 'unknown', + resolution: null, + }, + ], + }); + + assert.equal(report.summary.unresolved_buyer_gaps, 0); + assert.equal(report.summary.unknown_buyer_gap_states, 1); + assert.equal(report.buyer_gap_resolved.length, 1); + assert.equal(report.buyer_gap_unknown.length, 1); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gap-audit.test.mjs` around lines 43 - 72, Extend the test around evaluate to verify the legacy result shape when buyerGapEvidence is undefined: assert the complete returned structure is byte-for-byte equivalent or that no new buyer-gap fields are present, while retaining the existing maturity comparisons. Populate the configured evidence with representative resolved and unknown entries, then assert buyer_gap_resolved and unknown_buyer_gap_states report the expected nonzero counts alongside the existing unresolved count.packages/commercial-readiness/package.json (1)
7-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win파일 목록 중복을 줄이고,
build의 누락 항목을 확인하십시오.세 스크립트가 거의 같은 파일 목록을 반복합니다. 신규 파일 두 개를 추가하면서 중복이 커졌습니다.
node --check src/*.mjs같은 단일 글로브로 대체하면 파일 추가 시 세 곳을 수정할 필요가 없습니다.또한
build는schema.mjs,audit.mjs,pr-gate.mjs,render.mjs를 검사하지 않습니다. 이는lint및typecheck와 다릅니다. 의도한 차이인지 확인하십시오.♻️ 제안 변경
- "build": "node --check src/cli.mjs && node --check src/github-client.mjs && node --check src/buyer-gaps.mjs && node --check src/buyer-gap-cli.mjs", - "lint": "node --check src/schema.mjs && node --check src/audit.mjs && node --check src/pr-gate.mjs && node --check src/render.mjs && node --check src/github-client.mjs && node --check src/cli.mjs && node --check src/buyer-gaps.mjs && node --check src/buyer-gap-cli.mjs", + "build": "npm run lint", + "lint": "for file in src/*.mjs; do node --check \"$file\" || exit 1; done", "test": "node --test src/*.test.mjs", - "typecheck": "node --check src/schema.mjs && node --check src/audit.mjs && node --check src/pr-gate.mjs && node --check src/render.mjs && node --check src/github-client.mjs && node --check src/cli.mjs && node --check src/buyer-gaps.mjs && node --check src/buyer-gap-cli.mjs" + "typecheck": "npm run lint"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/package.json` around lines 7 - 10, package.json의 build, lint, typecheck 스크립트에서 반복된 파일 목록을 node --check src/*.mjs 단일 글로브로 통일하고, build에도 schema.mjs, audit.mjs, pr-gate.mjs, render.mjs 검사를 포함하십시오.packages/commercial-readiness/src/buyer-gap-cli.mjs (2)
86-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value토큰 검증을 파일 읽기보다 먼저 수행하십시오.
GITHUB_TOKEN이 없으면 이 함수는 네 개 JSON 파일을 읽고 검증한 뒤에 실패합니다. 필수 자격 증명이 없으면 즉시 실패하는 편이 진단에 유리합니다. 토큰 검사를 함수 시작으로 옮기십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gap-cli.mjs` around lines 86 - 99, Move the GITHUB_TOKEN validation in the CLI entry function ahead of the Promise.all JSON reads and subsequent manifest, registry, snapshot, and policy validation. Preserve the existing required-token condition and error behavior, but fail immediately before any file I/O.
76-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
rename실패 시 임시 파일을 제거하십시오.
writeFile이 성공하고rename이 실패하면.tmp파일이 출력 디렉터리에 남습니다. 워크플로가 재실행될 때마다 잔여 파일이 누적됩니다. 실패 경로에서 임시 파일을 삭제하십시오.♻️ 제안 수정
+import { lstat, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises';const temporary = `${target}.${randomUUID()}.tmp`; - await writeFile(temporary, content, { encoding: 'utf8', mode: 0o600 }); - await rename(temporary, target); + try { + await writeFile(temporary, content, { encoding: 'utf8', mode: 0o600 }); + await rename(temporary, target); + } catch (error) { + await rm(temporary, { force: true }); + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gap-cli.mjs` around lines 76 - 82, Update writeAtomic so a failed rename removes the temporary file created by writeFile. Wrap the rename operation in failure handling, clean up temporary on error, and preserve the original rename failure after cleanup.packages/commercial-readiness/src/buyer-gaps.mjs (1)
342-350: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value중복 조건을 제거하십시오.
issue.state === 'closed'비교는 항상 참입니다. 앞의 두 분기가unknown과open을 이미 처리합니다. 조건을 단순화하면 의도가 명확해집니다.♻️ 제안 변경
const resolution = resolutionFor(issue); - if (issue.state === 'closed' && resolution !== null) { + if (resolution !== null) { resolved.push(gapEvidence(gap, 'closed', resolution)); continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gaps.mjs` around lines 342 - 350, In the issue-state handling around resolutionFor, remove the redundant issue.state === 'closed' check and rely on the prior unknown/open branches to reach this path. Keep the resolution !== null guard and existing resolved evidence behavior unchanged.packages/commercial-readiness/src/buyer-gap-cli.test.mjs (1)
1-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win파일 경계의 실패 동작 테스트를 추가하십시오.
이 파일은
parseBuyerGapArguments만 검증합니다.readJson과writeAtomic의 실패 동작은 검증되지 않습니다. 두 함수는 보안 및 무결성 경계를 담당합니다. 다음 사례를 추가하십시오.
- 심볼릭 링크 입력 거부:
'Buyer gap audit input must be a regular file'.- 크기 제한 초과 입력 거부:
'Buyer gap audit input exceeded the size limit'.- 잘못된 JSON 거부:
'Buyer gap audit JSON was invalid'.GITHUB_TOKEN이 비어 있을 때runBuyerGapAudit실패:'GitHub token is required'.writeAtomic결과 파일의 모드가0o600인지 확인.파서 거부 사례도 두 가지가 빠져 있습니다. 값이
--로 시작하는 경우와 값 길이가 500자를 초과하는 경우입니다.As per coding guidelines: "Tests must prove realistic domain accuracy and failure behavior, not only mocked call counts."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/buyer-gap-cli.test.mjs` around lines 1 - 53, Extend buyer-gap CLI tests beyond parseBuyerGapArguments to cover readJson and writeAtomic security and integrity failures: reject symlink inputs, oversized inputs, and invalid JSON with the specified messages, verify runBuyerGapAudit rejects an empty GITHUB_TOKEN, and assert writeAtomic output files use mode 0o600. Also add parser rejection cases for option values beginning with -- and values exceeding 500 characters, preserving the existing invalid-command error behavior.Source: Coding guidelines
packages/commercial-readiness/src/audit.mjs (1)
234-237: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value신규 buyer-gap 필드 부재로 기존 소비자가 안전함을 문서에 명시하십시오.
renderCommercialReadinessIssue는summary.unresolved_buyer_gaps가 없을 때 buyer-gap evidence를 “not evaluated”로 처리하지만,schema자체는 legacy 형식과 buyer-gap evidence가 추가된 형태 모두life-os.commercial-readiness-report.v1입니다. 워크플로와 design 문서에서 저장된 보고서를 읽을 때summary의 buyer-gap 필드가 없는 legacy 형태도 허용한다는 점을 명시하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-readiness/src/audit.mjs` around lines 234 - 237, Update the workflow and design documentation for renderCommercialReadinessIssue to explicitly state that stored life-os.commercial-readiness-report.v1 reports remain compatible when summary.unresolved_buyer_gaps is absent. Document that this legacy form is treated as buyer-gap evidence “not evaluated,” while reports containing the new buyer-gap field continue to be supported.packages/commercial-development-agent/vitest.config.mjs (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value전역 타임아웃 대신 해당 테스트에만 타임아웃을 지정하십시오.
이 값은 패키지의 모든 테스트에 적용됩니다. 느린 테스트는 OpenCode 바이너리를 실행하는 하나뿐이며, 그 테스트는 이미
spawnSync에timeout: 30_000을 지정합니다. 전역 45초 설정은 나머지 결정적 테스트에서 발생하는 정지를 늦게 드러냅니다. Vitest는 세 번째 인자로 테스트별 타임아웃을 받습니다.♻️ 제안 수정
packages/commercial-development-agent/vitest.config.mjs:test: { - testTimeout: 45_000, coverage: {
packages/commercial-development-agent/src/workflow-contract.test.mjs:linuxX64Test( 'registers a NVIDIA model absent from the bundled OpenCode catalog without discovery', () => { // ... }, 45_000, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-development-agent/vitest.config.mjs` at line 5, Remove the global testTimeout setting from the Vitest configuration and apply the 45-second timeout only to the slow OpenCode binary test in workflow-contract.test.mjs. Pass 45_000 as the third argument to that test’s invocation, preserving the existing spawnSync timeout and all other tests’ default timeout..github/workflows/ci.yml (1)
50-51: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
validate를compose_runtime에 직렬 연결하면 모든 PR의 CI 시간이 늘어납니다.두 작업은 서로 산출물을 공유하지 않습니다.
needs: compose_runtime은 게이트 역할만 하며, 그 대가로validate의 lint/typecheck/test/build가 Compose 기동이 끝날 때까지 시작하지 못합니다. 두 작업을 병렬로 실행하고 각각을 필수 체크로 등록하면 동일한 게이트 효과를 유지하면서 대기 시간을 줄일 수 있습니다.계약 테스트가
needs: compose_runtime문자열을 단언하므로, 변경 시packages/commercial-development-agent/src/workflow-contract.test.mjs의 해당 단언도 함께 갱신하십시오.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 50 - 51, Remove the unnecessary needs: compose_runtime dependency from the validate job in the workflow so validation runs in parallel, while keeping both jobs as required checks. Update the corresponding assertion in workflow-contract.test.mjs that currently expects the needs dependency.packages/commercial-development-agent/src/workflow-contract.test.mjs (1)
367-372: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win지역 변수
compose가 모듈 수준compose를 가립니다. 이름을 바꾸십시오.모듈 수준의
compose(24-26행)는compose.yaml의 내용입니다. 이 지역 변수는 워크플로 단계 텍스트입니다. 같은 이름이 두 가지 의미를 가집니다. 바로 아래 테스트(403-412행)는 모듈 수준compose를 사용하므로 현재 동작은 정확합니다. 그러나 이후에 단언을 블록 사이로 옮기면 의미가 조용히 바뀝니다. 지역 변수를composeStep으로 바꾸십시오.♻️ 제안 수정
- const compose = step( + const composeStep = step( 'Validate Compose configuration through trusted boundary', ); - expect(compose).toContain('docker compose'); - expect(compose).toContain('--file "$MODEL_WORKSPACE/compose.yaml"'); - expect(compose).toContain('--project-directory "$MODEL_WORKSPACE"'); - expect(compose).toContain('config --quiet'); - expect(compose).not.toContain('sudo -u opencode_model'); - expect(compose).not.toContain('${{ secrets.'); + expect(composeStep).toContain('docker compose'); + expect(composeStep).toContain('--file "$MODEL_WORKSPACE/compose.yaml"'); + expect(composeStep).toContain('--project-directory "$MODEL_WORKSPACE"'); + expect(composeStep).toContain('config --quiet'); + expect(composeStep).not.toContain('sudo -u opencode_model'); + expect(composeStep).not.toContain('${{ secrets.');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/commercial-development-agent/src/workflow-contract.test.mjs` around lines 367 - 372, Rename the local variable returned by step() in the workflow contract test from compose to composeStep, and update its associated assertions. Leave the module-level compose fixture and the later assertions that reference it unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/identity-service/migrations/0004_session_authentication_age.sql`:
- Around line 31-34: Split the migration around the
sessions_authentication_not_after_creation constraint and authenticated_at
nullability: add the CHECK constraint as NOT VALID, validate it in a separate
migration step, then perform the final SET NOT NULL transition only after
validation succeeds. Preserve the existing constraint expression and
identity.sessions target while keeping the final locking operation as short as
possible.
In
`@apps/identity-service/src/session-authentication-migration.integration.test.ts`:
- Around line 46-148: Extend the “backfills every legacy rotated session...”
integration test coverage to create separate invalid chains where
rotated_from_id crosses users and crosses workspaces. Run the authentication
migration for each case and assert it fails, confirming authenticated_at
constraints are not bypassed and invalid lineage is rejected. Keep the existing
valid-chain assertions intact.
In `@packages/commercial-development-agent/src/workflow-contract.test.mjs`:
- Around line 216-222: Replace the inferred executable path construction in the
workflow contract test with a command or repository-defined path that resolves
the actually installed OpenCode executable, such as the project’s pnpm exec
mechanism. Remove the dependency on realpathSync(opencode-ai) and the hardcoded
opencode-linux-x64/bin/opencode path while preserving the test’s executable
invocation behavior.
In `@packages/commercial-readiness/src/buyer-gaps.mjs`:
- Around line 363-379: Update attachBuyerGapEvidence to validate that evidence
is a plain object and that evidence.unresolved, evidence.unknown, and
evidence.resolved are all arrays before accessing their length or mapping them;
throw the established clear validation error for invalid evidence instead of
allowing a TypeError.
In `@packages/commercial-readiness/src/render.mjs`:
- Around line 34-38: 함수 capabilityList, renderCanonicalBuyerGaps,
renderCapabilityEvidenceGaps, renderCommercialReadinessIssue에 기여자가 구현을 추적하지 않고
이해할 수 있는 JSDoc을 추가하십시오. 각 함수의 입력과 반환값, 새 buyer-gap 보고서 계약에서의 역할, canonical
buyer-gap과 capability evidence의 구분을 명시하고 기존 동작은 변경하지 마십시오.
---
Outside diff comments:
In `@apps/identity-service/src/auth-security.ts`:
- Around line 208-224: Update the SessionRecord and ActiveSession interface
declarations with explanatory docstrings documenting that authenticatedAt is the
UTC timestamp of the initial authentication and remains unchanged after session
rotation. Keep the contract clear for new contributors and preserve the existing
field definitions.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 50-51: Remove the unnecessary needs: compose_runtime dependency
from the validate job in the workflow so validation runs in parallel, while
keeping both jobs as required checks. Update the corresponding assertion in
workflow-contract.test.mjs that currently expects the needs dependency.
In `@packages/commercial-development-agent/src/workflow-contract.test.mjs`:
- Around line 367-372: Rename the local variable returned by step() in the
workflow contract test from compose to composeStep, and update its associated
assertions. Leave the module-level compose fixture and the later assertions that
reference it unchanged.
In `@packages/commercial-development-agent/vitest.config.mjs`:
- Line 5: Remove the global testTimeout setting from the Vitest configuration
and apply the 45-second timeout only to the slow OpenCode binary test in
workflow-contract.test.mjs. Pass 45_000 as the third argument to that test’s
invocation, preserving the existing spawnSync timeout and all other tests’
default timeout.
In `@packages/commercial-readiness/package.json`:
- Around line 7-10: package.json의 build, lint, typecheck 스크립트에서 반복된 파일 목록을 node
--check src/*.mjs 단일 글로브로 통일하고, build에도 schema.mjs, audit.mjs, pr-gate.mjs,
render.mjs 검사를 포함하십시오.
In `@packages/commercial-readiness/src/audit.mjs`:
- Around line 234-237: Update the workflow and design documentation for
renderCommercialReadinessIssue to explicitly state that stored
life-os.commercial-readiness-report.v1 reports remain compatible when
summary.unresolved_buyer_gaps is absent. Document that this legacy form is
treated as buyer-gap evidence “not evaluated,” while reports containing the new
buyer-gap field continue to be supported.
In `@packages/commercial-readiness/src/buyer-gap-audit.test.mjs`:
- Around line 8-31: Update the manifest fixture to pass through
validateCapabilityManifest, following the pattern used by the manifest() helper
in audit.test.mjs, so it includes the validated schema and matches the real
manifest contract before evaluateCapabilities consumes it.
- Around line 43-72: Extend the test around evaluate to verify the legacy result
shape when buyerGapEvidence is undefined: assert the complete returned structure
is byte-for-byte equivalent or that no new buyer-gap fields are present, while
retaining the existing maturity comparisons. Populate the configured evidence
with representative resolved and unknown entries, then assert buyer_gap_resolved
and unknown_buyer_gap_states report the expected nonzero counts alongside the
existing unresolved count.
In `@packages/commercial-readiness/src/buyer-gap-cli.mjs`:
- Around line 86-99: Move the GITHUB_TOKEN validation in the CLI entry function
ahead of the Promise.all JSON reads and subsequent manifest, registry, snapshot,
and policy validation. Preserve the existing required-token condition and error
behavior, but fail immediately before any file I/O.
- Around line 76-82: Update writeAtomic so a failed rename removes the temporary
file created by writeFile. Wrap the rename operation in failure handling, clean
up temporary on error, and preserve the original rename failure after cleanup.
In `@packages/commercial-readiness/src/buyer-gap-cli.test.mjs`:
- Around line 1-53: Extend buyer-gap CLI tests beyond parseBuyerGapArguments to
cover readJson and writeAtomic security and integrity failures: reject symlink
inputs, oversized inputs, and invalid JSON with the specified messages, verify
runBuyerGapAudit rejects an empty GITHUB_TOKEN, and assert writeAtomic output
files use mode 0o600. Also add parser rejection cases for option values
beginning with -- and values exceeding 500 characters, preserving the existing
invalid-command error behavior.
In `@packages/commercial-readiness/src/buyer-gaps.mjs`:
- Around line 342-350: In the issue-state handling around resolutionFor, remove
the redundant issue.state === 'closed' check and rely on the prior unknown/open
branches to reach this path. Keep the resolution !== null guard and existing
resolved evidence behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a0b9e973-4d08-4140-8a81-901765499a0b
📒 Files selected for processing (33)
.github/workflows/ci.yml.github/workflows/commercial-readiness.yml.github/workflows/opencode-commercial-development.ymlARCHITECTURE.mdCHANGELOG.mdapps/identity-service/migrations/0004_session_authentication_age.sqlapps/identity-service/src/auth-security.tsapps/identity-service/src/authentication-age.test.tsapps/identity-service/src/oauth-callback-application.tsapps/identity-service/src/oauth-http-boundary.tsapps/identity-service/src/postgres-security-repositories.integration.test.tsapps/identity-service/src/postgres-security-repositories.test.tsapps/identity-service/src/postgres-security-repositories.tsapps/identity-service/src/session-authentication-migration.integration.test.tscompose.yamldocs/operations/opencode-commercial-development-loop.mddocs/research/2026-08-07-opencode-commercial-development-loop-standards.mddocs/superpowers/plans/2026-08-07-opencode-commercial-development-loop.mddocs/superpowers/specs/2026-08-07-opencode-commercial-development-loop-design.mdpackages/commercial-development-agent/src/workflow-contract.test.mjspackages/commercial-development-agent/vitest.config.mjspackages/commercial-readiness/package.jsonpackages/commercial-readiness/src/audit.mjspackages/commercial-readiness/src/buyer-gap-audit.test.mjspackages/commercial-readiness/src/buyer-gap-cli.mjspackages/commercial-readiness/src/buyer-gap-cli.test.mjspackages/commercial-readiness/src/buyer-gap-report.test.mjspackages/commercial-readiness/src/buyer-gap-validation.test.mjspackages/commercial-readiness/src/buyer-gaps.mjspackages/commercial-readiness/src/buyer-gaps.test.mjspackages/commercial-readiness/src/exact-head-workflow.test.mjspackages/commercial-readiness/src/render.mjsproduct/buyer-gaps.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/identity-service/tests/session-authentication-migration.integration.test.ts`:
- Around line 8-9: Update TEMPORARY_DATABASE_NAME to include a unique
per-test-run value such as a UUID, ensuring parallel executions on the same
PostgreSQL cluster use isolated temporary databases while preserving the
existing database setup and cleanup flow.
In `@packages/commercial-readiness/src/buyer-gaps.mjs`:
- Around line 367-374: Extend the evidence validation around the unresolved,
resolved, and unknown arrays to validate every element against the existing
buyer-gap item schema, reusing validateBuyerGapSnapshot’s item validation or a
dedicated equivalent. Reject the entire function when any element has invalid
gap_id, issue_number, capability_ids, state, resolution, or unexpected fields,
and add a buyer-gap-validation.test.mjs failure case covering an invalid array
element.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ea9fdeb-1577-441c-a66b-3ce60b37cd03
📒 Files selected for processing (8)
apps/identity-service/migrations/0004_session_authentication_age.sqlapps/identity-service/migrations/0005_finalize_session_authentication_age.sqlapps/identity-service/tests/session-authentication-migration-contract.test.tsapps/identity-service/tests/session-authentication-migration.integration.test.tspackages/commercial-development-agent/src/workflow-contract.test.mjspackages/commercial-readiness/src/buyer-gap-validation.test.mjspackages/commercial-readiness/src/buyer-gaps.mjspackages/commercial-readiness/src/render.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/identity-service/migrations/0004_session_authentication_age.sql
- packages/commercial-readiness/src/buyer-gap-validation.test.mjs
- packages/commercial-readiness/src/render.mjs
Contributor
Author
|
@opencode-agent Fix only the current causal GREEN gap on exact contributor head Fresh CI proved the new regression RED in
Run the focused commercial-readiness test first and prove GREEN, then the package/full relevant validation. Do not modify identity-session feature files, workflow authority, merge/release policy, secrets, or documentation in this task. |
Contributor
Author
|
@coderabbitai review |
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/identity-service/tests/session-authentication-migration.integration.test.ts`:
- Around line 80-89: 정리 로직에서 migrationPool.end()와 데이터베이스 삭제를 각각 중첩된 finally 블록으로
감싸, 앞선 단계가 실패해도 lockHeld인 경우 pg_advisory_unlock이 항상 실행되고 마지막으로 adminPool.end()도
호출되도록 수정하십시오.
In `@packages/commercial-readiness/src/buyer-gaps.mjs`:
- Around line 362-364: Document the contracts of failBuyerGapEvidence,
normalizeAttachedGapEvidence, and normalizeBuyerGapEvidence with JSDoc. Describe
each function’s inputs, normalization result, and failure conditions, including
the error behavior of failBuyerGapEvidence, so a new contributor can understand
the API without inspecting the implementation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 39a4ab9d-960b-43e5-857c-278dcdf043eb
📒 Files selected for processing (4)
apps/identity-service/tests/session-authentication-migration.integration.test.tspackages/commercial-development-agent/src/workflow-contract.test.mjspackages/commercial-readiness/src/buyer-gap-validation.test.mjspackages/commercial-readiness/src/buyer-gaps.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/commercial-readiness/src/buyer-gap-validation.test.mjs
- packages/commercial-development-agent/src/workflow-contract.test.mjs
This was referenced Aug 9, 2026
seonghobae
added a commit
that referenced
this pull request
Aug 9, 2026
* test(planning): define durable Today synchronization contract * feat(planning): implement durable Today domain contract * test(planning): define atomic Today persistence contract * feat(planning): persist Today with atomic optimistic writes * feat(planning): add durable Today persistence schema * feat(planning): compose durable Today runtime * test(planning): define Today HTTP precondition boundary * feat(planning): enforce authenticated Today HTTP preconditions * feat(planning): expose authenticated Today sync API * fix(planning): avoid duplicate migration ordinal * fix(planning): order durable Today migration after repository constraints * test(planning): verify durable Today PostgreSQL behavior * test(web): define authenticated Today sync BFF contract * feat(web): add authenticated durable Today BFF * feat(web): expose same-origin Today sync route * test(web): include durable Today sync in quality gates * test(web): define explicit local-to-workspace Today migration * feat(web): implement explicit local-to-workspace Today migration * test(web): verify workspace Today migration path * feat(web): label local and durable Today states * feat(web): localize durable Today sync states * feat(web): add explicit durable Today controls * feat(web): connect explicit workspace Today controls * test(web): verify explicit durable Today migration * test(ci): require browser journey verification * ci(web): execute Playwright buyer journeys * test(web): verify stale Today reconciliation * docs(research): ground durable Today synchronization * docs(planning): add durable Today operations runbook * docs(planning): design durable Today workspace sync * docs(planning): plan durable Today workspace sync * test(web): preserve local Today across retry * test(web): scope accessibility live-region assertions * test(web): bind durable search acceptance to semantic controls * test(web): follow the current Today capture label * style(planning): restore canonical provider formatting * style(planning): format concurrent Today assertion * fix(web): remove unreachable Today save disabled check * fix(web): narrow optional Today request body * test(web): make Today fetch fixtures exact-optional safe * test(web): preserve explicit optional fetch init in Today sync * style(web): format Today workspace synchronization * fix(planning): acquire Today advisory locks in order * test(planning): enforce deterministic Today lock order * test(planning): stress identical Today replay concurrency * style(web): format Today sync boundary * style(web): format Today workspace client * style(web): format Today sync tests * style(web): format Today workspace tests * test(planning): match SQL client result contract * test(planning): isolate lock-order integration fixture * test(planning): move lock-order fixture out of production source * test(planning): remove dynamic SQL from lock-order fixture * style(web): format Today workspace sync client * style(web): format Today sync boundary * style(web): format Today sync tests * fix(web): parse strong ETags with valid regex syntax * fix(web): use valid strong ETag parser in BFF * feat(identity): enforce recent-authentication policy for data rights (#136) Add a fail-closed recent-authentication policy that uses the preserved authentication provenance from #134, distinguishing authentication age from session rotation. Includes test-first boundary, stale/future/malformed provenance rejection, and exact-head CI/security validation. * test(planning): exercise Today concurrency independently * fix(planning): make Today date constraint DateStyle-independent * fix(planning): type Today SQL parameters explicitly * test(identity): bind data-rights ownership to recent authenticated session (#137) * test(identity): define recent authentication gate for data rights * feat(identity): enforce recent authentication policy * test(identity): define authenticated data-rights context boundary * feat(identity): bind data-rights export to recent authenticated session * fix(planning): serialize Today writes in explicit transactions * fix(planning): pin Today transactions to one PostgreSQL connection * test(planning): cover transactional Today persistence * test(planning): verify transaction lifecycle and cleanup * test(ci): bind Today concurrency to contributor head * test(planning): remove SQL-text lock-order surrogate * ci: capture exact Today prettier patch * test(web): expose in-flight Today save overwrite * fix(web): preserve edits during Today save * ci: apply bounded Today formatting * ci: expose read-only Today format patch * style(web): format Today sync client * style(web): format Today workspace sync * style(web): format Today sync tests * chore(ci): remove Today format diagnostic * test(planning): reject malformed Today lookup scope before SQL * ci: verify Today lookup validation red * ci: apply verified Today lookup validation * fix(ci): compare repair lease to contributor head * fix(ci): include staged self-removal in repair lease * test(planning): reject malformed Today repository lookups * test(planning): distinguish corrupted Today persistence * fix(planning): classify invalid Today persistence separately * fix(planning): validate Today lookup scope before SQL * test(planning): fail explicitly on leaked Today connections * chore(ci): remove superseded Today repair workflow * fix(web): lint complete source globs * test(planning): make Today concurrency cleanup deterministic * docs(today): classify standards publication status * docs(today): align validation and readiness plan * test(planning): define shared Today invariants contract * feat(planning): centralize Today validation invariants * refactor(planning): reuse shared Today invariants * refactor(planning): share Today invariants with persistence * refactor(planning): reuse Today invariants at HTTP boundary * test(planning): import Today persistence error from domain boundary * test(planning): use shared Today persistence error boundary * fix(web): preserve destructive-copy warning in Korean * test(ci): bind browser acceptance to its workflow job * test(web): close Today BFF authority branch gaps * ci(web): stage one-shot canonical formatter * style(web): apply canonical formatter output * feat(identity): persist data-rights request receipts (#138) * test(identity): define durable data-rights request ledger * feat(identity): persist data-rights request state * feat(identity): add data-rights request ledger schema * test(identity): preserve data-rights receipts through erasure * fix(identity): retain data-rights receipts after erasure * chore(identity): sequence data-rights request migration * chore(identity): remove duplicate migration sequence * docs(identity): record durable data-rights ledger boundary * test(identity): expose request-id collision as domain conflict * fix(identity): normalize request ledger conflicts * test(identity): harden data-rights ledger integration harness * test(identity): cover dual request conflict evidence * docs(identity): align data-rights ledger implementation status * docs(changelog): record durable data-rights ledger * fix(identity): resolve ledger migration path portably * test(identity): require immutable terminal receipt storage * test(identity): model pg timestamp rows as Date values * test(identity): satisfy pg parameter mutability contract * fix(identity): enforce immutable terminal data-rights receipts * test(identity): assert immutable receipt no-op at database boundary * test(identity): bind migration fixture lock to one PostgreSQL session * test(web): expose Today media-type and conflict coupling * test(web): execute Today review regressions * fix(web): preserve existing test dependencies * fix(web): normalize Today media types and conflict semantics --------- Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Buyer-visible outcome
Advance #55's authenticated/recent-auth data-rights boundary without resurrecting the superseded parallel
data-rights-service: preserve the actual authentication instant independently from ordinary session rotation so a future export/deletion HTTP boundary can enforce recent re-authentication honestly.Test-first evidence
The implementation contract requires:
authenticatedAtinstant;createdAtwhile preserving the originalauthenticatedAtrather than falsely treating rotation as re-authentication;Implemented on the current branch
SessionRecordandActiveSessioncarryauthenticatedAtseparately from session issuance time;authenticatedAtto the authentication instant;createdAt;authenticated_atexplicitly;0004_session_authentication_age.sqlreconstructs legacy authentication provenance through the retainedrotated_from_idchain, carries each root session's creation instant forward to its descendants, refuses cross-user/workspace lineage, and stages authentication constraints withNOT VALID;0005_finalize_session_authentication_age.sqlvalidates the staged constraints before the short finalSET NOT NULLtransition;tests/boundary use one fixed static disposable database name, apply the legacy schema, verify a three-session rotation chain, and require corrupt cross-user/workspace lineage to fail closed;expiresAt) rather than coupling callback tests to every internal session metadata field;pnpm --filter ... exec opencodepath rather than inferring a platform package directory;Security and migration boundary
created_at; descendants inherit the authenticated chain root and unresolved/corrupt lineage fails the migration rather than fabricating a recent authentication instant;Remaining before merge
Implemented on active PRuntil protected integration;Refs #55.
Summary by CodeRabbit
새로운 기능
보안 및 안정성 개선
문서