Skip to content

Fix/sock pids store#110

Merged
donk8r merged 3 commits into
masterfrom
fix/sock-pids-store
Jul 17, 2026
Merged

Fix/sock pids store#110
donk8r merged 3 commits into
masterfrom
fix/sock-pids-store

Conversation

@donk8r

@donk8r donk8r commented Jul 17, 2026

Copy link
Copy Markdown
Member

No description provided.

donk8r added 2 commits July 17, 2026 14:21
- Remove ast_grep and list_files from filesystem documentation
- Replace ast_grep with workdir in agent role configurations
- Document --schema CLI flag for structured output
- Clarify event streams and schema enforcement distinctions
- Update provider compatibility and JSONL tool examples
- Clean up octofs capabilities in role documentation
- Replace keyring-core and db-keystore with JSON file storage
- Implement atomic writes and file locking for token access
- Move session sockets and PID files to XDG_RUNTIME_DIR
- Add ownership and permission validation for the run directory
- Remove turso, loom, miette, and other unused dependencies
- Update CLI reference and troubleshooting guides for socket paths
- Add tests for run directory privacy and location
- Clean up redundant crate versions in Cargo.lock
@github-actions

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Relocate session runtime files to XDG_RUNTIME_DIR and migrate OAuth token store from SQLite to JSON

Overall risk: 🟡 MEDIUM · Cards: 3

# Change Risk Confidence
1 Session socket/PID files moved from data dir to $XDG_RUNTIME_DIR or /tmp/octomind-<uid> with 0700 perms + ownership validation 🟡 ●●●
2 OAuth token storage replaced keyring-core/SQLite with plain JSON file using atomic rename, in-process Mutex 🟡 ●●●
3 Docs: removed ast_grep/list_files from filesystem tool references, documented --schema CLI flag 🟢 ●●●

Card 1/3: Run directory relocated to runtime/temp storage with security hardening · 🟡 · ●●●

INTENT
Session Unix sockets and PID files are host-local runtime state — they can't be bound on NFS and are meaningless across hosts. Moving them out of the persistent data dir prevents stale sockets from lingering after crashes/reboots and aligns with XDG conventions.

WHAT CHANGED

  • get_run_dir() now returns $XDG_RUNTIME_DIR/octomind (when set) or <system tmp>/octomind-<uid> instead of <data_dir>/run
  • New ensure_private_dir() creates the dir with mode 0700, or validates a pre-existing one: rejects symlinks, rejects foreign-uid ownership, repairs permissive modes
  • Legacy <data_dir>/run is silently removed on every call to get_run_dir() to clean up stale files from prior versions
  • All consumers (inject_listener.rs, commands/send.rs, print_directory_info) call get_run_dir() so they automatically follow the new location

IMPACT RADIUS

  • inject_listener.rs binds Unix sockets and writes PID files via get_run_dir() — no change needed, already routed through the function
  • commands/send.rs resolves the socket path via get_run_dir() — same, automatically follows
  • print_directory_info() displays the new path — cosmetic

RISK
🟡 MEDIUM. XDG_RUNTIME_DIR is unset in many headless environments (cron, SSH non-login shells, systemd services without pam_systemd). The fallback to /tmp/octomind-<uid> works, but /tmp is not guaranteed to be cleaned on reboot on all systems (though typically is via systemd-tmpfiles). The ensure_private_dir security checks are sound: symlink_metadata prevents TOCTOU on the dir itself, ownership check prevents squatting in shared /tmp, and permission repair handles drift. The legacy remove_dir_all ignores errors silently — acceptable since it's best-effort cleanup.

QUESTIONS

  • The legacy run/ dir is removed on every get_run_dir() call, not just once. Is the repeated exists() + remove_dir_all check intentional as ongoing cleanup, or should it be a one-time migration?

📎 Source

Card 2/3: OAuth token store migrated from SQLite/keyring to atomic JSON file · 🟡 · ●●●

INTENT
Replace the keyring-core + db-keystore (SQLite) credential store with a plain JSON file to eliminate dependencies that don't work well on NFS-mounted homes (SQLite WAL mode) and simplify the headless/CI deployment story.

WHAT CHANGED

  • save_token, load_token, clear_token now read/write a single keystore.json file (mode 0600) instead of per-server keyring entries backed by SQLite
  • Writes are atomic: temp file (.keystore.<pid>.tmp, mode 0600) → fs::sync_allrename over target
  • In-process Mutex<()> serializes read-modify-write cycles within a single process
  • read_all returns SerializationError on corrupt JSON rather than silently starting fresh — prevents endless re-auth loops
  • keystore_path() is #[cfg(test)]-gated to use an isolated temp file so tests never touch the real keystore
  • keyring-core and db-keystore dependencies removed from Cargo.toml; ~950 lines of transitive deps removed from Cargo.lock

IMPACT RADIUS

  • src/mcp/server.rs:470 calls token_store::get_valid_token() — unchanged signature, transparent migration
  • src/mcp/oauth/callback_server.rs:370 calls save_token() — unchanged signature, transparent migration
  • src/mcp/oauth/mod.rs re-exports the same public API — no downstream breakage

RISK
🟡 MEDIUM. The FILE_LOCK is a std::sync::Mutex — only serializes within one process. Two concurrent octomind processes writing tokens for different servers will last-writer-wins: the second process's read_all → insert → write_all cycle overwrites the first's entry for a different server. The commit message acknowledges this ("a lost update at worst forces one extra re-auth"), but in practice this means a token saved by one session can silently disappear if another session saves a different token concurrently. FILE_LOCK.lock().unwrap() will panic if the Mutex is poisoned — unlikely given the critical section returns Result rather than panicking, but worth noting.

DIVERGENCE
🧩 Incomplete change: The write_all temp file name uses std::process::id() — two threads in the same process share the same PID, so concurrent save_token calls from different async tasks would collide on the temp file name. The FILE_LOCK prevents this within a process, but the coupling between the lock and the temp-file naming is implicit, not enforced.

📎 Source

Card 3/3: Documentation: filesystem tool list cleanup and --schema flag docs · 🟢 · ●●●

INTENT
Update documentation to reflect that ast_grep and list_files are no longer provided by the octofs filesystem server, and document the existing --schema CLI flag for structured output enforcement.

WHAT CHANGED

  • Removed ast_grep and list_files from all filesystem tool references across 7 doc files
  • Updated 11-structured-output.md to document --schema <file> on octomind run (previously stated schema enforcement was "not user-accessible")
  • Updated socket path references in all docs to reflect the new $XDG_RUNTIME_DIR location
  • Removed keyring from the external crates list in architecture docs

IMPACT RADIUS

  • Documentation-only change. The --schema flag already exists in src/commands/run.rs:67 and is wired through src/session/chat/session/setup.rs — docs are catching up to code.
  • ast_grep and list_files still appear in code (src/mcp/utils.rs:26, src/utils/truncation.rs:251-254) as defensive fallbacks for tool routing and truncation hints — these handle the case where an older octofs version still provides those tools. No code change needed.

RISK
🟢 LOW. Documentation-only. No behavioral impact.

📎 Source


📂 Files changed (18 files, ~1319 lines)

{tokens:337667,cost:0.5151058}

- Bump rust dependency versions
- Update crate versions in lockfile
@github-actions

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

This is a single commit (chore(deps): update rust dependencies and lockfile) that touches only Cargo.lock — no source code changes. 25 dependency patch/minor bumps. Let me produce the brief.

📦 Brief: Dependency lockfile update — 25 crates bumped, no source code changes

Overall risk: 🟢 LOW · Cards: 1

# Change Risk Confidence
1 Patch/minor bumps to 25 dependencies including tokio, clap, rustls, octolib 🟢 ●●●
Card 1/1: Dependency lockfile refresh — 25 crates bumped · 🟢 · ●●●

INTENT
Routine dependency maintenance — bump transitive and direct crate versions to latest patch/minor releases. Commit message: chore(deps): update rust dependencies and lockfile.

WHAT CHANGED

  • 25 crates bumped, all patch or minor level. Notable ones:
    • tokio 1.52.3 → 1.53.0 (minor — async runtime)
    • octolib 0.25.1 → 0.25.2 (minor — the project's own library dependency)
    • clap 4.6.1 → 4.6.2, clap_builder 4.6.0 → 4.6.2
    • rustls 0.23.41 → 0.23.42 (TLS)
    • http-body 1.0.1 → 1.1.0, http-body-util 0.1.3 → 0.1.4
    • regex 1.13.0 → 1.13.1, regex-automata 0.4.15 → 0.4.16
    • mio 1.2.1 → 1.2.2, socket2 0.6.4 → 0.6.5
    • uuid 1.23.4 → 1.24.0, syn 2.0.118 → 2.0.119
    • toml 1.1.2 → 1.1.3, bstr 1.12.3 → 1.13.0, globset 0.4.18 → 0.4.19
    • simd_cesu8 1.1.1 → 1.2.0, portable-atomic 1.13.1 → 1.14.0
  • No Cargo.toml changes — all bumps are lockfile-only resolution updates
  • No source code files modified

IMPACT RADIUS
No source code changed. The only behavioral surface is whether the updated transitive crates introduce regressions at runtime. tokio 1.52→1.53 and octolib 0.25.1→0.25.2 are the two with the widest blast radius (async runtime and the project's own library), but both are minor bumps following semver.

RISK
🟢 LOW. Lockfile-only update with no source changes. All bumps are patch or minor semver. The main verification is that the project still compiles and tests pass against the new versions — particularly tokio 1.53.0 and octolib 0.25.2.

QUESTIONS
None — the change is self-explanatory and matches the commit message.

📎 Source

  • Cargo.lock — lockfile with 25 dependency version bumps, no source files touched
📂 Files changed (1 file, ~108 lines)
  • Cargo.lock — dependency lockfile; 25 crates bumped to latest patch/minor versions

{tokens:138020,cost:0.202627}

@donk8r
donk8r merged commit ff212ae into master Jul 17, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant