Skip to content

feat(integration): add durable plugin delivery attempt admission - #252

Draft
seonghobae wants to merge 73 commits into
feat/plugin-vault-postgres-driver-v1from
feat/plugin-delivery-attempt-lifecycle-v1
Draft

seonghobae wants to merge 73 commits into
feat/plugin-vault-postgres-driver-v1from
feat/plugin-delivery-attempt-lifecycle-v1

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Current exact authority — 2026-09-19

Current exact head is e67fa3de97cde2331e2ea771b6150b84c0d986b1, open / Draft / mergeable, directly based on #245 workflow-free exact 39ebec99d907356a99a1a15528a3a1df713472ed.

The latest row-collection hardening was revalidated on exact parent e40fc9c60f122d7f3ed120075bfb07d273281711 by Verify Plugin Delivery Attempt Row Collection run 35280418160, which completed SUCCESS after exact checkout, Node 24 setup, frozen workspace install, focused formatting, hostile row-collection regression, hostile application-authority regression, and Integration typecheck. The ordinary descendant e67fa3de... deletes only that purpose verifier; production source and retained regressions are unchanged from the proven exact head.

The earlier verifier attempt 35029252125 on f254e592... failed at Set up Node 24 before any product regression ran, so it is infrastructure/harness evidence rather than a product RED. The subsequent exact GREEN above is the current bounded authority for this row-collection slice; it is not represented as protected-main merge or release authority.

Buyer gap

Adds an Integration-owned durable plugin-delivery-attempt admission boundary directly on the #245 PostgreSQL runtime lane. It deliberately does not depend on mutable #250/#251 HTTP transport and does not grant outbound networking. Durable rows contain only opaque authority/lifecycle fields; provider payload, secret material, origin URI and network authorization remain outside this bounded context.

Database and domain invariant

Migration 0006_plugin_delivery_attempt_record.sql creates service-owned plugin_delivery_attempt_record with UUIDv4 checks, pending admission status, attempt_count = 0, bounded max_attempts, scheduling/timestamp constraints and scoped indexes. Its BEFORE INSERT trigger joins the exact active delivery-origin grant and owning active installation under matching workspace/user authority and acquires FOR SHARE; missing or revoked authority fails closed with SQLSTATE 23514. No cross-service SQL or copied sibling persistence is introduced.

The repository uses INSERT ... ON CONFLICT DO NOTHING RETURNING; a zero-row idempotency conflict is followed by a fresh exact-scoped SELECT in a new PostgreSQL statement because Read Committed can suppress on a concurrent winner that is not visible to the same INSERT statement snapshot. Replay is accepted only when immutable admission scope matches.

Preserved RED / repair authority

Regression-only 788873a413caa24fd4597d57dc29b4276d3f9ad9 proves the application capability is absent before implementation. Regression d9bf3384fc59219b2c8ebed4aa6360bd97c60460 proves the concurrent idempotency-winner case requires a fresh statement snapshot. Hostile-envelope RED f47638672d005a5609764cec4a82ca7b3ef61c51 → repair 912958fa1ee4541f851f203a2c1d7f41134d7449 bounds Proxy/getter evidence before semantic validation.

Dependency-rejection RED proof head 987f0a8ca1ffb132146eee784bf3a3f281801f58, run 34202494161, job 101984290407, failed exactly the two raw dependency-error cases while repository and real-PostgreSQL migration acceptance stayed GREEN. Minimum repair c6330c06b6372b2bcdbebf08b16e51a4339d300a bounds only origins.getGrant() and store.createIfAbsent(). Exact run 34203636724, job 101987958062, completed GREEN on Ubuntu 24.04/PostgreSQL 16 across historical RED proofs, canonical formatting, diff hygiene, focused acceptance, Integration typecheck and the complete Integration suite.

Fresh CodeRabbit review then produced four valid repair lines rather than waivers: distinct requestedAt/updatedAt/nextAttemptAt persistence (8ef43e...5bfdee3...); destructive PostgreSQL target and explicit TLS policy (9f2d9b...aca4c3.../da94bb...); bracketed IPv6 loopback (87b247...f6ba35...); and table-level ownership/admission declaration (c77d0a...171f15...). Exact proof head 9e6e88e49e14d0b2d247e747df968e2494697666, run 34209314512, job 102006262067, completed GREEN across all four RED proofs, formatting/diff hygiene, focused delivery-attempt acceptance, Integration typecheck and the complete Integration suite. All four CodeRabbit threads are resolved. Workflow-retirement descendant cbb3caaddf3ad8d84ef27a6c7ba9aa624bed972a removed only that purpose verifier.

Hostile SQL durable-evidence repair

A later repository-boundary review found that PostgresPluginDeliveryAttemptStore read SQL result envelopes, durable row getters and stored timestamp conversion directly after treating them as untrusted persistence evidence. A throwing Proxy/accessor could therefore surface native dependency detail before the fixed PluginDeliveryAttemptPersistenceEvidenceError boundary.

Reality regression b390f37ed651ce2d9cb74d862fbae02641f3a55f carries hostile SQL-result and row getters with a test-only sensitive sentinel and requires both to collapse to the fixed credential-free persistence evidence error. Minimum causal repair e9a8b212b24b4914069a48a113d118ff4e4c0a56 adds one bounded evidence-read seam, snapshots SQL result/row evidence before semantic validation, and bounds stored Date.toISOString() conversion. It does not change SQL text, schema, admission identity, idempotency, retry budget or network authority.

Exact proof head ec2353a003c15c9f464b5d5d2454d3593427d768, run 34213508199, job 102019765395, completed GREEN on Ubuntu 24.04/PostgreSQL 16. The verifier first checked out b390f37... and reproduced one intended repository-test failure: the hostile getter's test sentinel escaped instead of the fixed evidence error. On the repaired exact head, canonical formatting and diff hygiene passed; the focused four-file delivery-attempt suite passed 24/24; Integration typecheck passed; and the complete Integration suite passed 403 tests with 3 environment-dependent skips across 65 files (63 passed, 2 skipped).

CHANGELOG descendant fe51a79c1e19cc7fb14e5296f04b3f91b41bf535 then completed its own exact verifier run 34213937828, job 102021136219, GREEN across the RED replay, canonical formatting/diff hygiene, focused acceptance, Integration typecheck and the complete Integration suite. The later row-collection/revoked-evidence line advanced through permanent Integration-owned source and regression commits without copying sibling source. Its current bounded proof is e40fc9c60f122d7f3ed120075bfb07d273281711 / run 35280418160 SUCCESS; workflow-retirement descendant e67fa3de97cde2331e2ea771b6150b84c0d986b1 removes only .github/workflows/verify-plugin-delivery-attempt-row-collection.yml.

Current parent adoption

#245 remains workflow-free exact 39ebec99d907356a99a1a15528a3a1df713472ed. This PR remains directly based on that exact #245 head and independent of mutable #250/#251 transport. Current e67fa3de... is an ordinary ahead-only descendant on the #252 lane; no parent source was copied, rewritten or force-pushed.

Deliberate boundary

This slice admits durable attempts only. It does not claim worker execution, claim/lease, retry/backoff transitions, append-only sanitized outcomes, dead-letter/pause/resume, per-attempt revocation fencing, operator status, restart recovery, DNS resolution, redirect/proxy handling, outbound HTTPS, or provider credential use. A later executor must consume immutable released canonical egress authority for connect-time SSRF controls. Durable origin/delivery identity is not network authorization.

Keep Draft. The exact hostile-evidence and row-collection repairs plus their purpose-verifier retirements are complete, but current repository/security evidence, independent approval and stacked prerequisites remain required before normal integration. No merge/release claim, self-approval, bypass, force-push, destructive rebase, stale evidence reuse or gate weakening is made.

Refs #130, #205, #235, #244, #245, #250, #251; ContextualWisdomLab/.github#712.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

플러그인 전달 시도 admission 기능을 추가했습니다. 애플리케이션은 활성 grant와 요청 데이터를 검증합니다. PostgreSQL 저장소는 멱등 삽입과 충돌 재조회를 수행합니다. 마이그레이션은 권한과 상태 제약조건을 적용합니다. CI는 focused 및 전체 테스트를 실행합니다.

Changes

플러그인 전달 시도 입장

Layer / File(s) Summary
전달 시도 권한 검증
apps/integration-service/src/plugin-delivery-attempt.ts, apps/integration-service/src/plugin-delivery-attempt.test.ts
입력, 설치 컨텍스트, 활성 grant와 durable 결과를 검증하는 PluginDeliveryAttemptApplication을 추가했습니다. 유효한 결과만 pending 레코드로 반환합니다. 예외와 민감한 데이터 노출은 fail-closed 방식으로 처리합니다.
전달 시도 데이터베이스 계약
apps/integration-service/migrations/0006_plugin_delivery_attempt_record.sql
전달 시도 테이블, UUID와 상태 제약조건, 활성 권한 trigger 및 pending 조회 인덱스를 추가했습니다.
멱등 PostgreSQL 저장소
apps/integration-service/src/plugin-delivery-attempt-repository.ts, apps/integration-service/src/plugin-delivery-attempt-repository.test.ts
매개변수화된 INSERT ... ON CONFLICT DO NOTHING을 추가했습니다. 충돌 결과가 비어 있으면 새 statement로 승자 레코드를 조회합니다. 입력 또는 저장 증거가 유효하지 않으면 전용 오류를 발생시킵니다.
통합 검증과 회귀 방지
apps/integration-service/src/plugin-delivery-attempt-migration.integration.test.ts, .github/workflows/verify-plugin-delivery-attempt-admission.yml
마이그레이션, 권한 취소, 제약조건 및 민감한 필드 부재를 검증합니다. 과거 RED 상태, focused 테스트, 타입 검사와 전체 Integration service 테스트를 CI에 추가했습니다.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant PluginDeliveryAttemptApplication
  participant PluginDeliveryAttemptOriginReader
  participant PostgresPluginDeliveryAttemptStore
  participant PostgreSQL
  Caller->>PluginDeliveryAttemptApplication: schedule(context, input)
  PluginDeliveryAttemptApplication->>PluginDeliveryAttemptOriginReader: getGrant(...)
  PluginDeliveryAttemptOriginReader-->>PluginDeliveryAttemptApplication: active grant
  PluginDeliveryAttemptApplication->>PostgresPluginDeliveryAttemptStore: createIfAbsent(candidate)
  PostgresPluginDeliveryAttemptStore->>PostgreSQL: parameterized insert
  PostgreSQL-->>PostgresPluginDeliveryAttemptStore: inserted or conflict
  PostgresPluginDeliveryAttemptStore-->>PluginDeliveryAttemptApplication: durable winner
  PluginDeliveryAttemptApplication-->>Caller: pending attempt or authority error
Loading

Merge Risk: 🟡 Moderate · up to 987f0

Dependency failures can escape the admission contract, a misconfigured test run can delete Integration data, and deferred attempts are persisted as immediately eligible. These issues should be fixed before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 Integration 서비스에 영속적인 plugin delivery attempt admission 경계를 추가하는 PR의 주요 변경 사항을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (2 skipped: 2 unsupported.)

  • 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 feat/plugin-delivery-attempt-lifecycle-v1

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

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

Copy link
Copy Markdown
Contributor Author

Fresh durable-evidence review found a second hostile SQL result-envelope gap after the earlier getter repair. oneOrUndefined() could still observe a mutable/revocable row collection inconsistently: repeated rows.length / rows[0] reads allowed a row to appear after zero-cardinality evidence or a revoked array proxy to leak a native error. Permanent regression: apps/integration-service/src/plugin-delivery-attempt-repository-hostile-collection.test.ts. Regression/verifier head f254e5922148d96d00123d1bc07540ec0882d28b (run 35029252125) is still queued. Minimum causal fix is current exact e5635161cc6e760599f44d97379112f5e0abe8fa: snapshot result fields once, validate array shape, snapshot cardinality once, early-return on zero, read row once. SQL/schema/admission/idempotency/network authority are unchanged. Exact-head verifier run 35029303274 is also still queued, so this is not claimed GREEN and the purpose verifier remains in place. Keep Draft; do not merge or restack descendants from this candidate until terminal exact-head proof.

seonghobae commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Current exact authority correction — 2026-09-16

Current #252 head is 2313688c8cb048763581467c35ce77adcb8e39d8, open/Draft/mergeable on #245 exact 39ebec99d907356a99a1a15528a3a1df713472ed. The PR body’s 9ed7847d... current-head sentence is historical.

Fresh source review first confirmed the durable-evidence repair at e5635161...: oneOrUndefined() snapshots the untrusted SQL result envelope, then snapshots row cardinality once before the zero/one-row branch and bounded index read. The same review found a separate owned-quality gap: internal production helpers in plugin-delivery-attempt.ts and plugin-delivery-attempt-repository.ts were not documented even though this lane requires complete production docstring coverage. Ordinary descendants 273ece88105cffc79320b1c987df1b28d73a83fe and 2313688c8cb048763581467c35ce77adcb8e39d8 add purpose-specific JSDoc to those authority/persistence helpers only. Runtime logic, SQL text, schema, admission identity, idempotency, retry budget, credential boundary and network authority are unchanged.

A fresh exact-head verifier exists for current 2313688c...: run 35038662980 (Verify Plugin Delivery Attempt Row Collection, run #4). It is currently queued with conclusion=null, so there is still no current-head GREEN to transfer. Keep the verifier, keep Draft, and do not merge or propagate this candidate into #253 yet.

Foundation order remains: exact #252 current-head terminal proof → purpose-verifier retirement → ordinary/non-force #253 parent adoption → descendant restack/revalidation. No force push, destructive rebase, source copy, or gate weakening.

Copy link
Copy Markdown
Contributor Author

Current exact authority is 72e34e2b5cce809ae69e4114aad2231e07fca6bf (Draft, base #245 exact 39ebec99d907356a99a1a15528a3a1df713472ed). Production/runtime source is unchanged from the row-collection repair plus JSDoc head 2313688c8cb048763581467c35ce77adcb8e39d8; this new commit changes only the temporary verifier harness.

Fresh Planning evidence exposed a shared harness defect in these bounded verifiers: actions/setup-node was configured with cache: pnpm before corepack enable, and the first runner to execute that shape failed in setup-node before any install/product test. #252 used the same ordering. Minimum repair removes only the pnpm cache request; Node 24 pinning, explicit corepack enable, frozen install, focused Prettier, hostile row-collection regression and Integration typecheck remain intact.

New exact-head verifier 35042839101 is queued/non-terminal. No prior GREEN is transferred to this head, the purpose verifier stays present, and child #253 must not restack onto this candidate until terminal exact-head proof followed by verifier retirement.

Copy link
Copy Markdown
Contributor Author

Current exact authority (2026-09-16): head 3a81bae892164cef0f5639cfcb82bbbc955bdfdd, direct base remains #245 exact 39ebec99d907356a99a1a15528a3a1df713472ed, Draft/mergeable.

Two related hostile-object gaps were found after the earlier row-cardinality snapshot repair. First, oneOrUndefined() and parseRow() still performed Array.isArray(...) on the SQL result envelope / durable row outside the bounded evidence seam, so a revoked Proxy could escape native TypeError instead of PluginDeliveryAttemptPersistenceEvidenceError. Regression 882a57ed18d120ce0f620ba31dd8a36c86a4b97e covers revoked result, row collection, and row evidence; minimum repository repair 7c6986d9c3a41f99d0567409bf2424db56bc700a bounds only those object-shape observations.

Second, the application boundary used the same unbounded Array.isArray object-shape check for trusted context, request input, and durable store evidence. Regression d3dcb6e9e43563f3a858737c4d649d462f34d457 proves revoked proxies must normalize to PluginDeliveryAttemptAuthorityError; repair e6e8d7fddb5b33b39efca60b82c7b552399dc450 introduces one bounded object classifier and leaves grant scope, persistence semantics, retry budget, credentials, and network authority unchanged.

The purpose verifier was extended at 3a81bae892164cef0f5639cfcb82bbbc955bdfdd to format/typecheck both repaired production boundaries and run both permanent hostile regressions. Exact-head run 35046845494 is currently queued / non-terminal. Do not reuse the superseded 72e34e2... run as current GREEN; keep Draft and do not propagate this parent into #253 until exact-head proof is terminal and the purpose verifier is retired.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current exact authority — 2026-09-17.

Predecessor exact 3a81bae892164cef0f5639cfcb82bbbc955bdfdd run 35046845494 / job 104638403226 reached GitHub-hosted Ubuntu 24.04, completed exact checkout, Node 24 setup and frozen workspace install, then failed only Check focused formatting. The hostile row-collection regression, hostile application-authority regression and Integration typecheck were all skipped. Treat this as a formatting harness RED, not delivery-attempt product RED.

Current ordinary descendant is exact 039e5f9f0e61cdfd99bb17d8f79740b5ec3cdb23. Its only new source is a contents: read diagnostic workflow that applies repository Prettier to the same four verifier-owned files in the ephemeral runner worktree and prints the resulting diff; it has no push credential, no production/source mutation and no gate relaxation. Normal verifier 35225432463 and formatter diagnostic 35225432604 are both current-head non-terminal evidence.

Next valid transition is: consume only the exact diagnostic patch → remove the purpose-complete diagnostic by ordinary descendant → reacquire the unchanged normal verifier on that exact workflow-free source before any GREEN, child restack, merge or release claim.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current exact authority — 2026-09-17.

The predecessor 35046845494 remains a formatting harness RED: exact checkout, Node and frozen install passed; focused Prettier failed before either hostile regression or Integration typecheck ran. A fresh cross-lane RCA on #258 also exposed the same verifier-selector defect present here: pnpm --filter @life-os/integration-service test -- <file> passes an unnecessary separator after a package script that already invokes Vitest, so it can enumerate the wider Integration suite rather than the named verifier file. This branch now passes each hostile file as the real positional argument without that extra separator.

Current exact 2a310040c2ee9ba11e12488c34c78c550476a8fe also hardens both normal and diagnostic checkout authority with ${{ github.sha }} and persist-credentials: false. The temporary formatter diagnostic is contents: read and now runs on every branch push so its diff belongs to the same exact head rather than an earlier diagnostic-only commit.

Current-head diagnostic 35225977578 and normal verifier 35225977580 are both queued/non-terminal. Valid next transition is exact diagnostic patch only → remove the purpose-complete diagnostic in an ordinary descendant → reacquire the unchanged normal verifier on that workflow-free exact. No GREEN, merge, or descendant restack before those exact results.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current exact authority update — 2026-09-18.

Hosted formatter diagnostic 35225977578 completed SUCCESS on predecessor 2a310040c2ee9ba11e12488c34c78c550476a8fe. Its canonical Prettier patch touched only plugin-delivery-attempt-repository.ts, plugin-delivery-attempt-repository-hostile-collection.test.ts, and plugin-delivery-attempt.test.ts; plugin-delivery-attempt.ts was already canonical. Normal verifier 35225977580 on that predecessor failed only at focused formatting, before both hostile regressions and Integration typecheck.

The diagnostic output has now been applied verbatim by ordinary descendants and the purpose-complete diagnostic workflow removed. Current head is 5f5c118a6c82ccb35e57db203f19a1bad1c23c94; production semantics, migration/SQL authority, admission scope, retry budget, and egress boundary are unchanged. Fresh normal verifier run 35253757603 is queued on this exact head. Do not transfer predecessor GREEN or call the hostile row/application authority accepted until that exact verifier reaches the focused regressions and typecheck. Keep Draft; no merge or descendant restack from this lane before exact terminal evidence.

Copy link
Copy Markdown
Contributor Author

Fresh hosted evidence changed this lane. Exact 5f5c118a6c82ccb35e57db203f19a1bad1c23c94 run 35253757603 reached the focused hostile row-collection regression after exact checkout, frozen install, and canonical formatting all passed. Three hostile cases passed; only rejects a revoked SQL result proxy... failed because the async test client attempted to fulfill a Promise with an already-revoked Proxy. Promise resolution itself probes the value's then property, so the revoked Proxy throws before PostgresPluginDeliveryAttemptStore.oneOrUndefined() can ever observe the alleged successful SQL result. Treating that raw Promise-resolution rejection as persistence evidence in production would incorrectly collapse genuine SQL/client operational failures into PluginDeliveryAttemptPersistenceEvidenceError.

The repair is therefore fixture-only, not a production catch-all. Ordinary descendants a2431d225803cb674fc4c94c7bff28aeefe4d630 -> e40fc9c60f122d7f3ed120075bfb07d273281711 replace that impossible fulfilled-revoked-result fixture with a hostile result Proxy that permits only the Promise protocol's then probe and throws when repository evidence (rows/rowCount) is actually read. The existing revoked row-array and revoked durable-row cases remain unchanged. No SQL/schema/admission/network/credential behavior changed. Current acceptance must come from exact-head hosted evidence on e40fc9c...; predecessor failure is not a product RED and predecessor GREENs do not transfer.

seonghobae added a commit that referenced this pull request Sep 18, 2026
Ordinary non-force adoption of workflow-free #252 exact e67fa3d into #253. Preserve #253 claim/lease delta and reacquire exact-head evidence.

Copy link
Copy Markdown
Contributor Author

2026-09-18 current exact override: e40fc9c60f122d7f3ed120075bfb07d273281711 completed Verify Plugin Delivery Attempt Row Collection run 35280418160 SUCCESS after frozen install and canonical formatting; hostile row-collection regression, hostile application-authority regression, and Integration typecheck all passed. The purpose verifier has now been retired by ordinary descendant e67fa3de97cde2331e2ea771b6150b84c0d986b1; compare from e40fc9c... to e67fa3d... removes only .github/workflows/verify-plugin-delivery-attempt-row-collection.yml. Production/test/migration semantics are unchanged by retirement. #252 remains Draft/unshipped; repository/security/independent-review gates remain separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant