docs(reports): archive v4.4.4 codex PASS review report (Issues #12, #8, #7)

Reviewed HEAD fbe2731 against base 8a09e02; 242 selected tests passed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
GodopuandClaude Fable 5.1 committed 2026-09-13 15:00:05 +09:00
1 parent fbe2731980
commit c714323dcf
1 file changed
+65
@@ -0,0 +1,65 @@
# Codex review — fix/issue-12-8-7
Job: `737157bc`. Reviewer: `planner-reviewer-codex-01`.
Base: `8a09e02`. Reviewed HEAD: `fbe27319802a8403ed21691ba59a2c3d6e5c5319`.
PASS. No blocking defect found in the requested branch changes. Reviewed the full branch diff and six commits through fbe2731; archived peer reports were background context, not substitutes for fresh verification.
## Issue #12 — broker secrets
`BrokerConfig.to_registry_block()` excludes the password. `_atomic_write_record()` removes the key before persistence, including caller-supplied broker blocks and subsequent writes to old records. Because the in-memory registration record is scrubbed before `init_job_log`, newly seeded metadata also receives the scrubbed block.
`broker_config_from_job()` overlays record routing/authentication metadata on environment configuration, retaining MQTT_PASSWORD when the record omits it. Publisher CLI and job subscriber call `ensure_env_loaded()` before preparing or connecting; reconcile's embedded subscriber also loads the environment before building its configuration. The service publishes a prepared event using the same configuration resolver and transports. As required by the existing D6 library contract, a direct embedding application's entry point must initialize the environment (or export the variables); DelegationService itself does not load .mam.env. This prerequisite should not be confused with automatic dotenv loading in every library call.
The migration holds the registry lock during selection and rewriting. Only a truthy stored password is selected. An independent synthetic temporary-registry check confirmed:
- Dry-run leaves all record bytes unchanged.
- Missing, null, and empty-string passwords remain byte-identical after actual migration.
- The valued-password record loses only that key; status, sequence, auth token and dispatch token remain unchanged.
The G6 exception for `_cmd_scrub_secrets` is narrowly justified: it changes stored credential data without a lifecycle transition. Retaining auth_token and dispatch_token supports the agent's per-job HMAC signing and attempt binding. This is removal of shared broker credentials from live job records and future writes, not a claim to erase historical transcripts or old audit copies. Legacy record passwords remain usable until scrubbed; environment reconstruction was independently checked with synthetic data.
## Issue #8 — update state
Successful updates keep the installer's new version and hash ledgers. Only old install_state keys absent from the fresh file are merged. Exact awk field comparison avoids regex-key interpretation, lines without `=` and empty keys are skipped, the fresh file's final newline is repaired, and an unterminated old final line is processed. An independent execution of the actual merge block in temporary directories verified these properties with a dotted key, a similar existing key, a malformed line and both missing-final-newline cases.
The failure restoration function is unchanged and still copies install_state, asset_hashes.txt and version.txt from backup. The passing Issue #8 test performs a real local install→stale-metadata→update cycle. Failure-path preservation was checked by code comparison; a separate injected-installer-failure test was not run.
## Issue #7 — workspace packing
Candidate workspace IDs are deduplicated in pane order after normalized CWD matching. Each candidate first offers an agent-less pane; otherwise the unchanged layout engine determines a split. Overflow continues to the next candidate. Selection sets existing_ws before allocating or reusing, and rename applies only to the chosen workspace. Single-candidate behavior and the existing agent-reuse path remain intact.
The shell loop splits machine workspace IDs, not CWDs or display labels. Under the existing herdr ID representation this is appropriate; arbitrary whitespace or glob characters in future ID formats would require a different iteration form. No new handling of labels as IDs was introduced.
The opt-in mock geometry represents one full pane, two full-height columns, a three-pane partial grid, and a full four-pane grid per sampled workspace. Those shapes match the unchanged layout engine's grouping and overflow rules. Passing tests cover sibling packing, all-full allocation, later idle-pane reuse, chosen-workspace rename and single-workspace equivalence, alongside the broader shim and layout tests.
## Heartbeat test and release
fbe2731 still invokes the real publisher and checks eventual registry completion and wrapper success. Launching publishes asynchronously supplies disk heartbeats at a timer-like cadence independently of broker ACK latency, consistent with local recording before network publication. It does not change production wrapper behavior, and separate no-signal and overall-deadline tests remain.
Nonblocking coverage limit: the test still uses broker timing and does not explicitly synchronize on subscriber idle exit. It demonstrates periodic-heartbeat behavior, not equivalence to every agent that serially waits for each publish. A deterministic subscriber-exit signal would strengthen proof that the disk-grace branch was exercised under every broker timing. The current fresh run passed.
Version constants, all eight skill frontmatters, current documentation and the VERSIONS matrix are synchronized at 4.4.4. The release commit is exactly `release(v4.4.4): bump framework and eight skills to 4.4.4 in lockstep`. The checked current documents retain 4.4.3 only as a historical changelog heading. The changelog's original new-test count predates the two additional packing tests; that is a documentation bookkeeping nit, not a runtime failure.
## Fresh validation
**242 passed in 213.89 seconds** at the reviewed HEAD:
```text
tests/test_issue7_workspace_packing.py
tests/test_issue8_update_state.py
tests/test_issue12_registry_secrets.py
tests/test_issue5_phase1_delegation.py
tests/test_issue5_phase2a_loop.py
tests/test_herdr_shim_contract.py
tests/test_layout.py
tests/test_deploy_layout.py
tests/test_issue14_delivery_ack.py
tests/test_version_consistency.py
```
Full-branch `git diff --check`, individual `bash -n` checks for lib.sh and update.sh, and `verify_framework_version()` using the shell constant all passed. The independent scrub, configuration and merge checks above also passed. No full repository suite was rerun.
Review only: no source changes, commits, pushes, merges, production registry migration, or real-conversation purge. Synthetic migration identifiers were generated with uuid4; all mutation probes used temporary directories. Final HEAD and tracked-worktree checks were clean. Progress events were published during review and testing.
[VERDICT: PASS]