fix: harden Flow inspector safety#28
Conversation
Fail closed on incomplete check evidence, reconcile mutually exclusive labels, and keep all mutations behind explicit flags. Add atomic locked assignment updates, portable configuration, table-driven coverage, and operator guidance. Closes #24. Signed-off-by: John McChesney TenEyck Jr <59268465+jmcte@users.noreply.github.com>
athena-omt
left a comment
There was a problem hiding this comment.
Review Summary
Approved. This PR hardens the Flow inspector by removing machine-specific hardcoded paths, extracting pure classification logic into a separate module (inspector_core.py), and making all mutation gates explicit.
Safety Improvements
-
No implicit paths — Removed
WORKSPACE,GATE,DEFAULT_REPORT_DIR, andDEFAULT_ASSIGNMENTSconstants that tied the inspector to Pheidon's machine. All paths are now parameterized or default to explicit user input (--reporequired,--assignmentsrequired for dispatch). -
Mutation gated —
--apply-labelsand--dispatch-repairsare opt-in flags only. Default mode reportsmode: read-onlyand printsproposedWriteswithout executing any side effects. -
Atomic assignment writes —
dispatch_repairsnow usesfcntl.flock(LOCK_EX)for exclusive locking during the read/modify/write transaction,tempfile.NamedTemporaryFile+os.replace()for atomic file replacement with directory fsync, and cleanup of temp files in afinallyblock. -
Check rollup fail-closed —
check_summarynow treats empty rollups (observed: 0) as non-green (indeterminate), and handlesCANCELLED,STALE,STARTUP_FAILURE, andTIMED_OUTas failures rather than silently ignoring them. -
Maintenance gate parameterized — Moved from hardcoded
GATEpath to explicit--maintenance-gate <path>argument, allowing operator configuration per host.
Architecture
- Pure classification logic (
STATE_LABEL_BY_FLOW,LANE_LABEL_BY_ACTOR,LABEL_SPECS,classify_pr,classify_issue,plan_label_changes,plan_repair_dispatch, etc.) extracted intoinspector_core.py— importable, testable, zero side effects. - CLI layer in
inspect_repo_flow.pyhandles argparse, file I/O, and subprocess calls only.
Tests
All 15 tests pass locally on OMT-NAS (Athena's node):
| Category | Tests | Scope |
|---|---|---|
| CheckSummary | 2 | Empty/pending/passing/failing/neutral/skipped/cancelled/unknown classification |
| PR Classification | 2 | State matrix + change-request evidence |
| Issue Classification | 3 | Intake/planning, unlabelled contract detection, blocked-human routing |
| Mutation Planning | 4 | Mutually exclusive label families, add+remove mutation, dedup dispatch, atomic lock verify |
| CLI Safety | 4 | --repo required, no mutation flags by default, --dispatch-repairs requires --assignments, read-only main |
Evidence
- Branch:
pr28at7316625a— rebased on latestorigin/main(no drift) - CI: all 4 checks green (Analyze actions/python, CI Gate, CodeQL)
- Local:
py_compilepasses for both modules;python3 -m unittest discover -s tests -p 'test_flow_inspector.py' -v— 15/15 OK - No pre-existing reviews or comments
Residual Risk / Follow-ups
- None blocking. The refactoring is clean and conservative by design.
- Operator should ensure
--maintenance-gatepath exists on any new host before enabling maintenance checks.
Summary
Governing Issue
Closes #24
Validation
CI GateEvidence:
PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s tests -v— 83 tests passedpython3 -m py_compile scripts/flow/inspector_core.py scripts/flow/inspect_repo_flow.py— passedgit diff --check— passedpython3 scripts/flow/inspect_repo_flow.py --repo OMT-Global/flow --issues --limit 10— live read-only smoke passed and reported proposed writes without mutationBootstrap Governance
CONTRIBUTING.md,.github/PULL_REQUEST_TEMPLATE.md, anddocs/bootstrap/onboarding.mdwhen applicableNo contributor or generated-bootstrap contract changed; the operator-facing README and dedicated inspector guide were updated.
Flow Contract
Flow Merge Readiness
Next actor: Ares or another non-author reviewer. Next action: validate fail-closed classifications, mutation planning, and atomic assignment semantics, then approve or file exact repair feedback.
Merge Automation
gh pr merge --auto --squash, or the reason it is unavailable/unsafe is noted belowAuto-merge will be armed immediately after PR creation; branch protection remains authoritative.
Notes