Skip to content

feat: add open command for opening directories as projects#70

Open
paschdan wants to merge 11 commits intogrinev:mainfrom
paschdan:feature/add-open-command-for-adding-projects
Open

feat: add open command for opening directories as projects#70
paschdan wants to merge 11 commits intogrinev:mainfrom
paschdan:feature/add-open-command-for-adding-projects

Conversation

@paschdan
Copy link
Copy Markdown

@paschdan paschdan commented Apr 1, 2026

Description of changes

  • New /open command: Adds a directory browser via Telegram inline keyboard, allowing users to navigate the local filesystem and select a folder as a project — without needing to know the exact path upfront.
  • switchToProject extraction: Refactors the project-switching state logic (clear session, reset pinned message, refresh keyboard) out of projects.ts into a shared src/bot/utils/switch-project.ts utility, eliminating duplication between /projects and /open.
  • file-tree utility: Adds src/bot/utils/file-tree.ts with pure functions for directory scanning, pagination (8 entries/page), hidden-directory filtering, tilde-based display paths, and i18n-aware subfolder counts.
  • Callback-data path encoding: Handles Telegram's 64-byte callback_data limit by transparently switching to compact indexed references (#0, #1, …) for long absolute paths.
  • i18n: All new user-facing strings added to all 6 locales (en, de, es, fr, ru, zh).

Closes issue (optional)

How it was tested

  • 73 new unit tests across 3 test files:
    • tests/bot/commands/open.test.ts — command, callback routing, busy guard, error paths, path encoding round-trips, stale index invalidation
    • tests/bot/utils/switch-project.test.ts — all state-clearing branches, keyboard rebuild, pinned message failure resilience
    • tests/bot/utils/file-tree.test.ts — real temp-directory integration tests for scanning, pagination, hidden-dir filtering, error codes
  • npm run build — clean
  • npm run lint — clean
  • npm test — all pass

Checklist

  • PR title follows Conventional Commits: feat: add /open command for browsing and adding project directories
  • This PR contains one logically complete change
  • Branch is rebased on the latest main
  • I ran npm run lint, npm run build, and npm test
  • If this PR is OS-sensitive, behavior/limitations for Linux/macOS/Windows are described

OS note: Directory scanning uses node:fs and node:path — works on all platforms. The pathToDisplayPath function uses path.sep for correct tilde substitution on both Unix and Windows. Hidden-directory filtering (.-prefixed) follows Unix convention; on Windows, system/hidden directories without a dot prefix will still appear in the listing.

@grinev grinev closed this Apr 4, 2026
@grinev grinev reopened this Apr 4, 2026
@grinev
Copy link
Copy Markdown
Owner

grinev commented Apr 4, 2026

@paschdan thanks for the PR! Only today I had time to look at it carefully and think about the feature as a whole.

In general, I see the value of /open. It can be useful in real life, especially when the user does not remember the exact path and wants to add a project quickly from Telegram.

At the same time, in the current form I am a bit uncomfortable with this feature from a security point of view. Right now it allows browsing almost any local directory and selecting it as a project. That means the bot can be used not only for known project folders, but also for system folders or other sensitive locations on the machine. Even if the bot is limited to one Telegram user, this still feels too broad by default.

I think this feature could work much better if we limit it to a configurable allowlist of roots.

My suggestion:

  • Add a new env config, for example PROJECT_BROWSER_ROOTS
  • If it is not set, use only the user's home directory as the default root
  • Parse it as a list of allowed root paths
  • In /open, first show these allowed roots
  • After the user enters one root, allow browsing only inside that root
  • Do not allow going above the selected root
  • Normalize paths before checks (resolve, and ideally realpath)
  • On Windows, compare paths case-insensitively
  • If someone wants more locations, they can add them manually in .env

For me this would be a much safer balance:

  • the feature still exists and stays useful
  • default behavior is conservative
  • broader access requires explicit local configuration

Below are also some technical remarks from the review:

  1. src/bot/index.ts
    The global clearOpenPathIndex() cleanup is called for any handled inline cancel. Because /open uses a shared in-memory path index for long callback data, a stale cancel from another menu can break the active /open menu.
  2. src/project/manager.ts
    getProjectByWorktree() uses exact string equality, but on Windows path handling is effectively case-insensitive in other parts of the code. This can make project selection fail for paths that differ only by letter case.
  3. src/bot/commands/open.ts
    upsertSessionDirectory() is called before the full project switch finishes successfully. If a later step fails, the directory may still stay in the cached project list.
  4. src/bot/commands/open.ts / src/bot/utils/file-tree.ts
    Page numbers are not normalized strongly enough. If directory contents change or callback data is stale/crafted, the UI can end up showing an invalid page like (4/2).

Overall, I think the implementation work is solid, and I especially like the extraction of switchToProject() and the handling of Telegram callback length limits. My main concern is the product/security scope of /open in the current unrestricted form.

@paschdan
Copy link
Copy Markdown
Author

paschdan commented Apr 7, 2026

  1. src/bot/index.ts
    The global clearOpenPathIndex() cleanup is called for any handled inline cancel. Because /open uses a shared in-memory path index for long callback data, a stale cancel from another menu can break the active /open menu.

@grinev
I thought this was by design: The interactionManager holds a single state — calling start() clears any previous one (line 59–61). Only one inline menu can be active at a time. If /projects is open and the user opens /open, the projects menu becomes stale, and ensureActiveInlineMenu will reject any callbacks from it. So the scenario of "cancel from another menu wipes the active /open index" cannot happen — by the time a cancel fires, it's guaranteed to belong to the currently active menu.

@grinev
Copy link
Copy Markdown
Owner

grinev commented Apr 7, 2026

  1. src/bot/index.ts
    The global clearOpenPathIndex() cleanup is called for any handled inline cancel. Because /open uses a shared in-memory path index for long callback data, a stale cancel from another menu can break the active /open menu.

@grinev I thought this was by design: The interactionManager holds a single state — calling start() clears any previous one (line 59–61). Only one inline menu can be active at a time. If /projects is open and the user opens /open, the projects menu becomes stale, and ensureActiveInlineMenu will reject any callbacks from it. So the scenario of "cancel from another menu wipes the active /open index" cannot happen — by the time a cancel fires, it's guaranteed to belong to the currently active menu.

@paschdan ok, you can skip this part

paschdan added 8 commits April 8, 2026 09:59
Extract the project-switching state logic (clear session, reset pinned
message, refresh keyboard) from projects.ts into a shared utility so it
can be reused by the upcoming /open command without duplication.
Add file-tree.ts with pure utilities for browsing the local filesystem:
directory scanning with pagination, hidden-dir filtering, tilde-based
display paths, entry labels, and tree headers with i18n-aware counts.
Add a new /open bot command that lets users browse the local filesystem
via inline keyboard navigation and select a directory as a project.
Includes callback-data path encoding to stay within Telegram's 64-byte
limit, pagination, and i18n strings for all 6 supported locales.
Stale or crafted callback data could produce out-of-bounds page
indicators like (4/2). Clamp the page to the valid range and return
the normalized page in the scan result so all callers use the safe
value.
Move the session directory cache write to after getProjectByWorktree
and switchToProject complete successfully, so a failed switch does not
leave a stale entry in the cached project list.
…ndows

Reuse the existing worktreeKey() helper so getProjectByWorktree
compares paths case-insensitively on Windows, matching the behavior
already used in getProjects() for merging API and cached projects.
…wlist

Add OPEN_BROWSER_ROOTS env var (comma-separated absolute paths) to limit
which directories the /open command can browse. Defaults to the user's
home directory when not set.

- New browser-roots.ts utility: root parsing, path normalization,
  isWithinAllowedRoot guard with case-insensitive matching on Windows
- Guard all navigation, pagination, and selection callbacks against
  paths outside allowed roots
- Show root-selection keyboard when multiple roots are configured
- Suppress the Up button when at an allowed root boundary
- Replace Home button with Back to roots for multi-root setups
- i18n strings for root selection and access-denied in all 6 locales
@paschdan paschdan force-pushed the feature/add-open-command-for-adding-projects branch from 88ef040 to b715e96 Compare April 8, 2026 08:02
@paschdan
Copy link
Copy Markdown
Author

paschdan commented Apr 8, 2026

@grinev Thanks for the thorough review! Here's what I've done to address your feedback:

Rebased onto latest main

the branch now includes all changes through v0.15.0, including #72. The resolveProjectAgent and keyboardManager.updateAgent additions from #72 were merged into the switchToProject refactor commit so the extracted utility stays in sync with upstream.

Technical remarks — addressed as individual fix commits:

  1. clearOpenPathIndex on any cancel (Point 1) — after investigating, this is safe by design. The interactionManager enforces a single active menu at a time (start() clears any previous state), so a cancel callback can only fire for the currently active menu. No code change needed.

  2. Page number not clamped (Point 4) — scanDirectory now clamps the page to the valid range and returns the normalized value in the result. Stale or crafted callbacks can no longer produce (4/2) style indicators.

  3. upsertSessionDirectory called too early (Point 3) — moved the cache write to after getProjectByWorktree + switchToProject succeed. If either step fails, no stale entry is left behind.

  4. Case-insensitive getProjectByWorktree on Windows (Point 2) — now reuses the existing worktreeKey() helper that getProjects() already uses for merging, so path comparison is case-insensitive on win32.

Main concern — browsing scope:

Implemented the allowlist via OPEN_BROWSER_ROOTS env var (comma-separated absolute paths, defaults to home directory):

  • New browser-roots.ts utility with isWithinAllowedRoot / isAllowedRoot guards
  • All navigation, pagination, and selection callbacks are validated against the allowlist
  • "Up" button is suppressed at root boundaries
  • Multi-root mode shows a root-selection keyboard; single-root mode navigates directly
  • Case-insensitive path comparison on Windows
  • i18n strings for root selection and access-denied in all 6 locales

paschdan added 2 commits April 8, 2026 10:22
Expand leading ~ or ~/ to the user's home directory when parsing
OPEN_BROWSER_ROOTS, so entries like ~/projects resolve correctly.
Updated .env.example to show tilde-based examples.
@grinev
Copy link
Copy Markdown
Owner

grinev commented Apr 8, 2026

@paschdan thanks for your work! I will look and give feedback today/tomorrow

@grinev
Copy link
Copy Markdown
Owner

grinev commented Apr 8, 2026

@paschdan pls add command description to readme

@grinev
Copy link
Copy Markdown
Owner

grinev commented Apr 8, 2026

I tested the updated branch, and right now opening a project via /open does not work.

The problem is in the selection flow order:

  • selectDirectory() now calls getProjectByWorktree(directory) first
  • and only after a successful switch it calls upsertSessionDirectory(directory, ...)

Because of that, selecting a new directory fails if this path is not already known by OpenCode or present in the session-directory cache.

So the previous change fixed the stale-cache issue, but it also broke the main happy-path for /open: adding and opening a new project directory.

Example from logs:
Error: Project with worktree c:\users\ruslan\Documents\Visual Studio 2022 not found

This means /open currently cannot actually open a new directory as a project in many normal cases.
I think this flow needs to be adjusted so that /open can still register/select a new directory safely, without requiring it to already exist in the known project list.

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.

2 participants