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

# Close Journal Validation and Pre-Prune Archive Verification

## 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 second review confirmed that the declared tests pass but found two remaining fail-closed gaps. Persisted record drift bypasses journal admission, and the normal archive path prunes without verifying the artifact set it just committed. These defects can replay foreign evidence or delete the only journal copy after a corrupted archive commit.

## Archive Evidence Snapshot

- Prior task: `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal`
- Predicted current-loop evidence: `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/plan_local_G08_1.log` and `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/code_review_cloud_G09_1.log`
- Verdict: FAIL
- Findings: two Required findings covering missing persisted-record/sequence validation and missing normal-path artifact verification before journal cleanup; no Suggested or Nit findings
- Affected files: `apps/agent/internal/projectlog/store.go`, `apps/agent/internal/projectlog/store_test.go`
- Verification evidence: the focused suite, package suite, race suite, vet, formatting, and `git diff --check` passed; one reviewer-only focused reproducer proved that a pre-cleanup timeline conflict still prunes the journal and that a foreign persisted record replays without error
- 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 complete dynamic frontier 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-G09.md`
- `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/plan_local_G08_0.log`
- `agent-task/m-iop-agent-cli-runtime/20+13,17_project_log_journal/code_review_cloud_G08_0.log`
- `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/agentstate/store_test.go`
- `agent-contract/index.md`
- `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-ops/rules/project/domain/agent/rules.md`
- `agent-ops/rules/project/domain/platform-common/rules.md`
- `agent-ops/rules/project/domain/testing/rules.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 SDD `USER_REVIEW.md`
- Targeted scenario: S12, Milestone Task `project-logs`
- Acceptance: stable task loop/attempt/locator identity and exactly-once archive/cleanup only after terminal closure
- Evidence Map: S12 requires an archive reconciliation fixture and exactly-once archive evidence
- Plan effect: the checklist requires shared validation on every journal load plus verification of the complete immutable artifact set before cleanup. PASS remains child-level evidence and does not claim `project-logs`.

### Verification Context

- Supplied handoff: none
- Repository-native sources: `agent-test/local/rules.md`, the active and archived loop evidence, project-log record/journal tests, `agentstate` CAS tests, the standalone runtime contract, and SDD S12
- Preflight: `/config/.local/bin/go` resolves to `/config/opt/go/bin/go`; `go version go1.26.2 linux/arm64`; `GOROOT=/config/opt/go`
- Current checkout: intentionally dirty shared checkout with unrelated sibling task artifacts; this child owns only `store.go`, `store_test.go`, and its active review evidence
- Fresh reviewer results: focused project-log tests, full package tests, the project-log/agentstate race suite, vet, formatting, and `git diff --check` passed
- Reviewer reproducer: `Archive` returned `nil` and pruned two records after its `archive_before_cleanup` hook replaced the timeline; `ReplayRecords` returned a persisted record whose `ProjectID` disagreed with the store
- External verification: not applicable; package-level deterministic filesystem/CAS tests require no service, credential, remote runner, port, or OS-specific device
- Gap: existing tests validate only journal header drift and reconcile-time artifact conflicts; they do not exercise record drift during replay/append or artifact conflict in the normal Archive cleanup path
- Confidence: high; both failures have deterministic local reproductions and direct no-mutation oracles

### Test Coverage Gaps

- Persisted journal validation: partial. Header identity drift is covered, but record identity, record validity, strict sequence order, and `NextSequence` drift are not.
- Pre-prune artifact verification: absent. Reconcile verifies artifacts, but normal `Archive` cleanup bypasses that verifier.
- Retry immutability: absent. A retry can overwrite an existing ordinal artifact set instead of verifying or rejecting it.
- Concurrent suffix preservation: covered by `TestReconcilePreservesConcurrentAppend` and remains in scope as a regression guard.

### Symbol References

None. No exported symbol is renamed or removed. `loadJournal`, `verifyArchiveArtifacts`, and `reconcilePrune` remain package-internal.

### Split Judgment

Keep one plan. Journal validation and archive cleanup are one fail-closed persistence transaction: cleanup is safe only when the loaded records and the materialized artifact set are both validated against the same store identity and sequence prefix. Predecessor `13_agent_domain` is satisfied by `agent-task/archive/2026/07/m-iop-agent-cli-runtime/13_agent_domain/complete.log`; predecessor `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`.

### Scope Rationale

Modify only `store.go`, `store_test.go`, and the active review evidence file. Do not change `ProjectLogRecord` or `ArchiveManifest` schema, `agentstate.Store`, EventSink mapping, WORK_LOG rendering, CLI wiring, contracts, SDD, roadmap state, or the complete 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 `2`, verification `1` -> `G09`
- build route: base `grade-boundary`, lane `cloud`, filename `PLAN-cloud-G09.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=2`, `evidence_integrity_failure=true`; recovery boundary matched without replacing the grade-boundary basis
- capability-gap evidence: none

## Implementation Checklist

- [ ] Validate every persisted journal record and sequence invariant through shared load/cleanup validation, with deterministic no-mutation regressions.
- [ ] Verify or reuse the complete immutable archive artifact set before cleanup and reject pre-cleanup or retry conflicts without pruning, with deterministic regressions.
- [ ] Run fresh focused, package, race, vet, formatting, and diff verification.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.

### [REVIEW_API-1] Validate Persisted Journal State on Every Path

#### Problem

`apps/agent/internal/projectlog/store.go:135-153` validates only the journal schema and header identity. `ReplayRecords` can return a persisted record whose project/workspace identity or validation state disagrees with the store, and `AppendRecord` can assign a duplicate or non-monotonic sequence when `NextSequence` disagrees with the retained records.

#### Solution

Add one store-owned journal validator and call it from `loadJournal`. Validate schema/header identity, positive non-overflowing `NextSequence`, every record with `ProjectLogRecord.Validate`, every record's project/workspace identity against the store, strict contiguous sequence order within the retained slice, and `NextSequence == last.Sequence + 1` for non-empty journals. Do not reject an early terminal record followed by a legitimate concurrent suffix. Make `reconcilePrune` load through the same validated helper instead of decoding a weaker journal copy.

Before (`store.go:143-153`):

```go
var j journal
if err := json.Unmarshal(payload, &j); err != nil {
	return journal{}, "", false, fmt.Errorf("projectlog: decode journal: %w", err)
}
if j.SchemaVersion != journalSchemaVersion {
	return journal{}, "", false, fmt.Errorf("projectlog: journal schema %d", j.SchemaVersion)
}
if j.ProjectID != s.projectID || j.WorkspaceID != s.workspaceID {
	return journal{}, "", false, fmt.Errorf("%w: journal identity disagrees with store", ErrInvalidIdentity)
}
return j, revision, true, nil
```

After:

```go
var j journal
if err := json.Unmarshal(payload, &j); err != nil {
	return journal{}, "", false, fmt.Errorf("projectlog: decode journal: %w", err)
}
if err := s.validateJournal(j); err != nil {
	return journal{}, "", false, err
}
return j, revision, true, nil
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/projectlog/store.go` — add shared journal validation and reuse it in load, replay, append, archive, and cleanup paths.
- [ ] `apps/agent/internal/projectlog/store_test.go` — add a table-driven persisted record/sequence drift matrix and assert rejection without mutation.

#### Test Strategy

Add `TestStoreRejectsPersistedJournalDrift` in `store_test.go`. Inject checksum-valid CAS payloads for foreign record project/workspace identity, invalid record schema, duplicate/gapped sequence, stale `NextSequence`, zero `NextSequence`, and overflow boundary; assert replay and append fail with the typed validation error and the CAS revision/payload remain unchanged.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/projectlog -run 'TestStore(Append|RejectsJournalIdentity|RejectsPersistedJournalDrift)'
```

Expected: store-assigned append remains monotonic, every malformed persisted journal fails before replay or mutation, and valid post-archive suffix journals still pass.

### [REVIEW_API-2] Verify Immutable Archive Artifacts Before Cleanup

#### Problem

`apps/agent/internal/projectlog/store.go:271-323` writes or overwrites intent, JSONL, timeline, and manifest files, then calls `reconcilePrune` directly. `verifyArchiveArtifacts` is reached only through `Reconcile` at lines 363-368, so corruption between materialization and cleanup or a conflicting retry can delete the retained journal without a valid complete archive.

#### Solution

Treat an existing ordinal artifact set as immutable retry state: verify/reconcile it instead of overwriting it. After fresh materialization and the `archive_before_cleanup` hook, call `verifyArchiveArtifacts`, then pass only the returned verified intent and records to `reconcilePrune`. Preserve the journal and return `ErrArchiveConflict` or `ErrArchiveIncomplete` for any mismatch. A valid retry must converge without changing committed artifact bytes, and concurrent suffix cleanup must continue to prune only the verified prefix.

Before (`store.go:318-323`):

```go
if err := s.failAt("archive_before_cleanup"); err != nil {
	return err
}

return s.reconcilePrune(ctx, intent, records)
```

After:

```go
if err := s.failAt("archive_before_cleanup"); err != nil {
	return err
}
verifiedIntent, verifiedRecords, err := s.verifyArchiveArtifacts(ordinal)
if err != nil {
	return err
}
return s.reconcilePrune(ctx, verifiedIntent, verifiedRecords)
```

#### Modified Files and Checklist

- [ ] `apps/agent/internal/projectlog/store.go` — verify fresh artifacts before cleanup and make retry reuse fail closed without overwriting a conflicting ordinal.
- [ ] `apps/agent/internal/projectlog/store_test.go` — add pre-cleanup corruption and valid/conflicting retry regressions with journal-retention and byte-identity assertions.

#### Test Strategy

Add table-driven `TestArchiveRejectsPreCleanupArtifactDrift` for intent, JSONL, manifest, and timeline mutation from the existing failure hook; each case must return a typed conflict/incomplete error and retain the exact journal. Add `TestArchiveRetryReusesCommittedArtifacts` with valid and conflicting subtests: a valid pending archive converges without changing artifact bytes, while a conflicting artifact is never overwritten and prevents cleanup. Keep `TestReconcilePreservesConcurrentAppend` as the suffix regression.

#### Verification

```bash
go test -count=1 ./apps/agent/internal/projectlog -run 'TestArchive(RejectsPreCleanupArtifactDrift|RetryReusesCommittedArtifacts)|TestReconcilePreservesConcurrentAppend'
```

Expected: cleanup occurs only after a complete verified set, valid retry bytes remain immutable, conflicts retain the journal, and concurrent suffix records remain exactly once.

## 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 so archive and cleanup share one validated journal load. Run both focused matrices before the final package/race suite.

## Modified Files Summary

| File | Item |
|------|------|
| `apps/agent/internal/projectlog/store.go` | REVIEW_API-1, REVIEW_API-2 |
| `apps/agent/internal/projectlog/store_test.go` | REVIEW_API-1, REVIEW_API-2 |
| `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|TestStoreRejectsPersistedJournalDrift|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, formatting, and diff checks are clean; malformed journals and artifact conflicts never replay or prune evidence; valid retry and concurrent suffix reconciliation remain exactly once. Cached Go test output is not acceptable.

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