Posthog code/fix prompt session storage - #34
Merged
Conversation
… hook binary paths Prompt transcripts and authorship notes could silently stop being stored through two independent failure modes: 1. The daemon's flush loop only ran flush_cas_queue()/flush_notes()/ flush_pending_to_cloud() when the in-memory telemetry buffer was non-empty, so durable queues could sit pending indefinitely on an otherwise idle daemon. The loop now flushes every tick; each durable drain first does a cheap local pending-count check so an empty queue never triggers an auth check (and its potential token-refresh network call). When pending work exists but auth is unavailable, a rate-limited WARN is logged (previously debug-level, invisible in daemon logs) and a 5-minute backoff prevents refresh spam. 2. install-hooks persisted std::env::current_exe() into agent hook configs. When run from a cargo build directory (e.g. a temporary worktree's target/debug binary), the recorded path dies as soon as that directory is removed, and every subsequent checkpoint hook fails silently — no sessions, no prompts, empty authorship notes. resolve_hook_binary_path() now prefers the stable installed binary (~/.autter/bin/autter) whenever the current executable is a cargo artifact. Generated-By: PostHog Code Task-Id: e5855956-9de8-49a0-836b-8a2c91373bef
…nfig dirs Claude Code reads hooks from ~/.claude (or $CLAUDE_CONFIG_DIR), but CLIs built on the Claude Agent SDK run their sessions with CLAUDE_CONFIG_DIR pointing at their own config directory. Hooks installed there were invisible to `autter install-hooks` run from a terminal, so a stale hook in a harness config silently broke checkpointing for every session of that harness with no repair path. Each `autter checkpoint claude` invocation runs inside the harness's environment, so it now records any non-default CLAUDE_CONFIG_DIR in ~/.autter/internal/claude-config-dirs.json. The Claude hook installer then installs, repairs, and uninstalls hooks across the primary settings file and every registered harness directory on each install/update run. Harness entries are best-effort (one malformed harness config can't fail the whole install) and no vendor paths are hardcoded, so this works for any current or future Claude-based CLI. Generated-By: PostHog Code Task-Id: e5855956-9de8-49a0-836b-8a2c91373bef
…ud sync Authorship notes previously went to exactly one destination: git refs/notes/ai (git_notes) or the cloud data plane via notes-db (http). notes_backend.kind = "both" now writes every note to git refs/notes/ai AND queues it for cloud upload, so the full record of sessions, prompts, and tool calls travels with the repository (offline, team-shareable via the existing notes push/fetch hooks) while the cloud copy powers hosted features. Semantics in Both mode: - Writes: git note first (durable local copy), then best-effort cloud enqueue — a cloud failure never loses the git note and can be backfilled with `autter notes migrate`. - Reads: local git notes are authoritative; the notes-db cache and cloud API cover commits whose notes only exist remotely (e.g. cloud-only teammates). Blob-OID fast paths work again since notes live in git. - Sync: pre-push pushes refs/notes/ai as in git_notes mode; fetch warms the cloud cache in addition to fetching the notes ref; the daemon flushes pending notes-db rows as in http mode. Transcripts already store dual-destination in the default prompt storage mode (permanent local cas_cache + cloud CAS upload), so notes were the missing piece for full local+cloud coverage. Generated-By: PostHog Code Task-Id: e5855956-9de8-49a0-836b-8a2c91373bef
…ed working Cloud sync pauses silently when auth breaks — previously the only signal was a daemon-log warning nobody reads. Interactive autter commands and git-proxy commands now print a one-line stderr reminder with the pending queue counts and the fix (`autter login`). Detection covers both logged-out shapes: - refresh token past its expiry timestamp, and - a token that is valid by timestamp but rejected by the server (revoked or rotated) — the daemon flush loop persists a sync-auth-blocked stamp whenever pending work meets failed auth, and clears it on the first successful authenticated sync. The notice never fires for users who never logged in, is gated on an interactive stdout so scripts and pipes stay clean, fires at most once per process, and rate-limits to once per 24h across processes. Generated-By: PostHog Code Task-Id: e5855956-9de8-49a0-836b-8a2c91373bef
🚦 Pre-merge checks ·
|
| Check | Status | Explanation |
|---|---|---|
| Mixed concerns (refactor + behavior change) | PR combines multiple refactoring/structural changes (new modules, schema additions) with behavior changes (NotesBackendKind::Http variant, telemetry flush refactor, claude_config_registry registration logic) in a single change set. Split into at least two PRs: (1) Infrastructure/schema PR: add NotesBackendKind::Http va ... [truncated 440 chars] | |
| Migration + app logic + UI combined in one PR | PR combines database schema changes (authorship/internal_db.rs, notes/db.rs, file_changes/db.rs) with application logic (NotesBackendKind::Http routing in fetch_notes, notes_migrate, log, push_hooks) and user-facing notice/output changes (src/auth/notice.rs wired to src/main.rs startup path). Split into two PRs: (1) Mi ... [truncated 365 chars] | |
| Missing audit logging | 10 potential issue(s) detected (max risk 63/100): src/auth/notice.rs:3, src/auth/notice.rs:26, src/auth/notice.rs:119, src/auth/notice.rs:164, src/commands/git_handlers.rs:277. | |
| Removed observability | 1 potential issue(s) detected (max risk 45/100): src/daemon/telemetry_worker.rs:65. | |
| Silent exception swallowing | 5 potential issue(s) detected (max risk 25/100): src/auth/notice.rs:54, src/auth/notice.rs:56, src/auth/notice.rs:61, src/auth/notice.rs:96, src/auth/notice.rs:98. | |
| Multi-write without detected transaction | 2 potential issue(s) detected (max risk 48/100): src/mdm/agents/claude_code.rs:28, src/mdm/claude_config_registry.rs:15. | |
| Optimistic locking not detected | 2 potential issue(s) detected (max risk 40/100): src/mdm/agents/claude_code.rs:24, src/mdm/claude_config_registry.rs:10. | |
| Batch size limit not detected | 4 potential issue(s) detected (max risk 48/100): src/git/notes_api.rs:90, src/mdm/agents/claude_code.rs:39, src/mdm/claude_config_registry.rs:56, src/mdm/utils.rs:809. | |
| Pagination not detected | 3 potential issue(s) detected (max risk 52/100): src/authorship/internal_db.rs:426, src/file_changes/db.rs:265, src/notes/db.rs:345. | |
| Missing linked tracker issue | 1 potential issue(s) detected (max risk 50/100): src/auth/notice.rs:1. | |
| Missing security-team review on sensitive path | 2 potential issue(s) detected (max risk 88/100): src/auth/mod.rs:5, src/auth/notice.rs:1. | |
| Multi-write without transaction wrapper | 1 potential issue(s) detected (max risk 80/100): src/mdm/agents/claude_code.rs:28. | |
| Overbroad try/catch swallowing all exceptions | 2 potential issue(s) detected (max risk 55/100): src/auth/notice.rs:177, src/git/notes_api.rs:915. | |
| Established pattern ignored | 1 potential issue(s) detected (max risk 45/100): src/commands/install_hooks.rs:8. | |
| Unhandled edge case (null / empty / zero / boundary) | 2 potential issue(s) detected (max risk 40/100): src/auth/notice.rs:79, src/commands/fetch_notes.rs:100. | |
| Doc-copy code with insecure defaults | 1 potential issue(s) detected (max risk 70/100): src/git/notes_api.rs:947. | |
| New required field added to request | 1 potential issue(s) detected (max risk 88/100): src/file_changes/mod.rs:80. | |
| OAuth callback / redirect handling changed | 1 potential issue(s) detected (max risk 85/100): src/auth/notice.rs:6. | |
| Code correctness issue | 1 finding(s) on changed lines. | |
| Runtime error risk | 1 finding(s) on changed lines. | |
| Data integrity risk | 1 finding(s) on changed lines. | |
| Dead export (no callers) | 2 finding(s) on changed lines. | |
| Code duplication / DRY violation | 2 finding(s) on changed lines. |
✅ Passed checks (113)
| Check | Status | Explanation |
|---|---|---|
| Too many files changed | ✅ Passed | Changed 25 file(s), within the limit of 50. |
| Too many lines changed | ✅ Passed | Changed 935 line(s), within the limit of 1000. |
| Too many unrelated chapters | ✅ Passed | 2 chapter(s) detected, within the limit of 6. |
| Generated files hiding real changes | ✅ Passed | Generated-file volume (0 lines) does not obscure the 935 hand-written line(s). |
| Missing PR context | ✅ Passed | PR context looks sufficient. |
| Sensitive data in logs | ✅ Passed | No sensitive data in logs issues detected. |
| Log injection | ✅ Passed | No log injection issues detected. |
| Unhandled promise rejection | ✅ Passed | No unhandled promise rejection issues detected. |
| Circuit breaker not detected | ✅ Passed | No circuit breaker not detected issues detected. |
| Stack trace leakage | ✅ Passed | No stack trace leakage issues detected. |
| Possible TOCTOU in critical path | ✅ Passed | No possible toctou in critical path issues detected. |
| Idempotency key not detected | ✅ Passed | No idempotency key not detected issues detected. |
| Possible non-atomic read-modify-write | ✅ Passed | No possible non-atomic read-modify-write issues detected. |
| Rate limiting not detected | ✅ Passed | No rate limiting not detected issues detected. |
| Rate limiting removed | ✅ Passed | No rate limiting removed issues detected. |
| Publicly exposed storage | ✅ Passed | No publicly exposed storage issues detected. |
| Over-permissive IAM policy | ✅ Passed | No over-permissive iam policy issues detected. |
| Security group open to the internet | ✅ Passed | No security group open to the internet issues detected. |
| Unencrypted storage at rest | ✅ Passed | No unencrypted storage at rest issues detected. |
| Infrastructure missing access logging | ✅ Passed | No infrastructure missing access logging issues detected. |
| Hardcoded secret in IaC | ✅ Passed | No hardcoded secret in iac issues detected. |
| Infrastructure misconfiguration | ✅ Passed | No infrastructure misconfiguration issues detected. |
| Deprecated Kubernetes API version | ✅ Passed | No deprecated kubernetes api version issues detected. |
| Compound IaC attack chain | ✅ Passed | No compound iac attack chain issues detected. |
| PII in logs | ✅ Passed | No pii in logs issues detected. |
| PII or internals leaked in error response | ✅ Passed | No pii or internals leaked in error response issues detected. |
| PII stored without application-level encryption | ✅ Passed | No pii stored without application-level encryption issues detected. |
| User data stored without retention controls | ✅ Passed | No user data stored without retention controls issues detected. |
| PII sent to external / cross-border destination | ✅ Passed | No pii sent to external / cross-border destination issues detected. |
| Lockfile resolution / integrity tampered | ✅ Passed | No lockfile resolution / integrity tampered issues detected. |
| Dependency runs install-time lifecycle script | ✅ Passed | No dependency runs install-time lifecycle script issues detected. |
| Possible dependency-confusion attack | ✅ Passed | No possible dependency-confusion attack issues detected. |
| Lockfile resolves a dependency the manifest does not declare | ✅ Passed | No lockfile resolves a dependency the manifest does not declare issues detected. |
| Checked-in build artefact modified without source change | ✅ Passed | No checked-in build artefact modified without source change issues detected. |
| Dockerfile build-step is insecure | ✅ Passed | No dockerfile build-step is insecure issues detected. |
| External artefact pulled in without integrity pinning | ✅ Passed | No external artefact pulled in without integrity pinning issues detected. |
| Changed export, importer not updated | ✅ Passed | No changed export with an un-updated importer detected. |
| Missing CODEOWNERS reviewer approval | ✅ Passed | No missing codeowners reviewer approval issues detected. |
| Source changes without matching tests | ✅ Passed | No source changes without matching tests issues detected. |
| Migration missing rollback / down step | ✅ Passed | No migration missing rollback / down step issues detected. |
| Frontend importing database client directly | ✅ Passed | No frontend importing database client directly issues detected. |
| Route handler bypassing service layer | ✅ Passed | No route handler bypassing service layer issues detected. |
| Backend service importing UI module | ✅ Passed | No backend service importing ui module issues detected. |
| Cross-context internals import | ✅ Passed | No cross-context internals import issues detected. |
| Workspace package rule violation | ✅ Passed | No workspace package rule violation issues detected. |
| Inconsistent logging pattern | ✅ Passed | No inconsistent logging pattern issues detected. |
| Inconsistent error handling | ✅ Passed | No inconsistent error handling issues detected. |
| Endpoint missing input validation | ✅ Passed | No endpoint missing input validation issues detected. |
| New feature shipped without feature flag | ✅ Passed | No new feature shipped without feature flag issues detected. |
| Module placed in the wrong workspace package | ✅ Passed | No module placed in the wrong workspace package issues detected. |
| Hallucinated import (package not installed) | ✅ Passed | No hallucinated import (package not installed) issues detected. |
| Nonexistent package (not found in registry) | ✅ Passed | No nonexistent package (not found in registry) issues detected. |
| Call to function that does not exist | ✅ Passed | No call to function that does not exist issues detected. |
| Generic placeholder identifier in production logic | ✅ Passed | No generic placeholder identifier in production logic issues detected. |
| Repetitive boilerplate (duplicated block) | ✅ Passed | No repetitive boilerplate (duplicated block) issues detected. |
| TODO / FIXME on critical path | ✅ Passed | No todo / fixme on critical path issues detected. |
| Comment contradicts or fabricates code behaviour | ✅ Passed | No comment contradicts or fabricates code behaviour issues detected. |
| Abstraction defined but never used | ✅ Passed | No abstraction defined but never used issues detected. |
| Code style differs from rest of codebase | ✅ Passed | No code style differs from rest of codebase issues detected. |
| Dead code (defined but never referenced) | ✅ Passed | No dead code (defined but never referenced) issues detected. |
| Deprecated API call | ✅ Passed | No deprecated api call issues detected. |
| API pattern from wrong library version | ✅ Passed | No api pattern from wrong library version issues detected. |
| API endpoint removed | ✅ Passed | No api endpoint removed issues detected. |
| HTTP method changed (GET ↔ POST etc.) | ✅ Passed | No http method changed (get ↔ post etc.) issues detected. |
| Field removed from response schema | ✅ Passed | No field removed from response schema issues detected. |
| Response field type changed | ✅ Passed | No response field type changed issues detected. |
| HTTP status code changed | ✅ Passed | No http status code changed issues detected. |
| Auth requirement added / removed / changed | ✅ Passed | No auth requirement added / removed / changed issues detected. |
| Error response shape changed | ✅ Passed | No error response shape changed issues detected. |
| Pagination behaviour changed | ✅ Passed | No pagination behaviour changed issues detected. |
| Outbound webhook payload schema changed | ✅ Passed | No outbound webhook payload schema changed issues detected. |
| GraphQL field removed without deprecation | ✅ Passed | No graphql field removed without deprecation issues detected. |
| GraphQL enum value removed | ✅ Passed | No graphql enum value removed issues detected. |
| SQL injection | ✅ Passed | No sql injection issues detected. |
| Cross-site scripting (XSS) | ✅ Passed | No cross-site scripting (xss) issues detected. |
| Path traversal | ✅ Passed | No path traversal issues detected. |
| Command injection | ✅ Passed | No command injection issues detected. |
| Insecure deserialization | ✅ Passed | No insecure deserialization issues detected. |
| Weak cryptography | ✅ Passed | No weak cryptography issues detected. |
| Hardcoded secret | ✅ Passed | No hardcoded secret issues detected. |
| Insecure randomness for security material | ✅ Passed | No insecure randomness for security material issues detected. |
| Unsafe file upload | ✅ Passed | No unsafe file upload issues detected. |
| Missing input validation | ✅ Passed | No missing input validation issues detected. |
| Unsafe CORS configuration | ✅ Passed | No unsafe cors configuration issues detected. |
| Unsafe / open redirect | ✅ Passed | No unsafe / open redirect issues detected. |
| Missing CSRF protection | ✅ Passed | No missing csrf protection issues detected. |
| Unsafe cookie / session settings | ✅ Passed | No unsafe cookie / session settings issues detected. |
| Sensitive data exposure | ✅ Passed | No sensitive data exposure issues detected. |
| API key in source | ✅ Passed | No api key in source detected. |
| Access token in source | ✅ Passed | No access token in source detected. |
| Private key in source | ✅ Passed | No private key in source detected. |
| Database connection URL with embedded credentials | ✅ Passed | No database connection url with embedded credentials detected. |
| Cloud credential in source | ✅ Passed | No cloud credential in source detected. |
| Webhook signing secret in source | ✅ Passed | No webhook signing secret in source detected. |
| OAuth client secret in source | ✅ Passed | No oauth client secret in source detected. |
| JWT signing secret in source | ✅ Passed | No jwt signing secret in source detected. |
| Auth middleware removed from route | ✅ Passed | No auth middleware removed from route issues detected. |
| Route protection changed (protected → public) | ✅ Passed | No route protection changed (protected → public) issues detected. |
| Permission / RBAC check removed | ✅ Passed | No permission / rbac check removed issues detected. |
| Required role weakened | ✅ Passed | No required role weakened issues detected. |
| Admin-only route exposed to lower privilege | ✅ Passed | No admin-only route exposed to lower privilege issues detected. |
| Token validation skipped in middleware chain | ✅ Passed | No token validation skipped in middleware chain issues detected. |
| JWT verification weakened or changed | ✅ Passed | No jwt verification weakened or changed issues detected. |
| Session expiration / TTL changed | ✅ Passed | No session expiration / ttl changed issues detected. |
| Password reset flow changed | ✅ Passed | No password reset flow changed issues detected. |
| Webhook endpoint missing signature verification | ✅ Passed | No webhook endpoint missing signature verification issues detected. |
| Public route touches private/PII data | ✅ Passed | No public route touches private/pii data issues detected. |
| Resource leak risk | ✅ Passed | No additional explanation was reported. |
| Maintainability issue | ✅ Passed | No additional explanation was reported. |
| Redundant alias / duplicate import | ✅ Passed | No additional explanation was reported. |
| Redundant type construct | ✅ Passed | No additional explanation was reported. |
| Unnecessary type assertion | ✅ Passed | No additional explanation was reported. |
| Module smell | ✅ Passed | No additional explanation was reported. |
This comment is updated automatically whenever Autter reviews a new PR revision.
Autter task list
Generated from PR diff, blast radius, and context. Issues found
And 18 additional finding(s) in the Autter review. 🛠 Fix options (coming soon)Choose how you would like Autter to package unresolved fixes once automated fix PRs are available.
These checkboxes are currently informational and do not trigger a fix run yet. |
🧭 PR hygiene & process suggestionsAutter has 2 suggestion(s) about the shape of this PR (size, scope, reviewability). These are process guidance — not code defects — so they are consolidated here instead of posted as inline comments on individual files. 🟠 Migration + app logic + UI in one PR — Risk: 65/100PR combines database schema changes (authorship/internal_db.rs, notes/db.rs, file_changes/db.rs) with application logic (NotesBackendKind::Http routing in fetch_notes, notes_migrate, log, push_hooks) and user-facing notice/output changes (src/auth/notice.rs wired to src/main.rs startup path). The bundle spans References:
🛠 AI fix prompt (copy & paste into your coding agent)🟠 PR mixes refactor and behavior change — Risk: 55/100PR combines multiple refactoring/structural changes (new modules, schema additions) with behavior changes (NotesBackendKind::Http variant, telemetry flush refactor, claude_config_registry registration logic) in a single change set. References:
🛠 AI fix prompt (copy & paste into your coding agent)Flagged by Autter PR-hygiene checks. |
🔇 45 finding(s) suppressed as likely false positives by Autter's verification passThese were flagged by a detector but a second, full-file verification judged them not to be real issues. Listed here for transparency — review if you disagree.
|
![autter-dev[bot]](https://avatars.githubusercontent.com/in/2920597?s=60&v=4){kind=small}
There was a problem hiding this comment.
Autter completed PR review for #34. Generated a PR summary, 8 task(s), and 48 finding(s).
| @@ -16,14 +16,14 @@ mod amp; | |||
| mod attribution_tracker_comprehensive; | |||
There was a problem hiding this comment.
Captain Autter Patch via Autter
why do we even need to make this change randomly?
There was a problem hiding this comment.
Autter found 13 security/observability issue(s) in this PR:
- 🔴 [ai] No Test Coverage for Both Option in NotesBackendKind (risk 85/100) —
src/config.rs:43 — downstream: 1 affected - 🔴 [ai] Missing Test for register_active_config_dir Function (risk 80/100) —
src/commands/checkpoint_agent/presets/claude.rs:53 — downstream: 1 affected - 🟠 [ai] New Durable Sync Auth Backoff Logic Not Tested (risk 75/100) —
src/daemon/telemetry_worker.rs:284 — downstream: 1 affected - 🟠 [ai] Lack of Test for count_pending_cas Method (risk 70/100) —
src/authorship/internal_db.rs:423 — downstream: 1 affected - 🟠 [ai] Potential Inefficiency with CAS Queue Handling (risk 60/100) —
src/authorship/internal_db.rs:423 — downstream: 1 affected - 🟠 [ai] flush_pending_to_cloud silently exits on unauthenticated state without arming the durable-queue backoff (risk 52/100) —
src/file_changes/mod.rs:92 — downstream: 2 affected - 🟠 [ai] flush_pending_to_cloud does not participate in the durable-queue auth backoff, causing repeated token-refresh attempts every 3 s when only the file-changes queue has pending data (risk 52/100) —
src/file_changes/mod.rs:92 — downstream: 3 affected - 🟠 [ai] flush_pending_to_cloud silently bypasses auth backoff and blocked-sync notice on unauthenticated failure (risk 52/100) —
src/file_changes/mod.rs:92 — downstream: 4 affected - 🟡 [ai] TOCTOU lost-update race in register_active_config_dir: concurrent checkpoint hooks corrupt the registry (risk 42/100) —
src/mdm/claude_config_registry.rs:62 — downstream: 3 affected - 🟡 [ai] Unbounded DB Calls (risk 40/100) —
src/notes/db.rs:342 — downstream: 1 affected - 🟡 [ai] Redundant Configuration Management Addition (risk 30/100) —
src/mdm/claude_config_registry.rs:1 — downstream: 2 affected - 🟡 [ai] flush_pending_to_cloud auth failure does not arm the backoff, enabling per-tick token-refresh calls to the auth server (risk 25/100) —
src/file_changes/mod.rs:92 — downstream: 3 affected - 🟡 [ai] Missing User Feedback on Authentication Failure (risk 20/100) —
src/commands/git_handlers.rs:275 — downstream: 3 affected
|
|
||
| impl NotesBackendKind { | ||
| pub fn as_str(&self) -> &'static str { | ||
| match self { |
There was a problem hiding this comment.
🔴 [ai] No Test Coverage for Both Option in NotesBackendKind — Risk: 85/100
The introduction of the Both option in NotesBackendKind lacks test coverage, posing a risk for missed integration behaviors.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/config.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
|
|
||
| // This process runs inside the agent's environment: when a harness | ||
| // built on the Claude Agent SDK routes hooks through its own | ||
| // CLAUDE_CONFIG_DIR, record that directory so install/update runs |
There was a problem hiding this comment.
🔴 [ai] Missing Test for register_active_config_dir Function — Risk: 80/100
The call to register_active_config_dir within claude.rs lacks testing, which may lead to unverified configuration behavior.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/commands/checkpoint_agent/presets/claude.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| let config = Config::get(); | ||
| let distinct_id = get_or_create_distinct_id(); | ||
|
|
||
| // Flush metrics (always processed — uploaded or stored in SQLite) |
There was a problem hiding this comment.
🟠 [ai] New Durable Sync Auth Backoff Logic Not Tested — Risk: 75/100
The implementation of durable sync auth backoff logic lacks tests to verify its effectiveness and correct behavior.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/daemon/telemetry_worker.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| @@ -423,6 +423,18 @@ impl InternalDatabase { | |||
| Ok(records) | |||
There was a problem hiding this comment.
🟠 [ai] Lack of Test for count_pending_cas Method — Risk: 70/100
The newly added count_pending_cas method in InternalDatabase.rs does not have a corresponding test, potentially leading to unverified function behavior.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/authorship/internal_db.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| @@ -423,6 +423,18 @@ impl InternalDatabase { | |||
| Ok(records) | |||
There was a problem hiding this comment.
🟠 [ai] Potential Inefficiency with CAS Queue Handling — Risk: 60/100
Unnecessary queuing and dequeueing without checking if sync is possible might lead to resource exhaustion.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/authorship/internal_db.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| } | ||
|
|
||
| let path = registry_path(); | ||
| let mut dirs: Vec<String> = std::fs::read_to_string(&path) |
There was a problem hiding this comment.
🟡 [ai] TOCTOU lost-update race in register_active_config_dir: concurrent checkpoint hooks corrupt the registry — Risk: 42/100
register_active_config_dir performs an unguarded read-modify-write on the shared JSON registry file (~/.autter/internal/claude-config-dirs.json). The function reads the file (line 62), checks whether the directory is already present (line 68), appends the new entry (line 71), and writes back via write_atomic (line 77). No file lock or advisory lock protects this window. Because autter checkpoint claude is invoked for every pre-edit and post-edit hook event — and Claude agents routinely fire these hooks in rapid succession across parallel editing sessions — two processes can execute this function concurrently with the SAME registry file. Actor A reads ['/dir/harness-a'], sees '/dir/harness-b' is absent, appends, and writes ['harness-a','harness-b']. Actor B, having read the same original snapshot before A's write, also sees 'harness-b' absent, appends only 'harness-b', and its rename atomically overwrites A's write, resulting in ['harness-b'] alone — losing harness-a's registration. An additional hazard exists inside write_atomic itself: the temp file is always named claude-config-dirs.json.tmp (target_path.with_extension('tmp'), src/mdm/utils.rs:459). Two concurrent callers both call File::create on this same fixed path, which truncates the file; whichever writer is slower will overwrite the other's bytes before either rename fires, potentially leaving a zero-byte or interleaved JSON document that future reads will fail to parse (silently falling back to an empty list and erasing all previously registered directories). The consequence is that a harness config directory is silently dropped from the registry, causing autter install-hooks to skip it on the next run, and that harness's sessions will fire hooks against a stale (or missing) binary path — silently dropping all AI attribution checkpoints for those sessions.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/mdm/claude_config_registry.rs,src/commands/checkpoint_agent/presets/claude.rs,src/mdm/utils.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Replace the bare read-modify-write in register_active_config_dir with a lock-protected update. Use a file lock (e.g. fs2::FileExt::lock_exclusive on a companion .lock file, or the same registry file itself opened for write before reading) to serialize concurrent writers. Acquire the lock, read the current contents, check for duplicates, append if needed, write, flush, then release the lock. Alternatively, since the only mutation is appending a new unique string to a list, consider using a dedicated lock file (e.g. claude-config-dirs.lock) held via flock/LockFile for the duration of the read-check-write cycle. Also fix write_atomic to use a per-call unique temp file name (e.g. via tempfile::NamedTempFile in the same directory) rather than the fixed .tmp extension, so concurrent callers to write_atomic on ANY file don't clobber each other's temp files.
Flagged by Autter security & observability checks.
| @@ -342,6 +342,18 @@ impl NotesDatabase { | |||
|
|
|||
There was a problem hiding this comment.
🟡 [ai] Unbounded DB Calls — Risk: 40/100
Lack of boundary checks or limits akin to unbounded read might induce pressure on the database system.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/notes/db.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| @@ -0,0 +1,96 @@ | |||
| //! Registry of Claude-Code-compatible configuration directories. | |||
There was a problem hiding this comment.
🟡 [ai] Redundant Configuration Management Addition — Risk: 30/100
The claude_config_registry introduces directory management that overlaps significantly with existing configurations found in src/config.rs. This duplication could lead to increased complexity and maintenance issues. Consolidation of these configurations is recommended to unify management practices.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/config.rs,src/commands/config.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
| let default_client = ApiClient::new(crate::api::client::ApiContext::new(Some( | ||
| backend_url.clone(), | ||
| ))); | ||
| if !default_client.is_logged_in() && !default_client.has_api_key() { |
There was a problem hiding this comment.
🟡 [ai] flush_pending_to_cloud auth failure does not arm the backoff, enabling per-tick token-refresh calls to the auth server — Risk: 25/100
This PR introduces a 5-minute auth backoff (DURABLE_SYNC_AUTH_RETRY_AFTER) to prevent hammering the token-refresh endpoint when a durable queue has pending work but authentication is broken. The mechanism works correctly for flush_notes and flush_cas_queue, which call note_durable_sync_unauthenticated() on auth failure. However, flush_pending_to_cloud (file_changes/mod.rs:92-94) silently returns on auth failure without calling note_durable_sync_unauthenticated(). Consequently, when ONLY the file-changes queue has pending rows and auth is broken, the outer durable_sync_auth_backoff_active() guard is never set, and flush_pending_to_cloud runs every daemon tick (~3 s). Each invocation calls ApiClient::new() → try_load_auth_token() which attempts a network token-refresh call (src/api/client.rs:53) when the access token is expired but the refresh token is still nominally valid. This sends a POST to the auth server every 3 seconds until the session finally expires — precisely the abuse pattern the backoff mechanism was designed to prevent. The three co-sibling functions (flush_notes, flush_cas_queue, flush_pending_to_cloud) are documented as a group in telemetry_worker.rs:309-321, making the omission inconsistent with the stated design intent.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/file_changes/mod.rs,src/daemon/telemetry_worker.rs,src/api/client.rs
🛠 AI fix prompt (copy & paste into your coding agent)
In src/file_changes/mod.rs, after the `if !default_client.is_logged_in() && !default_client.has_api_key()` branch that returns early (currently line 92-94), add a call to crate::daemon::telemetry_worker::note_durable_sync_unauthenticated("file_changes", pending_count) before returning, mirroring the pattern in flush_notes (telemetry_worker.rs:651-653) and flush_cas_queue (telemetry_worker.rs:879-881). This requires exposing note_durable_sync_unauthenticated as pub(crate) or moving the auth-backoff logic to a shared module callable from file_changes/mod.rs.
Flagged by Autter security & observability checks.
| @@ -273,6 +273,9 @@ fn handle_git_inner(args: &[String]) { | |||
| // Warn (loudly, once per process) if this CLI is below the platform's | |||
| // minimum required version. Reads the cached releases payload — no network. | |||
| crate::commands::upgrade::maybe_warn_below_min_version(); | |||
There was a problem hiding this comment.
🟡 [ai] Missing User Feedback on Authentication Failure — Risk: 20/100
The proposed changes in PR #34 introduce user prompts for scenarios requiring authentication that previously lacked direct user engagement. While existing functions check authentication, they don't inform users about failures effectively. This enhancement is necessary for user awareness during command execution.
⚠ Downstream affected — if this fails, it cascades to the usage that depends on this file:
- Dependent files:
src/api/client.rs,src/commands/login.rs,src/commands/logout.rs
🛠 AI fix prompt (copy & paste into your coding agent)
Flagged by Autter security & observability checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Summary generated by Autter.
This PR fixes prompt session storage and extends the PostHog telemetry path to correctly handle Claude-based sessions. It introduces a
claude_config_registrymodule that tracks activeCLAUDE_CONFIG_DIRdirectories discovered at hook time, ensures those directories are maintained during install/update cycles, and wires aNotesBackendKind::Httpvariant (alongside the existingBothmode) through the notes pipeline—fetch, migrate, push, log, and the internal/notes DBs. The telemetry worker's flush loop is refactored to improve batch handling, and a newsrc/auth/notice.rsmodule provides structured auth-related notices to surface to the user.Changes
src/mdm/claude_config_registry.rs(new): Registry that recordsCLAUDE_CONFIG_DIRvalues seen during live hook execution, persisted per-user so that install/update runs can maintain hooks in non-default config directories.src/auth/notice.rs(new): Structured notice type for surfacing auth-related messages (e.g. token expiry, missing credentials) to the CLI output path.src/config.rs: AddsNotesBackendKind::Httpvariant and updatesNotesBackendKind::from_str/display logic; threads the new variant intonotes_backend_kind()resolution.src/mdm/agents/claude_code.rs: Callsclaude_config_registry::register_active_config_dir()during hook parsing; extends install/uninstall logic to iterate registered config dirs and maintain hooks within them.src/commands/checkpoint_agent/presets/claude.rs: Callsregister_active_config_dir()at parse time so that harnesses using a customCLAUDE_CONFIG_DIRare automatically tracked.src/daemon/telemetry_worker.rs: Refactors the flush loop—removes stale buffer-drain logic, adds a new flush path for PostHog session data, and improves retry/back-off handling.src/git/notes_api.rs: Extendswrite_notes_batch,read_note,read_notes_batch,read_authorship, and addswarm_cache_for_remoteto support theHttp/Bothbackend modes.src/file_changes/mod.rs/src/file_changes/db.rs: ThreadsNotesBackendKind::Httpintoflush_pending_to_cloudand the underlying SQLite schema.src/authorship/internal_db.rs/src/notes/db.rs: Schema additions to support the new storage paths.src/commands/{fetch_notes,notes_migrate,log,push_hooks}.rs,src/git/sync_authorship.rs: Each updated to branch onNotesBackendKind::HttporBothwhere relevant.src/authorship/cas_bridge.rs: Minor fix toresolve_cas_messages(single-line change, likely an API client context correction).src/main.rs: Wires theauth::noticemodule into the startup path.tests/integration/main.rs: Reorders/adjusts integration test module registration to reflect renamed or moved test modules.Acceptance Criteria
autter config set prompt_storage httpis accepted without error;parse_notes_backend_kindreturnsNotesBackendKind::Http.autter fetch-noteswithnotes_backend_kind = Httpcallswarm_cache_for_remoteand exits successfully without attempting a git fetch.autter fetch-noteswithnotes_backend_kind = Bothcallswarm_cache_for_remoteand then continues with the standard git fetch.CLAUDE_CONFIG_DIRcauses that directory to be registered inclaude_config_registryafter the first hook fires; subsequentautter install-hooksruns install hooks into that directory.autter notes migrateandautter logrespect theHttpbackend variant and do not fall through to git-notes paths whenHttpis configured.Test Plan
task lint && task fmt— confirm no lint/format regressions.task test— full test suite must pass on Ubuntu CI before Mac/Windows.prompt_storage = httpviaautter config set prompt_storage httpand runautter fetch-notes; confirm the HTTP warm-cache path is taken and no git fetch is attempted.prompt_storage = bothand runautter fetch-notes; confirm both warm-cache and git fetch paths execute.CLAUDE_CONFIG_DIRis set to a non-default path; fire a Claude hook; runautter install-hooksand verify hooks appear in the custom config dir.autter daemon tail).autter notes migratewith bothHttpandBothbackend kinds configured; confirm it routes correctly and emits no errors.autter logoutput for a repo withHttpbackend; confirm authorship notes are resolved from the HTTP cache rather than git notes.Rollback Plan
task dev(debug) or the release pipeline.NotesBackendKind::Httpwas written to a user's config, reset it:autter config set prompt_storage default(ornotes). The new variant is additive—existingdefault/notes/localusers are unaffected.claude_config_registryis a local per-user file; if it causes spurious installs, delete it manually from~/.autter/(exact path determined bydirs::data_dir()). Hook installs in extra config dirs are idempotent and removing them does not affect the primary install.auth::noticemodule is read-only at startup; removing the wiring inmain.rsis a one-line revert with no data impact.Related Issues
No linked issue was identified in the branch name (
posthog-code/fix-prompt-session-storage), commit messages, or diff context.Written for commit 83b3321. Summary will update on new commits.