# W3/T6 Repair Round 1 Re-Review

## Verdict

- **Base:** `ca85ab5`
- **Head:** `97b3b16`
- **Scope:** 4 files, +46/-23
- **Spec compliance:** PASS
- **Task quality:** Approved for the focused repair
- **Original W3-T6 Important finding:** **RESOLVED**
- **Blocking findings:** None

## Reviewed Inputs

- Prior failed report: `W3-T6.md`
- Repair brief: `.superpowers/sdd/task-briefs/T6-repair-1.md`
- Scoped diff: `ca85ab5..97b3b16`
- T6 brief, onboarding checklist spec, design, and prior wave reviews where relevant

## Original Finding Resolution

The original defect was that the active-item count was derived from a normal snapshot read after the transaction had already established its snapshot. Under MySQL-compatible `REPEATABLE READ`, two concurrent creates could both observe 49 active items and write the 51st row.

The repair addresses the actual transaction boundary:

1. `OnboardingChecklistService.createCompanyChecklistItem` now runs at `Isolation.READ_COMMITTED`.
2. It acquires a `PESSIMISTIC_WRITE` company lock before inspecting company items.
3. It reads company checklist rows through `findCompanyItemsForUpdate`, which is also `PESSIMISTIC_WRITE`.
4. Idempotency replay/conflict checks and the active-item count are performed from that locking read, not from an earlier ordinary snapshot.
5. The first writer locks the company row, inserts while holding the lock, and commits. The second writer waits; after acquiring the lock, its locking read observes the committed 50th item and returns `9703`.

The company-row lock plus locking item read are in the same transaction and use the same lock order as update/delete paths. This makes the 49-to-50 boundary deterministic for the supported write paths. The `PESSIMISTIC_WRITE` query includes inactive rows; active filtering remains in application logic, preserving the soft-delete accounting behavior.

## Regression Review

- Idempotent create replay still returns the existing row and performs no second insert.
- Different-body reuse of the same idempotency key still returns `9704`.
- Same idempotency key remains isolated by `company_id`.
- Soft-deleted items remain excluded from the active count.
- Existing rows with an idempotency key remain replayable after soft deletion.
- Company ownership and cross-company predicates are unchanged.
- No controller, DTO, error-code, migration, or user-visible API shape changed in this repair.
- Update/delete lock ordering is consistent with create: company row first, then company-scoped item access.

## Verification

### Required focused command

```powershell
cd D:\1\zhi\zhilai\mydreamjob-server
$env:JAVA_HOME='D:\java'; $env:Path='D:\java\bin;'+$env:Path
& 'D:\maven\apache-maven-3.9.7\bin\mvn.cmd' '-Dtest=OnboardingConcurrencyIntegrationTest,OnboardingCompanyChecklistServiceTest' test
```

Result:

```text
Tests run: 22, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```

### Stronger production-isolation reproduction

The boundary test was also run with `REPEATABLE_READ` forced at the Hikari/connection-default level, separately from the nested test property:

```powershell
& 'D:\maven\apache-maven-3.9.7\bin\mvn.cmd' `
  '-Dtest=OnboardingConcurrencyIntegrationTest$RepeatableReadLimitBoundary' `
  '-Dspring.datasource.hikari.transaction-isolation=TRANSACTION_REPEATABLE_READ' test
```

Result:

```text
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```

The SQL trace confirms both locking operations were issued:

```sql
select id from companies where id=? for update
select ... from onboarding_checklist_items
where scope_type='COMPANY' and scope_id=? and company_id=?
order by id for update
```

### Full onboarding regression

```powershell
& 'D:\maven\apache-maven-3.9.7\bin\mvn.cmd' '-Dtest=Onboarding*Test,CompanyKnowledge*Test' test
```

Result:

```text
Tests run: 85, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
```

`git diff --check ca85ab5..97b3b16` passed. No implementation files were modified during this re-review.

## Remaining Findings

### Minor

1. The earlier W3-T6 minor finding about a few broader concurrency tests over-constraining all callers to succeed under global `REPEATABLE_READ` remains outside this focused repair. The contract permits deterministic `9704`/`9705`/`9706` conflicts. The repaired 49→50 boundary itself now asserts exactly one success and one `9703`.
2. Real MySQL 8.4 execution was not available in this environment. H2 `MODE=MySQL` with forced `REPEATABLE_READ` passes, but the B3 integration gate should still confirm the same boundary on the real database.
3. The prior knowledge-service lock-timeout/deadlock mapping observation remains unchanged and is outside T6 repair round 1.

None of these block acceptance of the focused repair.

## Assessment

The original concurrency finding is resolved. The implementation now uses a locking/current read inside the create transaction and the strengthened repeatable-read boundary test passes. No idempotency, soft-delete, cross-company ownership, or API behavior regression was found in the reviewed scope.