<!-- task=m-iop-agent-cli-runtime/20+13,17_project_log_journal plan=1 tag=REVIEW_API -->

# Harden CAS Journal Admission and Archive Reconciliation

## For the Implementing Agent

Filling every implementation-owned section in `CODE_REVIEW-*-G??.md` is mandatory. Implement the checklist, run every verification command, paste actual notes and stdout/stderr into the review file, keep both active files in place, and report ready for review; finalization is owned by the code-review skill. If blocked, record only the exact blocker, attempted commands/output, and resume condition in implementation-owned evidence fields. Do not ask the user, call user-input tools, create control-plane stop files, classify the next state, archive logs, or write `complete.log`.

## Background

The first review found that the nominal archive suite passes while four journal invariants remain broken: store-assigned sequence admission, collision-free journal identity, complete archive artifact validation, and cleanup after a concurrent append. These defects can reject valid callers, merge distinct project journals, or delete/duplicate terminal evidence during restart reconciliation.

## Archive Evidence Snapshot

- Prior task: `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal`
- Predicted archive evidence: `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/plan_local_G08_0.log` and `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/code_review_cloud_G08_0.log`
- Verdict: FAIL
- Findings: four Required findings covering pre-assignment sequence validation, incomplete manifest/JSONL/timeline validation, concurrent-suffix reconciliation, and ambiguous CAS journal keys/store identity drift; no Suggested or Nit findings
- Affected files: `apps/agent/internal/projectlog/store.go`, `apps/agent/internal/projectlog/store_test.go`
- Verification evidence: the original focused suite, package race suite, vet, formatting, and `git diff --check` passed; reviewer-only focused reproducers failed for unassigned sequence, missing/tampered committed JSONL, colliding identity tuples, and append-before-cleanup prefix retention
- Roadmap carryover: this remains an S12 `project-logs` journal subset and does not claim the Milestone Task on PASS; EventSink mapping, WORK_LOG projection, and the full dynamic frontier closure remain in later children

## Analysis

### Files Read

- `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/PLAN-local-G08.md`
- `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/CODE_REVIEW-cloud-G08.md`
- `apps/agent/internal/projectlog/store.go`
- `apps/agent/internal/projectlog/store_test.go`
- `apps/agent/internal/projectlog/record.go`
- `apps/agent/internal/projectlog/record_test.go`
- `packages/go/agentstate/store.go`
- `packages/go/agenttask/types.go`
- `agent-contract/inner/iop-agent-cli-runtime.md`
- `agent-roadmap/phase/automation-runtime-bridge/PHASE.md`
- `agent-roadmap/phase/automation-runtime-bridge/milestones/iop-agent-cli-runtime.md`
- `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`
- `agent-test/local/rules.md`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/13_agent_domain/complete.log`
- `agent-task/archive/2026/07/m-iop-agent-cli-runtime/17+13_project_log_records/complete.log`

### SDD Criteria

- SDD: `agent-roadmap/sdd/automation-runtime-bridge/iop-agent-cli-runtime/SDD.md`
- Status: approved; SDD lock released; no active `USER_REVIEW.md`
- Targeted scenario: S12, Milestone Task `project-logs`
- Acceptance: terminal closure is followed by exactly-once archive and cleanup while task loop/attempt/locator identity remains stable
- Evidence Map: S12 requires an archive reconciliation fixture and exactly-once archive evidence
- Plan effect: the checklist requires fail-closed artifact verification, collision-safe identity, concurrent append preservation, deterministic crash/restart regressions, and race execution. This child does not include `Roadmap Targets` because it cannot independently close the full S12 feature.

### Verification Context

- Supplied handoff: none
- Repository-native sources: `agent-test/local/rules.md`, the original plan commands, project-log tests, `agentstate` CAS implementation, the standalone runtime contract, and SDD S12
- Preflight: `/config/.local/bin/go`; `go version go1.26.2 linux/arm64`; `GOROOT=/config/opt/go`; current dirty shared checkout is intentional and contains unrelated sibling work
- Fresh baseline: `go test -count=1 ./apps/agent/internal/projectlog -run 'TestStore|TestArchive|TestReconcile'`, `go test -count=1 -race ./apps/agent/internal/projectlog ./packages/go/agentstate`, `go vet ./apps/agent/internal/projectlog`, formatting check, and `git diff --check` passed
- Constraints: package-level verification only; no external service, credential, remote runner, or cached test result is required
- Gap: existing tests do not exercise unassigned input sequence, tuple key collision/store identity drift, post-manifest artifact loss/tamper, missing/tampered timeline, or append between manifest commit and cleanup
- Confidence: high; each Required behavior has deterministic local filesystem/CAS reproduction and a direct success oracle

### Test Coverage Gaps

- Store-owned sequence admission: uncovered; current helpers pre-populate `Sequence`.
- Store/journal identity and tuple-key isolation: uncovered.
- Manifest-present JSONL and timeline loss/tamper: uncovered; the current conflict test changes only the manifest checksum.
- Concurrent append after manifest commit: uncovered; current crash tests have no competing CAS writer.

### Symbol References

None. No exported symbol is renamed or removed. `integrationRecordKey`, journal validation helpers, and reconciliation helpers are internal to `apps/agent/internal/projectlog`.

### Split Judgment

Keep one plan. Admission identity, artifact validation, and prefix cleanup are one CAS/archive transaction invariant; splitting them would allow an intermediate state that still prunes unverified or duplicated evidence. Runtime predecessors are satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/13_agent_domain/complete.log` and `agent-task/archive/2026/07/m-iop-agent-cli-runtime/17+13_project_log_records/complete.log`.

### Scope Rationale

Modify only `store.go`, `store_test.go`, and the active review evidence file. Do not change `ProjectLogRecord`/`ArchiveManifest` schema, `agentstate.Store`, EventSink mapping, WORK_LOG projection, CLI wiring, contracts, SDD, roadmap state, or the later full S12 scenario.

### Final Routing

- evaluation_mode: `isolated-reassessment`
- finalizer: `finalize-task-policy.sh pair`
- build closures: scope/context/verification/evidence/ownership/decision all closed; no capability gap
- build grade scores: scope `2`, state `2`, blast `2`, evidence `1`, verification `1` → `G08`
- build route: `local-fit`, lane `local`, filename `PLAN-local-G08.md`
- review closures: scope/context/verification/evidence/ownership/decision all closed; no capability gap
- review grade scores: scope `2`, state `2`, blast `2`, evidence `2`, verification `1` → `G09`
- review route: `official-review`, lane `cloud`, filename `CODE_REVIEW-cloud-G09.md`
- large_indivisible_context: `false`
- matched loop risks: `temporal_state`, `concurrent_consistency`, `boundary_contract` (count `3`)
- recovery signals: `review_rework_count=1`, `evidence_integrity_failure=false`
- capability-gap evidence: none

## Implementation Checklist

- [ ] Correct store-assigned sequence admission and enforce collision-safe store/journal identity with deterministic regressions.
- [ ] Validate or reconstruct the complete intent/manifest/JSONL/timeline set before cleanup and add corruption/loss regressions.
- [x] CAS-prune an exact verified archive prefix while preserving concurrent suffix records, with deterministic ordering and race coverage.
- [x] Run fresh focused, package, race, vet, formatting, and diff verification.
- [x] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Fix Append Admission and Journal Identity

#### Problem

`apps/agent/internal/projectlog/store.go:141` validates a caller-owned `Sequence` before the store assigns the authoritative value at line 164. `apps/agent/internal/projectlog/store.go:490` concatenates unrestricted project/workspace identities with `:`, while load/write paths validate neither the decoded journal nor record identity against the store, so distinct tuples can share and mutate one CAS record.

#### Solution

Require an unassigned input sequence, load and validate the journal identity/invariants, assign the candidate sequence, then run full persisted-record validation before CAS. Replace delimiter concatenation with a deterministic collision-safe framed hash and use one helper on append, replay, archive, and reconcile load paths.

Before (`store.go:141-166`, `store.go:490-492`):

```go
if err := record.Validate(); err != nil {
	return 0, err
}
// ...
seq := j.NextSequence
record.Sequence = seq
// ...
return "projectlog:" + string(projectID) + ":" + string(workspaceID)
```

After:

```go
if record.Sequence != 0 {
	return 0, fmt.Errorf("%w: append sequence must be unassigned", ErrInvalidIdentity)
}
// load and validate the exact store-owned journal
record.Sequence = j.NextSequence
if record.ProjectID != s.projectID || record.WorkspaceID != s.workspaceID {
	return 0, fmt.Errorf("%w: record identity disagrees with store", ErrInvalidIdentity)
}
if err := record.Validate(); err != nil {
	return 0, err
}
// derive "projectlog:"+sha256(length-framed project/workspace tuple)
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/projectlog/store.go` — add collision-safe keys and strict append/journal identity validation.
- [ ] `apps/agent/internal/projectlog/store_test.go` — add unassigned-sequence, preassigned-sequence rejection, mismatched identity, malformed journal, and colliding tuple regressions.

#### Test Strategy

Write `TestStoreAppendAssignsSequence`, `TestStoreAppendRejectsAssignedSequence`, and a table-driven `TestStoreRejectsJournalIdentityDrift`. Use one real `agentstate.Store` for the tuple-collision case and assert the second tuple cannot observe or mutate the first journal.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/projectlog -run 'TestStoreAppend|TestStoreRejectsJournalIdentity'
```

Expected: callers append with `Sequence == 0`, persisted records receive monotonic values, and every identity drift/collision case fails without mutation.

### [REVIEW_API-2] Verify the Complete Archive Artifact Set

#### Problem

`apps/agent/internal/projectlog/store.go:378-388` trusts a manifest when only its checksum string matches the intent and does not require/read the JSONL. Lines 417-420 accept a missing timeline. Missing or tampered committed evidence can therefore return success and reach journal pruning.

#### Solution

Validate intent schema, store identity, filename ordinal, range, count, checksum, timestamp, and terminal flag. Always read/decode the JSONL, recompute its checksum, validate every record, derive the expected manifest, call `ArchiveManifest.Validate`, and compare every manifest/intent field. Derive deterministic timeline bytes from the verified records; reconstruct a missing timeline atomically and reject non-matching existing content. Only return cleanup eligibility after the complete set is valid.

Before (`store.go:378-420`):

```go
if err == nil {
	var m ArchiveManifest
	// ...
	if m.Checksum != intent.Checksum {
		return ErrArchiveConflict
	}
}
// ...
if _, err := os.Stat(timelinePath); err != nil && !os.IsNotExist(err) {
	return err
}
```

After:

```go
intent, records, expectedManifest, expectedTimeline, err := s.verifyArchiveArtifacts(ordinal)
if err != nil {
	return err
}
// atomically materialize only missing deterministic artifacts;
// reject every conflicting existing artifact
return s.reconcilePrune(ctx, intent, records)
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/projectlog/store.go` — centralize strict intent/manifest/JSONL/timeline verification and deterministic missing-artifact reconstruction.
- [ ] `apps/agent/internal/projectlog/store_test.go` — cover missing/tampered JSONL, manifest field drift with unchanged checksum, malformed intent, and missing/tampered timeline.

#### Test Strategy

Add table-driven `TestReconcileRejectsCommittedArtifactDrift` and `TestReconcileReconstructsMissingArtifact`. Every conflict must preserve the journal; every reconstructable missing deterministic artifact must converge to the same bytes before cleanup.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/projectlog -run 'TestReconcile(Rejects|Reconstructs)|TestArchiveManifestChecksum'
```

Expected: loss is reconstructed only from verified authoritative records, conflicting content fails closed, and no failed case prunes terminal evidence.

### [REVIEW_API-3] Preserve Concurrent Suffix Records During Cleanup

#### Problem

`apps/agent/internal/projectlog/store.go:425-453` prunes only when the whole journal's first and last sequences equal the intent range. If another CAS writer appends after manifest commit, reconciliation returns success while retaining the already archived prefix; its terminal record then makes the combined journal unarchivable.

#### Solution

Pass the verified archived records into cleanup. Under bounded CAS retry, validate that the current journal starts with that exact sequence/content prefix, remove only the prefix, preserve any suffix, advance the archive ordinal monotonically, and keep `NextSequence` unchanged. Treat divergent overlap as `ErrArchiveConflict`; keep empty/already-pruned state idempotent.

Before (`store.go:447-453`):

```go
if j.Records[0].Sequence != intent.FirstSequence ||
	j.Records[len(j.Records)-1].Sequence != intent.LastSequence {
	return nil
}
j.Records = nil
j.ArchiveOrdinal = intent.ArchiveOrdinal
```

After:

```go
if !recordsMatchPrefix(j.Records, archivedRecords) {
	return ErrArchiveConflict
}
j.Records = append([]ProjectLogRecord(nil), j.Records[len(archivedRecords):]...)
j.ArchiveOrdinal = intent.ArchiveOrdinal
```

#### Modified Files and Checklist

- [x] `apps/agent/internal/projectlog/store.go` — CAS-prune verified prefixes and preserve concurrent suffix records.
- [x] `apps/agent/internal/projectlog/store_test.go` — inject an append at `archive_before_cleanup`, reconcile, and assert only the suffix remains with the next ordinal usable.

#### Test Strategy

Add `TestReconcilePreservesConcurrentAppend` and `TestReconcileRejectsDivergentPrefix`. Run both normally and in the package race suite.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/projectlog -run 'TestReconcile(PreservesConcurrentAppend|RejectsDivergentPrefix)'
```

Expected: verified prefix cleanup is exactly once, concurrent records remain once, and divergent retained state fails closed.

## Dependencies and Execution Order

- `13_agent_domain` is satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/13_agent_domain/complete.log`.
- `17+13_project_log_records` is satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/17+13_project_log_records/complete.log`.
- Implement REVIEW_API-1 before REVIEW_API-2/3 so every subsequent load uses strict journal identity; REVIEW_API-2 must establish verified records before REVIEW_API-3 prunes their prefix.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/projectlog/store.go` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |
| `apps/agent/internal/projectlog/store_test.go` | REVIEW_API-1, REVIEW_API-2, REVIEW_API-3 |
| `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/CODE_REVIEW-cloud-G09.md` | implementation evidence |

## Final Verification

```bash
gofmt -w apps/agent/internal/projectlog/store.go apps/agent/internal/projectlog/store_test.go
go test -count=1 ./apps/agent/internal/projectlog -run 'TestStoreAppend|TestStoreRejectsJournalIdentity|TestArchive|TestReconcile'
go test -count=1 ./apps/agent/internal/projectlog
go test -count=1 -race ./apps/agent/internal/projectlog ./packages/go/agentstate
go vet ./apps/agent/internal/projectlog
test -z "$(gofmt -d apps/agent/internal/projectlog/store.go apps/agent/internal/projectlog/store_test.go)"
git diff --check
```

Expected: every fresh focused/package/race test passes, vet and formatting are clean, artifact corruption never prunes evidence, and concurrent append reconciliation retains only the unarchived suffix. Cached Go test output is not acceptable.

After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.
