Per Claude Code plugin docs (code.claude.com/docs/en/plugins):
- commands/ belongs at plugin ROOT, not inside .claude-plugin/ (only
plugin.json goes in .claude-plugin/). Moved via git mv.
- Repo-local .claude/plugins/<name>/ is NOT auto-discovered. Load via
`claude --plugin-dir` or `claude plugin init`.
- Plugin commands are namespaced: invoke as /code-review-tea:code-review,
not /code-review-tea.
- Bake verified Task 2 facts into RESOLVED_INVOCATION content: -F not -f,
/repos/ prefix required, tea pulls --fields diff empty (use git diff),
SHA must be remote-present.
Plan only (no plugin source changed beyond the dir move).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Incorporate findings from Task 2 verification:
- -F (typed field) reads @-/@file; -f posts literal '@-'. Use -F.
- Endpoint requires /repos/ prefix: /repos/{owner}/{repo}/...
- tea pulls --fields diff returns empty in tea 0.14.1 -> use git diff
- SHA must be remote-present (PR headSha); local-only SHA 404s
- tea api prints JSON to stdout; -o is file-output not format
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Loops phone-recorded mp4 files as local RTSP streams via MediaMTX +
ffmpeg so Vigilar's camera pipeline can be exercised end-to-end
without physical cameras. Reads camera ids from vigilar.toml,
expects videos/<id>.mp4 for each, writes a vigilar.sim.toml with
rtsp_urls rewritten to the local endpoints.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Task-by-task plan for the dev-only RTSP simulator: gitignore entries,
MediaMTX downloader, syntax-check test guard, the sim script itself,
and a human acceptance walk-through.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dev-only tooling to loop phone-recorded mp4s as local RTSP streams
(MediaMTX + ffmpeg) so the real camera pipeline can be exercised
without physical cameras.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follow-up to the holistic review of the PIN-unification branch:
- /system/status now reads the real arm state from the arm_state_log
table via get_current_arm_state, instead of returning a hardcoded
'DISARMED' stub. Without this, polling after the new async 202
arm/disarm flow was a UX dead-end — clients never saw the state
change they just requested. DB read failures degrade gracefully.
- Operator guide: correct the claim that 'vigilar config set-pin'
populates recovery_passphrase_hash. It doesn't. recovery_passphrase
_hash has no CLI helper today; it must be set manually.
- Tests: add a fail-closed regression for verify_pin on malformed
stored hashes, and a companion test confirming the deprecation
warning stays silent on a fully migrated config.
All address specific review comments on the branch; no scope creep.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Describes the canonical [security] pin_hash key, the PBKDF2 format
emitted by 'vigilar config set-pin', and the deprecation warning for
the legacy [system] arm_pin_hash. Drops the three-way mismatch
known-limitation.
Code review on 9f203d8 caught a silent-failure mode: MessageBus.connect
logs and returns without raising when the MQTT handshake times out, so
an overloaded broker would let bus.publish() enqueue into paho's outbox
only to be discarded by the immediate disconnect(). The web endpoint
would return 202 even though the FSM never received the request.
Guard with 'if not bus.connected: raise RuntimeError'. The existing
try/except in arm_system/disarm_system catches the exception and turns
it into a 503 with the same log message as other bus failures.
Follow-up to efd5c4a. The plan invented {"accepted": True, ...} for
the new 202 responses, but every other 2xx endpoint in the Flask app
returns {"ok": True, ...} — including cameras.py:108 which is direct
prior art for a 202 with the same convention. The shared JS helper
at static/js/settings.js:54 does 'if (resp.ok && result.ok)' and was
falling into the error branch on our success responses, showing a
bogus "Save failed" toast after every arm/disarm click.
Keep the 202 status. Swap the body key from 'accepted' to 'ok'.
No JS change needed.
Was: /system/api/arm verified the PIN against [security] pin_hash and
returned {ok: true} without ever calling the FSM. State never changed.
Now: the endpoint publishes a SYSTEM_ARM_REQUEST message to the local
MQTT broker. The event processor (see previous commit) picks it up,
ArmStateFSM verifies the PIN via alerts.pin.verify_pin and performs
the transition. Response is 202 Accepted; clients poll /system/status
for the new state.
Design: PIN travels over localhost-only MQTT, which matches the
existing trust boundary for the internal bus.
Code review follow-up on f4d66dd:
- _handle_arm_request signature used "ArmStateFSM" as a string forward
reference even though the type is imported at module top.
_handle_event uses the bare form; match it for consistency.
- Add a test asserting that omitting triggered_by in an arm-request
payload defaults to "unknown". That value feeds the audit log, so
it deserves explicit regression coverage.
No behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds _handle_arm_request and a dedicated bus.subscribe on
Topics.SYSTEM_ARM_REQUEST. Payload {mode, pin, triggered_by} is
dispatched to ArmStateFSM.transition, which verifies the PIN via
alerts.pin.verify_pin and performs the state change.
This is the missing link for web /system/api/arm to actually move
the system into an armed state. Part of issue #2.
Code review follow-up on c9dd348 — the log = logging.getLogger(__name__)
assignment was interleaved between 'import X' and 'from X import Y'
statements. Move it below all imports per standard ordering.
No behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If a config still has the legacy [system] arm_pin_hash set but no
[security] pin_hash, load_config logs a WARNING telling the operator
to re-run 'vigilar config set-pin'. The legacy field is still parsed
(so old configs don't fail validation) but ignored at runtime.
Part of issue #2 PIN hashing unification.
Adjacent secret leak in show_cmd noticed during Task 3 code review.
SecurityConfig has two sensitive fields and the redaction block only
covered pin_hash. vigilar config show would print the recovery
passphrase hash verbatim whenever one was configured.
One-line fix; same redaction pattern as the surrounding secrets.
Part of issue #2.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Was: HMAC-SHA256(random, pin) written to [system] arm_pin_hash —
no verifier in the codebase accepted this output.
Now: PBKDF2-SHA256 via alerts.pin.hash_pin written to [security]
pin_hash, matching what the web and FSM paths verify against.
Also fixes show_cmd to redact the new location.
Code review feedback on the Task 2 commit (7fda351):
- The 'verify_pin as _verify_pin_hash' alias was unnecessary — the
method self.verify_pin and the module-level verify_pin do not
collide (one is accessed via self, the other via the bare name).
Removing the alias matches how web/blueprints/system.py already
imports verify_pin and makes the call site read cleanly.
- The comment on the insert_arm_state None argument now explains
WHY (PBKDF2 salt is fresh per call, so re-hashing is worthless for
audit correlation) instead of only referencing the issue.
No behavior change. Part of issue #2.
Was: unsalted SHA-256 read from [system] arm_pin_hash.
Now: PBKDF2-SHA256 600k iterations read from [security] pin_hash,
matching the web arm/disarm path and the alerts/pin module.
Also drops the redundant pin re-hash on the arm_state_log audit row
(a fresh PBKDF2 salt made the column valueless for traceability).
Part of issue #2 PIN hashing unification.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Topic for web-originated arm/disarm requests that the event processor
will subscribe to and dispatch to ArmStateFSM.transition. Part of the
PIN unification work (issue #2).
Plan document for issue #2 — the three-way PIN hash mismatch across
CLI, events FSM, and web arm/disarm. Proposes canonicalizing on
PBKDF2-SHA256 via alerts/pin and [security] pin_hash, deprecating
[system] arm_pin_hash, and wiring web arm/disarm through MQTT to the
FSM so the web buttons actually transition state.
Nine tasks, TDD throughout. No code changes in this commit.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Closes#1.
The Flask event-timeline was dead: `broadcast_sse_event` existed in
`vigilar/web/blueprints/events.py` but had zero call sites. Clients
subscribed to `/events/stream`, received the initial "connected"
message, and then only keepalives — a page refresh was required to
see new events. (Web Push via VAPID was independent and already worked.)
The root cause was a process-boundary gap: the events subsystem runs
in its own OS process and emits to MQTT, while the Flask app runs in a
separate process with no MQTT client of its own.
This change adds a thin bridge:
- EventProcessor._handle_event now publishes a classified summary
(id, ts, type, severity, source_id, payload) to a new topic
`Topics.EVENTS_PUBLISHED = "vigilar/events/published"` right after
`insert_event()`. Classification logic stays in one place.
- A new module `vigilar/web/sse_bridge.py` provides `forward_event`
(MQTT handler) and `start_sse_bridge(cfg)` (creates a MessageBus,
subscribes forward_event to EVENTS_PUBLISHED, connects, returns the
bus).
- `vigilar/main.py:_run_web` starts the bridge after `create_app(cfg)`
and disconnects it on shutdown. Bridge failure is logged but does
not kill the web process — the UI still works without live updates.
- `create_app` is deliberately NOT changed. Keeping the bridge out of
the app factory means no existing test triggers a real MQTT
connection, and the bridge stays a production-only concern wired by
the supervisor.
Tests (all added with TDD, RED verified before GREEN):
- tests/unit/test_events.py::TestEventsPublishedBroadcast — asserts
`_handle_event` publishes the classified payload for a motion event
and does NOT publish for unclassified topics (heartbeats).
- tests/unit/test_sse_bridge.py — asserts `forward_event` reaches SSE
subscribers, and `start_sse_bridge` wires the handler to
`Topics.EVENTS_PUBLISHED` on a connected bus (fake bus, no real
MQTT in tests).
Also refreshes the docs that previously flagged the dead SSE as a
known limitation (operator guide, web architecture doc).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Some web endpoint handlers call _save_and_reload(), which resolves the
target path via VIGILAR_CONFIG env var with a fallback to the relative
"config/vigilar.toml". Any test exercising such an endpoint without
setting the env var rewrites the repo's committed config file via a
Pydantic model_dump round-trip, stripping comments and non-default
fields. The culprit discovered was test_reset_pin_correct_passphrase
in tests/unit/test_system_pin.py.
Add an autouse session-scoped fixture in tests/conftest.py that points
VIGILAR_CONFIG at a path inside pytest's session tmp dir so no test
can touch the real file. Restore the previous env var value on teardown.
Fixes#3.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The set_password_cmd docstring and inline comment claimed bcrypt /
SHA-256, but the implementation actually uses scrypt via
cryptography.hazmat.primitives.kdf.scrypt. Correct the docstring,
drop the misleading comment, and remove the now-unused hashlib import.
No behavior change.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Convert the "Where to go next" items in the architecture overview from
plain text to proper Markdown links. This was the only finding from the
Task 18 verification pass; everything else (links, commands, TOML
coverage, subsystem coverage, terminology) is self-consistent across
the 17 new doc files.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Front-door for the repo. Pitches the project, surfaces known limitations
honestly (timeline not live, CTR not authenticated, no migrations), and
links into the three doc trees: home user guide, operator guide, and
architecture overview + 12 subsystem references.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Linear walkthrough from bare mini PC to working cameras on phone, with
optional NAS backup. Verified against real install.sh, backup.sh, and
CLI subcommands; honest about the in-browser event timeline not being
wired to SSE yet (push notifications do work).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
18 tasks covering README, home user guide, operator guide, architecture
overview + conventions, and 12 per-subsystem reference docs. Each task
is grounded in reading real source to avoid invented facts.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Specify that backup timer snippets are inline in the guides, not
shipped as new unit files, to match the no-code-changes scope.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Captures scope and structure for top-level README, home user guide,
operator guide, and architecture docs (overview + conventions + 12
per-subsystem files). Approach 3 (hybrid): monolithic user guides,
split architecture reference.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
