From 8407a949fcfbb42416fcd5bba32142b3c48e26bc Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 15:11:17 +0100 Subject: [PATCH 01/13] ci, install: update workflows, packaging scripts, and installer suite Nightly/release workflows: matrix tweaks, artifact path fixes. Package-macos: streamline dmg build, codesigning improvements. Install scripts: rewrite install.sh and install-nightly.sh with robust error handling, add uninstall.sh, overhaul update.sh. --- .gitea/workflows/nightly.yml | 14 +- .gitea/workflows/release.yml | 17 ++- ci/package-macos.sh | 27 ++-- ci/verify-version.sh | 2 +- install-nightly.sh | 66 ++++++++ install.sh | 66 ++++++++ uninstall.sh | 41 ++++- update.sh | 283 ++++++++++++++++++++++------------- 8 files changed, 396 insertions(+), 120 deletions(-) diff --git a/.gitea/workflows/nightly.yml b/.gitea/workflows/nightly.yml index 7369585..47d1546 100644 --- a/.gitea/workflows/nightly.yml +++ b/.gitea/workflows/nightly.yml @@ -52,7 +52,15 @@ jobs: - name: Deploy to builds.skaldagent.net run: | cd "${GITHUB_WORKSPACE:-.}" - mkdir -p /var/www/builds.skaldagent.net/nightly - cp dist/*.tar.gz /var/www/builds.skaldagent.net/nightly/ + DEST=/var/www/builds.skaldagent.net/nightly + mkdir -p "$DEST" + # Nightly reuses a fixed filename, so publish atomically: copy to a + # temp name on the same filesystem, then rename over the target. A + # concurrent download never sees a half-written tarball. + for f in dist/*.tar.gz; do + name="$(basename "$f")" + cp "$f" "$DEST/.$name.tmp" + mv -f "$DEST/.$name.tmp" "$DEST/$name" + done echo "[nightly] Deployed:" - ls -lh /var/www/builds.skaldagent.net/nightly/ + ls -lh "$DEST/" diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index a4ce55a..e2f623d 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -85,11 +85,22 @@ jobs: VERSION="${{ steps.extract-version.outputs.version }}" TARGET="/var/www/builds.skaldagent.net/releases/${VERSION}" mkdir -p "$TARGET" - cp dist/*.tar.gz "$TARGET/" + # Publish each tarball atomically (temp name + rename) so a client can + # never fetch a half-written file. + for f in dist/*.tar.gz; do + name="$(basename "$f")" + cp "$f" "$TARGET/.$name.tmp" + mv -f "$TARGET/.$name.tmp" "$TARGET/$name" + done echo "[release] Deployed $VERSION:" ls -lh "$TARGET/" - name: Update latest version pointer run: | - echo "${{ steps.extract-version.outputs.version }}" > /var/www/builds.skaldagent.net/releases/LATEST - echo "[release] Updated releases/LATEST → ${{ steps.extract-version.outputs.version }}" + VERSION="${{ steps.extract-version.outputs.version }}" + DEST=/var/www/builds.skaldagent.net/releases + # Flip LATEST atomically — install.sh/update.sh read it to decide + # whether to upgrade, so it must never be observed empty or partial. + printf '%s\n' "$VERSION" > "$DEST/.LATEST.tmp" + mv -f "$DEST/.LATEST.tmp" "$DEST/LATEST" + echo "[release] Updated releases/LATEST → $VERSION" diff --git a/ci/package-macos.sh b/ci/package-macos.sh index e943bbc..6d248e2 100755 --- a/ci/package-macos.sh +++ b/ci/package-macos.sh @@ -90,19 +90,26 @@ fi echo "[package-macos] Uploading to ${REMOTE_HOST}..." if [ "$MODE" = "release" ]; then - # Create remote directory and copy tarball - ssh "$REMOTE_HOST" "mkdir -p ${REMOTE_BASE}/releases/${VERSION}" - scp dist/skald-circle-${VERSION}-darwin-arm64.tar.gz \ - "${REMOTE_HOST}:${REMOTE_BASE}/releases/${VERSION}/" + TARBALL="skald-circle-${VERSION}-darwin-arm64.tar.gz" + RDIR="${REMOTE_BASE}/releases/${VERSION}" + ssh "$REMOTE_HOST" "mkdir -p ${RDIR}" - # Update LATEST pointer - echo "$VERSION" | ssh "$REMOTE_HOST" "cat > ${REMOTE_BASE}/releases/LATEST" + # Publish atomically: scp to a temp name, then rename over the target so a + # concurrent download never sees a half-transferred tarball. + scp "dist/${TARBALL}" "${REMOTE_HOST}:${RDIR}/.${TARBALL}.tmp" + ssh "$REMOTE_HOST" "mv -f ${RDIR}/.${TARBALL}.tmp ${RDIR}/${TARBALL}" + + # Flip LATEST atomically (write temp + rename) — clients read it to decide + # whether to upgrade, so it must never be observed empty or partial. + ssh "$REMOTE_HOST" "printf '%s\n' '${VERSION}' > ${REMOTE_BASE}/releases/.LATEST.tmp && mv -f ${REMOTE_BASE}/releases/.LATEST.tmp ${REMOTE_BASE}/releases/LATEST" echo "[package-macos] ✅ Release ${VERSION} deployed + LATEST updated." else - # Nightly — copy into nightly/ directory - ssh "$REMOTE_HOST" "mkdir -p ${REMOTE_BASE}/nightly" - scp dist/skald-circle-nightly-darwin-arm64.tar.gz \ - "${REMOTE_HOST}:${REMOTE_BASE}/nightly/" + # Nightly — atomic publish into nightly/ (fixed filename, reused each run). + TARBALL="skald-circle-nightly-darwin-arm64.tar.gz" + NDIR="${REMOTE_BASE}/nightly" + ssh "$REMOTE_HOST" "mkdir -p ${NDIR}" + scp "dist/${TARBALL}" "${REMOTE_HOST}:${NDIR}/.${TARBALL}.tmp" + ssh "$REMOTE_HOST" "mv -f ${NDIR}/.${TARBALL}.tmp ${NDIR}/${TARBALL}" echo "[package-macos] ✅ Nightly deployed." fi diff --git a/ci/verify-version.sh b/ci/verify-version.sh index e099e8e..d3c6061 100755 --- a/ci/verify-version.sh +++ b/ci/verify-version.sh @@ -5,7 +5,7 @@ # branch. Runs in the repo root after checkout. # # Usage: -# ./scripts/verify-version.sh \ +# ./ci/verify-version.sh \ # --builds-dir /var/www/builds.skaldagent.net # # Exit codes: diff --git a/install-nightly.sh b/install-nightly.sh index 98dbcca..7ef694e 100755 --- a/install-nightly.sh +++ b/install-nightly.sh @@ -84,6 +84,49 @@ prompt_yes_no() { esac } +# Like prompt_yes_no but defaults to NO — for destructive confirmations. +prompt_yes_no_default_no() { + local msg="${1:-Continue? [y/N]}" + local ans="" + if [ "$IS_INTERACTIVE" = true ]; then + printf "%s" "$msg" >&2 + read -r ans || ans="" + elif (: /dev/null; then + printf "%s" "$msg" >/dev/tty + IFS= read -r ans /dev/null 2>&1 && \ + systemctl --user stop skald-circle.service 2>/dev/null || true + ;; + darwin) + command -v launchctl >/dev/null 2>&1 && \ + launchctl unload "$HOME/Library/LaunchAgents/com.skald.circle.plist" 2>/dev/null || true + ;; + esac + if command -v pgrep >/dev/null 2>&1; then + local i=0 + while pgrep -f "${INSTALL_DIR}/bin/skald" >/dev/null 2>&1; do + i=$((i + 1)) + [ "$i" -gt 20 ] && break + sleep 1 + done + else + sleep 2 + fi +} + # ── Docker install helper ───────────────────────────────────────────────────── install_docker() { if [ "$OS" = "linux" ]; then @@ -237,6 +280,26 @@ ask_install_docker # ── Optional dependency check ───────────────────────────────────────────────── check_optional_deps +# ── Existing installation? ──────────────────────────────────────────────────── +# This installer targets a fresh install; the supported in-place upgrade path is +# update.sh (keeps your data, restarts the service safely). If an install is +# already here, point the user there and only reinstall over it on request. +if [ -x "$INSTALL_DIR/bin/skald" ]; then + echo "" + warn "An existing installation was found at ${INSTALL_DIR}." + echo " To upgrade in place (keeping your database and config), use:" + echo " ${INSTALL_DIR}/update.sh" + echo "" + if prompt_yes_no_default_no " Reinstall over it instead? Data is kept, the service restarts. [y/N] "; then + info "⏹️ Stopping the running instance before reinstalling …" + stop_existing_service + else + info "Aborted — run ${INSTALL_DIR}/update.sh to upgrade." + exit 0 + fi + echo "" +fi + # ── Download & extract ──────────────────────────────────────────────────────── info "↓ Downloading Skald Circle (${DISPLAY_VERSION}) …" mkdir -p "$INSTALL_DIR" @@ -354,6 +417,9 @@ elif [ "$OS" = "darwin" ]; then PLIST + # Idempotent: unload any previously-loaded agent first, so a re-install + # reloads the freshly written plist instead of failing on "already loaded". + launchctl unload "$PLIST" 2>/dev/null || true launchctl load "$PLIST" info "✔ Agent installed and started" diff --git a/install.sh b/install.sh index 01b8273..aee48c0 100755 --- a/install.sh +++ b/install.sh @@ -87,6 +87,49 @@ prompt_yes_no() { esac } +# Like prompt_yes_no but defaults to NO — for destructive confirmations. +prompt_yes_no_default_no() { + local msg="${1:-Continue? [y/N]}" + local ans="" + if [ "$IS_INTERACTIVE" = true ]; then + printf "%s" "$msg" >&2 + read -r ans || ans="" + elif (: /dev/null; then + printf "%s" "$msg" >/dev/tty + IFS= read -r ans /dev/null 2>&1 && \ + systemctl --user stop skald-circle.service 2>/dev/null || true + ;; + darwin) + command -v launchctl >/dev/null 2>&1 && \ + launchctl unload "$HOME/Library/LaunchAgents/com.skald.circle.plist" 2>/dev/null || true + ;; + esac + if command -v pgrep >/dev/null 2>&1; then + local i=0 + while pgrep -f "${INSTALL_DIR}/bin/skald" >/dev/null 2>&1; do + i=$((i + 1)) + [ "$i" -gt 20 ] && break + sleep 1 + done + else + sleep 2 + fi +} + # ── Docker install helper ───────────────────────────────────────────────────── install_docker() { if [ "$OS" = "linux" ]; then @@ -242,6 +285,26 @@ ask_install_docker # ── Optional dependency check ───────────────────────────────────────────────── check_optional_deps +# ── Existing installation? ──────────────────────────────────────────────────── +# This installer targets a fresh install; the supported in-place upgrade path is +# update.sh (keeps your data, restarts the service safely). If an install is +# already here, point the user there and only reinstall over it on request. +if [ -x "$INSTALL_DIR/bin/skald" ]; then + echo "" + warn "An existing installation was found at ${INSTALL_DIR}." + echo " To upgrade in place (keeping your database and config), use:" + echo " ${INSTALL_DIR}/update.sh" + echo "" + if prompt_yes_no_default_no " Reinstall over it instead? Data is kept, the service restarts. [y/N] "; then + info "⏹️ Stopping the running instance before reinstalling …" + stop_existing_service + else + info "Aborted — run ${INSTALL_DIR}/update.sh to upgrade." + exit 0 + fi + echo "" +fi + # ── Download & extract ──────────────────────────────────────────────────────── info "↓ Downloading Skald Circle ${VERSION} …" mkdir -p "$INSTALL_DIR" @@ -359,6 +422,9 @@ elif [ "$OS" = "darwin" ]; then PLIST + # Idempotent: unload any previously-loaded agent first, so a re-install + # reloads the freshly written plist instead of failing on "already loaded". + launchctl unload "$PLIST" 2>/dev/null || true launchctl load "$PLIST" info "✔ Agent installed and started" diff --git a/uninstall.sh b/uninstall.sh index ee3391a..34e134a 100644 --- a/uninstall.sh +++ b/uninstall.sh @@ -46,11 +46,25 @@ echo " This will permanently delete Skald Circle and all its data:" echo " ${INSTALL_DIR}" echo "" -if [ -t 0 ]; then - printf "%s " "Are you sure? Type 'yes' to continue: " +# Confirm before a destructive delete. Read from the terminal even when stdin is +# piped (curl | sh); if there's no terminal at all, require SKALD_YES=1 so an +# install is never wiped with no confirmation. +if [ "${SKALD_YES:-}" = "1" ]; then + : +elif [ -t 0 ]; then + printf "%s " "Are you sure? Type 'yes' to continue:" read -r CONFIRM [ "$CONFIRM" = "yes" ] || { echo "Aborted."; exit 0; } echo "" +elif (: /dev/null; then + printf "%s " "Are you sure? Type 'yes' to continue:" >/dev/tty + IFS= read -r CONFIRM /dev/null 2>&1 && docker info >/dev/null 2>&1; then + CONTAINERS="$(docker ps -aq --filter 'name=skald-' 2>/dev/null || true)" + if [ -n "$CONTAINERS" ]; then + info "🐳 Removing Skald Docker containers …" + # shellcheck disable=SC2086 # word-splitting is intentional (multiple IDs) + docker rm -f $CONTAINERS >/dev/null 2>&1 || true + fi + IMAGES="$(docker images -q skald-runtime 2>/dev/null || true)" + if [ -n "$IMAGES" ]; then + info "🐳 Removing Skald runtime image …" + # shellcheck disable=SC2086 + docker rmi -f $IMAGES >/dev/null 2>&1 || true + fi +elif command -v docker >/dev/null 2>&1; then + warn "Docker daemon not reachable — skipping container cleanup." + warn "Remove leftovers later with: docker rm -f \$(docker ps -aq --filter name=skald-)" +fi + # ── Remove installation directory ───────────────────────────────────────────── if [ -d "$INSTALL_DIR" ]; then info "🗑️ Removing installation directory …" diff --git a/update.sh b/update.sh index c0adbaa..2c1ec84 100755 --- a/update.sh +++ b/update.sh @@ -8,7 +8,20 @@ # whether to pull from the release or nightly channel. On release, checks # the remote LATEST version first and skips the download if already current. # -# Stops the service before extracting, then restarts it afterwards. +# Robustness notes (why the flow looks the way it does): +# * The tarball is downloaded and validated in a *staging* directory BEFORE +# the running service is touched — a broken download never takes the app +# down. +# * The service is stopped and we WAIT until the process is actually gone +# before extracting: overwriting a live binary in place fails with ETXTBSY +# (Linux) or on a running Mach-O (macOS), which would abort the update with +# the service left down. +# * The whole flow runs inside main(), invoked on the very last line, so the +# shell has parsed the entire script into memory before `tar` overwrites +# update.sh with its own new copy (the tarball ships this script). Without +# this, the shell would read garbage for the tail and never restart. +# * A trap restarts the service if the update fails after the stop, so a +# failed update never leaves the box down. set -eu @@ -28,7 +41,7 @@ if [ ! -f "$CHANNEL_FILE" ]; then exit 1 fi -CHANNEL="$(cat "$CHANNEL_FILE" | tr -d '[:space:]')" +CHANNEL="$(tr -d '[:space:]' < "$CHANNEL_FILE")" # ── Colours (if terminal) ───────────────────────────────────────────────────── if [ -t 1 ]; then @@ -74,46 +87,13 @@ esac command -v curl >/dev/null 2>&1 || { err "curl is required but not installed."; exit 1; } -# ── Determine download URL ──────────────────────────────────────────────────── BASE_URL="https://builds.skaldagent.net" -case "$CHANNEL" in - release) - LATEST="$(curl -fsSL "${BASE_URL}/releases/LATEST" | head -1 | tr -d '[:space:]')" - if [ -z "$LATEST" ]; then - err "Could not fetch latest release version from ${BASE_URL}/releases/LATEST" - exit 1 - fi - - VERSION_FILE="${INSTALL_DIR}/.release-version" - if [ -f "$VERSION_FILE" ]; then - CURRENT="$(cat "$VERSION_FILE" | tr -d '[:space:]')" - if [ "$CURRENT" = "$LATEST" ]; then - info "✔ Already up to date (${CURRENT})." - exit 0 - fi - info "🚀 Update available: ${CURRENT} → ${LATEST}" - else - info "🚀 Installing latest release: ${LATEST}" - fi - - VERSION="$LATEST" - TARBALL_URL="${BASE_URL}/releases/${VERSION}/skald-circle-${VERSION}-${OS}-${ARCH}.tar.gz" - DISPLAY_VERSION="$VERSION" - ;; - - nightly) - TARBALL_URL="${BASE_URL}/nightly/skald-circle-nightly-${OS}-${ARCH}.tar.gz" - DISPLAY_VERSION="nightly" - info "🚀 Updating to latest nightly build …" - ;; - - *) - err "Unknown channel in ${CHANNEL_FILE}: '${CHANNEL}'" - err "Expected 'release' or 'nightly'." - exit 1 - ;; -esac +# ── Global state (referenced by the EXIT trap) ──────────────────────────────── +TMP_TARBALL="" +STAGING="" +STOPPED=0 # set once the service has been stopped +STARTED=0 # set once it has been (re)started # ── Stop service ────────────────────────────────────────────────────────────── stop_service() { @@ -137,6 +117,31 @@ stop_service() { esac } +# ── Wait until the server process is actually gone ──────────────────────────── +# systemctl stop is synchronous, but launchctl unload kills the process group +# asynchronously — so we poll for the real binary before overwriting it. +wait_until_stopped() { + # Match the server binary by path prefix. Deliberately unanchored: the + # server runs with no args today, but we'd rather over-match (a harmless + # extra wait) than miss a still-running process and race the extraction. + # `skald-setup` only runs at first-run setup, never during an update. + pat="${INSTALL_DIR}/bin/skald" + if command -v pgrep >/dev/null 2>&1; then + i=0 + while pgrep -f "$pat" >/dev/null 2>&1; do + i=$((i + 1)) + if [ "$i" -gt 20 ]; then + warn "Server still running after ~20s; proceeding anyway." + return 0 + fi + sleep 1 + done + else + # No pgrep: give the service manager a moment to tear the process down. + sleep 3 + fi +} + # ── Start service ───────────────────────────────────────────────────────────── start_service() { case "$OS" in @@ -155,72 +160,148 @@ start_service() { esac } +# ── Cleanup + safety net ────────────────────────────────────────────────────── +# Runs on every exit. Removes temp files and, if the update died after the +# service was stopped but before it came back up, makes a best-effort restart so +# the box is never left down. +cleanup() { + [ -n "${TMP_TARBALL:-}" ] && rm -f "$TMP_TARBALL" 2>/dev/null || true + [ -n "${STAGING:-}" ] && rm -rf "$STAGING" 2>/dev/null || true + if [ "$STOPPED" = "1" ] && [ "$STARTED" != "1" ]; then + warn "Update failed after the service was stopped — attempting to restart it …" + start_service || true + fi +} +trap cleanup EXIT + # ── Main ────────────────────────────────────────────────────────────────────── -banner "╔══════════════════════════════════════════╗" -banner "║ Skald Circle — Updater (${DISPLAY_VERSION}) ║" -banner "╚══════════════════════════════════════════╝" -echo "" -echo " Channel : ${CHANNEL}" -echo " Platform : ${OS}/${ARCH}" -echo " Install : ${INSTALL_DIR}" -echo "" +main() { + # ── Resolve download URL + version (may exit early if already current) ───── + case "$CHANNEL" in + release) + LATEST="$(curl -fsSL "${BASE_URL}/releases/LATEST" | head -1 | tr -d '[:space:]')" + if [ -z "$LATEST" ]; then + err "Could not fetch latest release version from ${BASE_URL}/releases/LATEST" + exit 1 + fi -stop_service + VERSION_FILE="${INSTALL_DIR}/.release-version" + if [ -f "$VERSION_FILE" ]; then + CURRENT="$(tr -d '[:space:]' < "$VERSION_FILE")" + if [ "$CURRENT" = "$LATEST" ]; then + info "✔ Already up to date (${CURRENT})." + exit 0 + fi + info "🚀 Update available: ${CURRENT} → ${LATEST}" + else + info "🚀 Installing latest release: ${LATEST}" + fi -# Download & extract -TMP_TARBALL="$(mktemp -t skald-update.XXXXXX.tar.gz)" -trap 'rm -f "$TMP_TARBALL"' EXIT + VERSION="$LATEST" + TARBALL_URL="${BASE_URL}/releases/${VERSION}/skald-circle-${VERSION}-${OS}-${ARCH}.tar.gz" + DISPLAY_VERSION="$VERSION" + ;; -info "↓ Downloading Skald Circle (${DISPLAY_VERSION}) …" -curl -fsSL -o "$TMP_TARBALL" "$TARBALL_URL" + nightly) + TARBALL_URL="${BASE_URL}/nightly/skald-circle-nightly-${OS}-${ARCH}.tar.gz" + DISPLAY_VERSION="nightly" + info "🚀 Updating to latest nightly build …" + ;; -info "📦 Extracting …" -tar xzf "$TMP_TARBALL" -C "$INSTALL_DIR" --strip-components=1 + *) + err "Unknown channel in ${CHANNEL_FILE}: '${CHANNEL}'" + err "Expected 'release' or 'nightly'." + exit 1 + ;; + esac -if [ ! -x "$INSTALL_DIR/bin/skald" ]; then - err "Extraction failed — skald binary not found." - exit 1 -fi - -# Update version file for release channel -if [ "$CHANNEL" = "release" ]; then - echo "$VERSION" > "$INSTALL_DIR/.release-version" -fi - -# Rebuild Python venv (best effort — new deps may have appeared) -info "🔧 Rebuilding Python virtual environment …" -VENV_DIR="${INSTALL_DIR}/.venv" -REQUIREMENTS="${INSTALL_DIR}/requirements.txt" - -rm -rf "$VENV_DIR" -if command -v uv >/dev/null 2>&1; then - uv venv --seed "$VENV_DIR" && uv pip install -r "$REQUIREMENTS" \ - && info "✔ Python venv ready (uv)" \ - || warn "Python venv setup failed — Python MCP servers will be unavailable." -elif command -v python3 >/dev/null 2>&1; then - python3 -m venv "$VENV_DIR" && "$VENV_DIR/bin/pip" install -r "$REQUIREMENTS" \ - && info "✔ Python venv ready (pip)" \ - || warn "Python venv setup failed — Python MCP servers will be unavailable." -else - warn "python3 not found — Python MCP servers will be unavailable." -fi - -start_service - -# ── Hint about optional deps ────────────────────────────────────────────────── -if [ -f "${INSTALL_DIR}/requirements-optional.txt" ]; then - info "💡 Optional GPU/ML dependencies available:" - info " ${INSTALL_DIR}/requirements-optional.txt" - echo " Install them manually if you use the Orpheus TTS plugin:" - echo " cd ${INSTALL_DIR} && .venv/bin/pip install -r requirements-optional.txt" + banner "╔══════════════════════════════════════════╗" + banner "║ Skald Circle — Updater (${DISPLAY_VERSION}) ║" + banner "╚══════════════════════════════════════════╝" + echo "" + echo " Channel : ${CHANNEL}" + echo " Platform : ${OS}/${ARCH}" + echo " Install : ${INSTALL_DIR}" echo "" -fi -# ── Done ────────────────────────────────────────────────────────────────────── -echo "" -info "✅ Skald Circle updated to ${DISPLAY_VERSION}!" -echo "" -echo " Status: $( [ "$OS" = "linux" ] && echo "systemctl --user status skald-circle" || echo "launchctl list com.skald.circle" )" -echo " Logs: $( [ "$OS" = "linux" ] && echo "journalctl --user -u skald-circle -f" || echo "tail -f ${INSTALL_DIR}/logs/stdout.log" )" -echo " Update: ${INSTALL_DIR}/update.sh" -echo "" + # ── Download + validate BEFORE touching the running service ──────────────── + # A broken download or a bad archive must never take the app down: we only + # stop the service once we hold a known-good tarball. + TMP_TARBALL="$(mktemp -t skald-update.XXXXXX.tar.gz)" + + info "↓ Downloading Skald Circle (${DISPLAY_VERSION}) …" + curl -fsSL -o "$TMP_TARBALL" "$TARBALL_URL" + + info "🔎 Verifying archive …" + STAGING="$(mktemp -d -t skald-update-staging.XXXXXX)" + tar xzf "$TMP_TARBALL" -C "$STAGING" --strip-components=1 + if [ ! -x "$STAGING/bin/skald" ]; then + err "Downloaded archive is invalid — skald binary not found." + err "The running server was left untouched." + exit 1 + fi + + # ── Stop the service and wait for the process to actually exit ───────────── + stop_service + STOPPED=1 + wait_until_stopped + + # ── Install (binary is no longer busy) ───────────────────────────────────── + info "📦 Installing update …" + tar xzf "$TMP_TARBALL" -C "$INSTALL_DIR" --strip-components=1 + + if [ ! -x "$INSTALL_DIR/bin/skald" ]; then + err "Extraction failed — skald binary not found." + exit 1 + fi + + # Update version file for release channel + if [ "$CHANNEL" = "release" ]; then + echo "$VERSION" > "$INSTALL_DIR/.release-version" + fi + + # ── Rebuild Python venv (best effort — new deps may have appeared) ───────── + info "🔧 Rebuilding Python virtual environment …" + VENV_DIR="${INSTALL_DIR}/.venv" + REQUIREMENTS="${INSTALL_DIR}/requirements.txt" + + rm -rf "$VENV_DIR" + if command -v uv >/dev/null 2>&1; then + uv venv --seed "$VENV_DIR" && uv pip install -r "$REQUIREMENTS" \ + && info "✔ Python venv ready (uv)" \ + || warn "Python venv setup failed — Python MCP servers will be unavailable." + elif command -v python3 >/dev/null 2>&1; then + python3 -m venv "$VENV_DIR" && "$VENV_DIR/bin/pip" install -r "$REQUIREMENTS" \ + && info "✔ Python venv ready (pip)" \ + || warn "Python venv setup failed — Python MCP servers will be unavailable." + else + warn "python3 not found — Python MCP servers will be unavailable." + fi + + # ── Restart ──────────────────────────────────────────────────────────────── + start_service + STARTED=1 + + # ── Hint about optional deps ─────────────────────────────────────────────── + if [ -f "${INSTALL_DIR}/requirements-optional.txt" ]; then + info "💡 Optional GPU/ML dependencies available:" + info " ${INSTALL_DIR}/requirements-optional.txt" + echo " Install them manually if you use the Orpheus TTS plugin:" + echo " cd ${INSTALL_DIR} && .venv/bin/pip install -r requirements-optional.txt" + echo "" + fi + + # ── Done ─────────────────────────────────────────────────────────────────── + echo "" + info "✅ Skald Circle updated to ${DISPLAY_VERSION}!" + echo "" + echo " Status: $( [ "$OS" = "linux" ] && echo "systemctl --user status skald-circle" || echo "launchctl list com.skald.circle" )" + echo " Logs: $( [ "$OS" = "linux" ] && echo "journalctl --user -u skald-circle -f" || echo "tail -f ${INSTALL_DIR}/logs/stdout.log" )" + echo " Update: ${INSTALL_DIR}/update.sh" + echo "" +} + +# Invoked on the very last line: by the time `tar` overwrites this file on disk +# (the tarball ships update.sh), the shell has already parsed the whole script, +# so the restart tail always runs. Do not add executable code below this line. +main "$@" From f34f800e5cb8e4b0acd2a9c31d27f35313d50847 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 18:35:13 +0100 Subject: [PATCH 02/13] projects: file explorer, ws improvements, i18n, and fs routing Add project-files component with tree navigation. Extend UserFs with shared-folder resolution. Wire API routes for file browsing. Improve WS session lifecycle and project-board layout. Add i18n keys for projects and inbox across all locales. --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/core-api/src/user_fs.rs | 26 ++ src/frontend/api/files.rs | 123 ++++++- src/frontend/api/mod.rs | 3 + src/frontend/api/projects.rs | 12 +- src/frontend/api/ws.rs | 16 +- web/components/agent-inbox.js | 8 +- web/components/projects/index.js | 37 ++- web/components/projects/project-board.js | 62 +++- web/components/projects/project-files.js | 400 +++++++++++++++++++++++ web/components/sidebar.js | 7 +- web/i18n/en.js | 21 +- web/i18n/fr.js | 21 +- web/i18n/it.js | 21 +- web/lib/chat-session.js | 12 + 16 files changed, 733 insertions(+), 40 deletions(-) create mode 100644 web/components/projects/project-files.js diff --git a/Cargo.lock b/Cargo.lock index 700a230..56aefb5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4172,7 +4172,7 @@ checksum = "8ee5873ec9cce0195efcb7a4e9507a04cd49aec9c83d0389df45b1ef7ba2e649" [[package]] name = "skald" -version = "0.1.0" +version = "0.1.1" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index c9d599a..f9a96d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ resolver = "2" [package] name = "skald" -version = "0.1.0" +version = "0.1.1" edition = "2024" [features] diff --git a/crates/core-api/src/user_fs.rs b/crates/core-api/src/user_fs.rs index cf72825..d8e8cd4 100644 --- a/crates/core-api/src/user_fs.rs +++ b/crates/core-api/src/user_fs.rs @@ -108,6 +108,32 @@ impl UserFs { .find(|m| m.owner_username == owner_username && m.slug == slug) } + /// Whether the user may **write** at this agent path: their home → always; + /// a shared-folder or project mount → the membership's `can_write` flag; + /// `docs/…` → never (read-only). A `shared/`/`projects/` mount the user is + /// not a member of → false (fail-closed, same as the read side). Purely + /// lexical: memory paths never reach here (classified earlier). + pub fn can_write_to(&self, agent_path: &str) -> bool { + let stripped = strip_home_prefix(agent_path); + let mut parts = stripped.splitn(2, ['/', '\\']); + match parts.next() { + Some("shared") => { + let rest = parts.next().unwrap_or(""); + let name = rest.splitn(2, ['/', '\\']).next().unwrap_or(""); + self.shared_mount(name).map(|m| m.can_write).unwrap_or(false) + } + Some("projects") => { + let rest = parts.next().unwrap_or(""); + let mut seg = rest.splitn(3, ['/', '\\']); + let owner = seg.next().unwrap_or(""); + let slug = seg.next().unwrap_or(""); + self.project_mount(owner, slug).map(|m| m.can_write).unwrap_or(false) + } + Some("docs") => false, + _ => true, + } + } + /// The bind mounts for `docker create`: `(host, container, writable)`, home first. pub fn mounts(&self) -> Vec<(PathBuf, PathBuf, bool)> { let mut out = vec![(self.home_host.clone(), self.container_home.clone(), true)]; diff --git a/src/frontend/api/files.rs b/src/frontend/api/files.rs index ff4d350..79424f4 100644 --- a/src/frontend/api/files.rs +++ b/src/frontend/api/files.rs @@ -2,6 +2,7 @@ use std::path::Path; use axum::{ Extension, Json, + body::Bytes, extract::{Query, State}, http::{HeaderValue, StatusCode, header}, response::{IntoResponse, Response}, @@ -9,6 +10,7 @@ use axum::{ use serde::{Deserialize, Serialize}; use std::sync::Arc; +use core_api::user_fs::UserFs; use skald_core::skald::Skald; use skald_core::latex::CompileError; use skald_core::tools::fs as fs_tools; @@ -16,12 +18,80 @@ use super::ApiError; use super::guard::AuthUser; use super::require_context; +/// Upload body cap for `POST /api/file/upload` (same budget as chat attachments). +pub const MAX_UPLOAD_BYTES: usize = 256 * 1024 * 1024; + #[derive(Serialize)] pub struct FileEntry { pub path: String, pub name: String, } +/// One row of a directory listing: name + agent path (round-trips through +/// `/api/file`) + the metadata the explorer table shows. `size` is files-only; +/// timestamps are RFC-3339 UTC (`None` when the filesystem can't provide them, +/// e.g. no birth-time support) and formatted client-side. +#[derive(Serialize)] +pub struct DirEntry { + pub name: String, + pub path: String, + pub is_dir: bool, + pub size: Option, + pub created_at: Option, + pub modified_at: Option, +} + +fn fmt_ts(t: std::time::SystemTime) -> String { + chrono::DateTime::::from(t).to_rfc3339() +} + +/// Reject a write when the caller's mount for this path is read-only +/// (a shared-folder / project membership without `can_write`, or the docs +/// tree). The container bind mount is the physical gate for in-container +/// writes; the host-side HTTP API needs its own check. +fn require_write(fs: &UserFs, agent: &str) -> Result<(), ApiError> { + if fs.can_write_to(agent) { + Ok(()) + } else { + Err(ApiError::forbidden(format!("read-only: {agent}"))) + } +} + +/// GET /api/files/dir?path=… — the immediate children of a directory (dirs +/// first, then name), resolved and scoped exactly like `GET /api/file`. +pub async fn list_dir( + State(state): State>, + Extension(auth): Extension, + Query(q): Query, +) -> Result>, ApiError> { + let ctx = require_context(&state, &auth.user_id).await?; + let (abs, agent) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &q.path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; + if !abs.is_dir() { + return Err(ApiError::bad_request(format!("not a directory: {agent}"))); + } + let mut entries: Vec = Vec::new(); + for entry in std::fs::read_dir(&abs)? { + let entry = entry?; + let name = entry.file_name().to_string_lossy().to_string(); + let md = entry.metadata().ok(); + let is_dir = md.as_ref().is_some_and(|m| m.is_dir()); + entries.push(DirEntry { + path: format!("{agent}/{name}"), + name, + is_dir, + size: md.as_ref().filter(|m| m.is_file()).map(|m| m.len()), + created_at: md.as_ref().and_then(|m| m.created().ok()).map(fmt_ts), + modified_at: md.as_ref().and_then(|m| m.modified().ok()).map(fmt_ts), + }); + } + entries.sort_by(|a, b| { + b.is_dir.cmp(&a.is_dir) + .then_with(|| a.name.to_lowercase().cmp(&b.name.to_lowercase())) + }); + Ok(Json(entries)) +} + pub async fn list_files( State(state): State>, Extension(auth): Extension, @@ -231,6 +301,9 @@ pub struct SavePayload { #[derive(Deserialize)] pub struct CreatePayload { pub path: String, + /// When `true`, create a directory instead of an empty file. + #[serde(default)] + pub dir: bool, } pub async fn create_file( @@ -239,15 +312,45 @@ pub async fn create_file( Json(body): Json, ) -> Result { let ctx = require_context(&state, &auth.user_id).await?; - let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &body.path) + let fs = ctx.fs.load(); + let (abs, display) = fs_tools::resolve_view_path(fs.as_ref(), &body.path) .map_err(|e| ApiError::bad_request(e.to_string()))?; + require_write(&fs, &display)?; if abs.exists() { return Err(anyhow::anyhow!("File already exists: {display}").into()); } + if body.dir { + std::fs::create_dir_all(&abs)?; + } else { + if let Some(parent) = abs.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(&abs, "")?; + } + Ok(StatusCode::CREATED) +} + +/// POST /api/file/upload?path=… — raw request-body bytes written to `path` +/// (create or replace), for binary uploads from the project explorer. The +/// route caps the body at [`MAX_UPLOAD_BYTES`]; parent dirs are created. +pub async fn upload_file( + State(state): State>, + Extension(auth): Extension, + Query(q): Query, + body: Bytes, +) -> Result { + let ctx = require_context(&state, &auth.user_id).await?; + let fs = ctx.fs.load(); + let (abs, display) = fs_tools::resolve_view_path(fs.as_ref(), &q.path) + .map_err(|e| ApiError::bad_request(e.to_string()))?; + require_write(&fs, &display)?; + if abs.is_dir() { + return Err(ApiError::bad_request(format!("is a directory: {display}"))); + } if let Some(parent) = abs.parent() { std::fs::create_dir_all(parent)?; } - std::fs::write(&abs, "")?; + std::fs::write(&abs, &body)?; Ok(StatusCode::CREATED) } @@ -257,8 +360,10 @@ pub async fn save_file( Json(body): Json, ) -> Result { let ctx = require_context(&state, &auth.user_id).await?; - let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &body.path) + let fs = ctx.fs.load(); + let (abs, display) = fs_tools::resolve_view_path(fs.as_ref(), &body.path) .map_err(|e| ApiError::bad_request(e.to_string()))?; + require_write(&fs, &display)?; if !abs.exists() { return Err(anyhow::anyhow!("File not found: {display}").into()); } @@ -283,6 +388,8 @@ pub async fn rename_file( .map_err(|e| ApiError::bad_request(e.to_string()))?; let (new_abs, new_disp) = fs_tools::resolve_view_path(fs.as_ref(), &body.new_path) .map_err(|e| ApiError::bad_request(e.to_string()))?; + require_write(&fs, &old_disp)?; + require_write(&fs, &new_disp)?; if !old_abs.exists() { return Err(anyhow::anyhow!("File not found: {old_disp}").into()); } @@ -302,12 +409,18 @@ pub async fn delete_file( Query(q): Query, ) -> Result { let ctx = require_context(&state, &auth.user_id).await?; - let (abs, display) = fs_tools::resolve_view_path(ctx.fs.load().as_ref(), &q.path) + let fs = ctx.fs.load(); + let (abs, display) = fs_tools::resolve_view_path(fs.as_ref(), &q.path) .map_err(|e| ApiError::bad_request(e.to_string()))?; + require_write(&fs, &display)?; if !abs.exists() { return Err(anyhow::anyhow!("File not found: {display}").into()); } - std::fs::remove_file(&abs)?; + if abs.is_dir() { + std::fs::remove_dir_all(&abs)?; + } else { + std::fs::remove_file(&abs)?; + } Ok(StatusCode::NO_CONTENT) } diff --git a/src/frontend/api/mod.rs b/src/frontend/api/mod.rs index b5b22e9..589dadc 100644 --- a/src/frontend/api/mod.rs +++ b/src/frontend/api/mod.rs @@ -203,8 +203,11 @@ pub fn router() -> Router> { .route("/mcp-media/{file}", get(mcp_media::get_media)) // Files .route("/files", get(files::list_files)) + .route("/files/dir", get(files::list_dir)) .route("/file", get(files::get_file)) .route("/file", post(files::create_file)) + .route("/file/upload", post(files::upload_file) + .layer(DefaultBodyLimit::max(files::MAX_UPLOAD_BYTES))) .route("/file", put(files::save_file)) .route("/file", patch(files::rename_file)) .route("/file", delete(files::delete_file)) diff --git a/src/frontend/api/projects.rs b/src/frontend/api/projects.rs index 66f8667..8227836 100644 --- a/src/frontend/api/projects.rs +++ b/src/frontend/api/projects.rs @@ -57,6 +57,9 @@ pub struct ProjectDetail { pub owner_name: String, pub is_owner: bool, pub can_write: bool, + /// The agent path of the project folder (`projects/{owner_username}/{slug}`) — + /// the explorer's root; round-trips through `/api/file*` endpoints. + pub root_path: String, pub created_at: String, pub updated_at: String, pub members: Vec, @@ -152,15 +155,22 @@ async fn remount(skald: &Skald, user_id: &str) { async fn detail(skald: &Skald, project: Project, caller: &str, can_write: bool) -> Result { let members = project_members::members(skald.db(), project.id).await?; let owner_name = user_label(skald, &project.owner_user_id).await; + // The agent path keys on the owner's *username* (the mount segment), which + // `owner_name` may not be (it's `display_name || username`). + let owner_username = match users::get(skald.db(), &project.owner_user_id).await { + Ok(Some(u)) => u.username, + _ => project.owner_user_id.clone(), + }; Ok(ProjectDetail { is_owner: project.owner_user_id == caller, owner_name, id: project.id, name: project.name, - slug: project.slug, + slug: project.slug.clone(), description: project.description, owner_user_id: project.owner_user_id, can_write, + root_path: format!("projects/{owner_username}/{}", project.slug), created_at: project.created_at, updated_at: project.updated_at, members: members.into_iter().map(Into::into).collect(), diff --git a/src/frontend/api/ws.rs b/src/frontend/api/ws.rs index 75029ff..5d162e2 100644 --- a/src/frontend/api/ws.rs +++ b/src/frontend/api/ws.rs @@ -428,10 +428,20 @@ async fn handle_socket(mut socket: WebSocket, skald: Arc, source: String, match event { Ok(ge) => { // Forward events for this connection's source. - // ApprovalResolved is forwarded regardless of source so the - // copilot can react to approvals resolved from other clients. + // The inbox lifecycle events (approval/clarification/ + // elicitation requested+resolved) are forwarded regardless + // of source: they carry no content — just ids — and let the + // sidebar badge and inbox pages refresh live when any of + // this user's sessions (chat, cron, background) raises or + // settles a pending item. let forward = ge.source.as_deref() == Some(source.as_str()) - || matches!(ge.event, ServerEvent::ApprovalResolved { .. }); + || matches!(ge.event, + ServerEvent::ApprovalRequested { .. } + | ServerEvent::ApprovalResolved { .. } + | ServerEvent::ClarificationRequested { .. } + | ServerEvent::ClarificationResolved { .. } + | ServerEvent::ElicitationRequested { .. } + | ServerEvent::ElicitationResolved { .. }); if !forward { continue; } debug!(event_type = ge.event.type_name(), "sending event to client"); if socket.send(to_msg(&ge.event)).await.is_err() { diff --git a/web/components/agent-inbox.js b/web/components/agent-inbox.js index 62aba20..996a952 100644 --- a/web/components/agent-inbox.js +++ b/web/components/agent-inbox.js @@ -20,6 +20,10 @@ export class AgentInboxPage extends I18nMixin(InboxMixin(LightElement)) { connectedCallback() { super.connectedCallback(); + // Live refresh: the chat WS pushes `inbox-changed` when any of this user's + // sessions raises or settles a pending item — reload immediately if open. + this.__onInboxChanged = () => { if (this._open) this._loadInbox(); }; + window.addEventListener('inbox-changed', this.__onInboxChanged); window.addEventListener('llm-page-change', (e) => { this._open = e.detail.page === 'inbox'; this.style.display = this._open ? 'flex' : 'none'; @@ -35,11 +39,13 @@ export class AgentInboxPage extends I18nMixin(InboxMixin(LightElement)) { disconnectedCallback() { super.disconnectedCallback(); this._stopPolling(); + window.removeEventListener('inbox-changed', this.__onInboxChanged); } _startPolling() { this._stopPolling(); - this._pollTimer = setInterval(() => this._loadInbox(), 8000); + // Fallback only — pushes via `inbox-changed` keep the page fresh. + this._pollTimer = setInterval(() => this._loadInbox(), 60000); } _stopPolling() { diff --git a/web/components/projects/index.js b/web/components/projects/index.js index a4891ab..96fc0cb 100644 --- a/web/components/projects/index.js +++ b/web/components/projects/index.js @@ -8,6 +8,7 @@ export class ProjectsPage extends LightElement { _open: { state: true }, _view: { state: true }, _projectId: { state: true }, + _tab: { state: true }, }; constructor() { @@ -15,6 +16,7 @@ export class ProjectsPage extends LightElement { this._open = false; this._view = 'list'; this._projectId = null; + this._tab = 'files'; } connectedCallback() { @@ -24,9 +26,25 @@ export class ProjectsPage extends LightElement { this._open = open; this.style.display = open ? 'flex' : 'none'; if (open) { - const { view, id } = this._parseHash(); + const { view, id, tab } = this._parseHash(); this._view = view; this._projectId = id; + this._tab = tab; + this._loadCurrent(); + } + }); + window.addEventListener('hashchange', () => { + // Back/forward (or manual edit) between board tabs: same project → just + // switch the tab, no reload; anything else → re-sync from the hash. + if (!this._open || !location.hash.startsWith('#projects')) return; + const { view, id, tab } = this._parseHash(); + if (view === 'board' && this._view === 'board' && id === this._projectId) { + this._tab = tab; + this.querySelector('project-board-section')?.setTab(tab); + } else { + this._view = view; + this._projectId = id; + this._tab = tab; this._loadCurrent(); } }); @@ -40,9 +58,10 @@ export class ProjectsPage extends LightElement { _parseHash() { const parts = location.hash.slice(1).split('/'); if (parts[0] === 'projects' && parts[1] && /^\d+$/.test(parts[1])) { - return { view: 'board', id: parseInt(parts[1], 10) }; + const tab = parts[2] === 'sharing' ? 'sharing' : 'files'; + return { view: 'board', id: parseInt(parts[1], 10), tab }; } - return { view: 'list', id: null }; + return { view: 'list', id: null, tab: 'files' }; } _loadCurrent() { @@ -50,7 +69,7 @@ export class ProjectsPage extends LightElement { if (this._view === 'list') { this.querySelector('project-list-section')?.load(); } else { - this.querySelector('project-board-section')?.load(this._projectId); + this.querySelector('project-board-section')?.load(this._projectId, this._tab); } }); } @@ -58,9 +77,10 @@ export class ProjectsPage extends LightElement { _navigateToBoard(id) { this._view = 'board'; this._projectId = id; + this._tab = 'files'; history.pushState({ page: 'projects', id }, '', `#projects/${id}`); this.updateComplete.then(() => { - this.querySelector('project-board-section')?.load(id); + this.querySelector('project-board-section')?.load(id, 'files'); }); } @@ -73,6 +93,12 @@ export class ProjectsPage extends LightElement { }); } + _onTabChange(tab) { + this._tab = tab; + const hash = tab === 'sharing' ? `#projects/${this._projectId}/sharing` : `#projects/${this._projectId}`; + history.pushState({ page: 'projects', id: this._projectId }, '', hash); + } + render() { if (!this._open) return nothing; return html` @@ -83,6 +109,7 @@ export class ProjectsPage extends LightElement { ` : html` this._navigateToList()} + @project-tab-change=${e => this._onTabChange(e.detail.tab)} > `} `; diff --git a/web/components/projects/project-board.js b/web/components/projects/project-board.js index afe0b32..37846f1 100644 --- a/web/components/projects/project-board.js +++ b/web/components/projects/project-board.js @@ -1,16 +1,18 @@ import { html, nothing } from 'lit'; import { LightElement } from '../../lib/base.js'; import { t } from '../../lib/i18n.js'; +import { ProjectFilesPanel } from './project-files.js'; -/// A project's detail page: header + description, a sharing panel (member picker with -/// read/write, mirroring the shared-folders UI), Open chat, and a Files section (the -/// future primary surface — a file explorer over the project folder). No ticket board. +/// A project's detail page: header + description, then two tabs — **Files** (a +/// live explorer over the project folder, ``) and +/// **Sharing** (member picker with read/write, mirroring the shared-folders UI). export class ProjectBoardSection extends LightElement { static properties = { _project: { state: true }, _users: { state: true }, _add: { state: true }, _error: { state: true }, + _tab: { state: true }, }; constructor() { @@ -20,6 +22,7 @@ export class ProjectBoardSection extends LightElement { this._add = { user_id: '', can_write: false }; this._error = null; this._projectId = null; + this._tab = 'files'; } connectedCallback() { @@ -33,10 +36,11 @@ export class ProjectBoardSection extends LightElement { super.disconnectedCallback(); } - async load(projectId) { + async load(projectId, tab) { this._projectId = projectId; this._project = null; this._error = null; + this._tab = tab === 'sharing' ? 'sharing' : 'files'; try { const [projRes, usersRes] = await Promise.all([ fetch(`/api/projects/${projectId}`), @@ -114,6 +118,19 @@ export class ProjectBoardSection extends LightElement { } } + // Switch the visible tab without reloading (host back/forward sync). + setTab(tab) { + this._tab = tab === 'sharing' ? 'sharing' : 'files'; + } + + _selectTab(tab) { + if (tab === this._tab) return; + this._tab = tab; + this.dispatchEvent(new CustomEvent('project-tab-change', { + detail: { tab }, bubbles: true, composed: true, + })); + } + _back() { this.dispatchEvent(new CustomEvent('project-back', { bubbles: true, composed: true })); } @@ -203,16 +220,19 @@ export class ProjectBoardSection extends LightElement { `; } - _renderFilesPanel() { - // The file explorer is the future primary surface (a directory listing endpoint over - // the project folder is a follow-on). For now, the chat's agent works in the folder. + _renderTabs() { + const tab = (id, icon, label) => html` +
  • + +
    • + `; return html` -
    -
    - -

    ${t('projects.files.placeholder')}

    -
    -
    +
      + ${tab('files', 'bi-folder2-open', t('projects.tabs.files'))} + ${tab('sharing', 'bi-people', t('projects.tabs.sharing'))} +
    `; } @@ -250,14 +270,20 @@ export class ProjectBoardSection extends LightElement {
    ${this._error}
    ` : nothing} + ${this._project.description ? html` +

    ${this._project.description}

    + ` : nothing} + + ${this._renderTabs()} +
    - ${this._project.description - ? html`

    ${this._project.description}

    ` - : nothing} - ${this._renderFilesPanel()} - ${this._renderSharePanel()} + + ${this._tab === 'sharing' ? this._renderSharePanel() : nothing}
    `; } } + +customElements.define('project-files-panel', ProjectFilesPanel); diff --git a/web/components/projects/project-files.js b/web/components/projects/project-files.js new file mode 100644 index 0000000..d23bf47 --- /dev/null +++ b/web/components/projects/project-files.js @@ -0,0 +1,400 @@ +import { html, nothing } from 'lit'; +import { LightElement } from '../../lib/base.js'; +import { t } from '../../lib/i18n.js'; +import { fileWatcher } from '../../lib/file-watcher.js'; + +/// The Files tab of a project board: a live explorer over the project folder. +/// +/// One directory at a time (`GET /api/files/dir`); clicking a folder navigates +/// into it, clicking a file opens it in the existing viewer (`window.openFile`). +/// The breadcrumb is rooted at the project folder (shown as `/`). The listing +/// reloads in real time: the shared `/api/file/watch` socket (the `fileWatcher` +/// singleton) pushes a `changed` event for the open directory whenever another +/// member — or the agent, from inside its container — creates/modifies/removes +/// a file in it. Write actions (new folder, upload, rename, delete) are offered +/// only to members with `can_write` and are gated server-side too. +export class ProjectFilesPanel extends LightElement { + static properties = { + project: { attribute: false }, + _rel: { state: true }, + _entries: { state: true }, + _loading: { state: true }, + _error: { state: true }, + _busy: { state: true }, + _modal: { state: true }, + _drag: { state: true }, + }; + + constructor() { + super(); + this.project = null; + this._rel = ''; // path relative to the project root ('' = root) + this._entries = null; + this._loading = false; + this._error = null; + this._busy = false; + this._modal = null; // { mode: 'mkdir'|'rename', name, target? } + this._drag = false; + this._unwatch = null; + this._reloadTimer = null; + this._onChanged = () => this._scheduleReload(); + } + + willUpdate(changed) { + // (Re)open the root only when the project itself changes — a refetch of the + // same project (member edits) must not reset the current folder. + if (changed.has('project')) { + const prev = changed.get('project'); + if (this.project?.root_path && this.project.root_path !== prev?.root_path) { + this._open(''); + } + } + } + + disconnectedCallback() { + this._unwatch?.(); + clearTimeout(this._reloadTimer); + super.disconnectedCallback(); + } + + _dirPath() { + const root = this.project?.root_path ?? ''; + return this._rel ? `${root}/${this._rel}` : root; + } + + async _open(rel) { + this._unwatch?.(); + this._unwatch = null; + this._rel = rel; + this._error = null; + await this._load(); + // Live updates for the open directory (best-effort: a dead watcher just + // means manual refresh; auto-reconnect + re-subscribe are handled inside). + try { + this._unwatch = await fileWatcher.watch(this._dirPath(), this._onChanged); + } catch { this._unwatch = null; } + } + + _scheduleReload() { + clearTimeout(this._reloadTimer); + this._reloadTimer = setTimeout(() => this._load(), 300); + } + + async _load() { + if (!this.project?.root_path) return; + this._loading = true; + try { + const res = await fetch(`/api/files/dir?path=${encodeURIComponent(this._dirPath())}`); + if (!res.ok) throw new Error(await res.text()); + this._entries = await res.json(); + this._error = null; + } catch (e) { + this._error = e.message; + } finally { + this._loading = false; + } + } + + // ── Navigation ──────────────────────────────────────────────────────────── + + _enter(entry) { + if (entry.is_dir) { + this._open(this._rel ? `${this._rel}/${entry.name}` : entry.name); + } else { + window.openFile(entry.path); + } + } + + _goTo(index) { + // -1 = project root, otherwise the segment index to land on. + const segs = this._rel ? this._rel.split('/') : []; + this._open(index < 0 ? '' : segs.slice(0, index + 1).join('/')); + } + + // ── Write actions ───────────────────────────────────────────────────────── + + _openModal(mode, target = null) { + this._modal = { mode, name: target?.name ?? '', target }; + this.updateComplete.then(() => this.querySelector('.pf-modal-input')?.focus()); + } + + async _submitModal(e) { + e.preventDefault(); + const name = (this._modal?.name ?? '').trim(); + if (!name || name.includes('/') || name.includes('\\')) { + this._error = t('projects.files.error.name'); + return; + } + this._busy = true; + try { + let res; + if (this._modal.mode === 'mkdir') { + res = await fetch('/api/file', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ path: `${this._dirPath()}/${name}`, dir: true }), + }); + } else { + res = await fetch('/api/file', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ old_path: this._modal.target.path, new_path: `${this._dirPath()}/${name}` }), + }); + } + if (!res.ok) throw new Error(await res.text()); + this._modal = null; + await this._load(); + } catch (err) { + this._error = err.message; + } finally { + this._busy = false; + } + } + + async _remove(entry) { + const key = entry.is_dir ? 'projects.files.confirm.delete_dir' : 'projects.files.confirm.delete_file'; + if (!confirm(t(key, { name: entry.name }))) return; + this._busy = true; + try { + const res = await fetch(`/api/file?path=${encodeURIComponent(entry.path)}`, { method: 'DELETE' }); + if (!res.ok) throw new Error(await res.text()); + await this._load(); + } catch (e) { + this._error = e.message; + } finally { + this._busy = false; + } + } + + async _uploadFiles(files) { + if (!files?.length) return; + this._busy = true; + this._error = null; + try { + for (const f of files) { + const target = `${this._dirPath()}/${f.name}`; + const res = await fetch(`/api/file/upload?path=${encodeURIComponent(target)}`, { + method: 'POST', + body: f, + }); + if (!res.ok) throw new Error(`${f.name}: ${await res.text()}`); + } + // The watcher will also fire; reload now in case it is down. + await this._load(); + } catch (e) { + this._error = e.message; + } finally { + this._busy = false; + } + } + + _pickFiles() { + this.querySelector('.pf-file-input')?.click(); + } + + // ── Rendering ───────────────────────────────────────────────────────────── + + _renderBreadcrumb() { + const segs = this._rel ? this._rel.split('/') : []; + return html` + + `; + } + + _renderToolbar() { + const canWrite = !!this.project?.can_write; + return html` +
    + ${this._renderBreadcrumb()} +
    + + ${canWrite ? html` + + + { this._uploadFiles([...e.target.files]); e.target.value = ''; }} /> + ` : nothing} +
    +
    + `; + } + + _iconFor(entry) { + if (entry.is_dir) return 'bi-folder-fill text-warning'; + const ext = entry.name.includes('.') ? entry.name.split('.').pop().toLowerCase() : ''; + const map = { + png: 'bi-file-image', jpg: 'bi-file-image', jpeg: 'bi-file-image', + gif: 'bi-file-image', webp: 'bi-file-image', svg: 'bi-file-image', + pdf: 'bi-file-pdf', + md: 'bi-file-text', txt: 'bi-file-text', tex: 'bi-file-text', latex: 'bi-file-text', + js: 'bi-file-code', ts: 'bi-file-code', py: 'bi-file-code', rs: 'bi-file-code', + json: 'bi-file-code', html: 'bi-file-code', css: 'bi-file-code', sh: 'bi-file-code', + zip: 'bi-file-zip', gz: 'bi-file-zip', tar: 'bi-file-zip', + mp3: 'bi-file-music', wav: 'bi-file-music', ogg: 'bi-file-music', + mp4: 'bi-file-play', mov: 'bi-file-play', webm: 'bi-file-play', + doc: 'bi-file-word', docx: 'bi-file-word', + xls: 'bi-file-excel', xlsx: 'bi-file-excel', csv: 'bi-file-excel', + }; + return map[ext] ?? 'bi-file-earmark'; + } + + _fmtDate(iso) { + if (!iso) return '—'; + const d = new Date(iso); + return isNaN(d) ? '—' : d.toLocaleString(); + } + + _fmtSize(n) { + if (n == null) return '—'; + if (n < 1024) return `${n} B`; + if (n < 1024 ** 2) return `${(n / 1024).toFixed(1)} KB`; + if (n < 1024 ** 3) return `${(n / 1024 ** 2).toFixed(1)} MB`; + return `${(n / 1024 ** 3).toFixed(2)} GB`; + } + + _renderRow(entry) { + const canWrite = !!this.project?.can_write; + return html` + this._enter(entry)}> + + ${entry.name} + ${this._fmtDate(entry.created_at)} + ${this._fmtDate(entry.modified_at)} + ${entry.is_dir ? '—' : this._fmtSize(entry.size)} + ${canWrite ? html` + e.stopPropagation()}> + + + + ` : nothing} + + `; + } + + _renderTable() { + const canWrite = !!this.project?.can_write; + if (!this._entries) { + return html`
    `; + } + if (this._entries.length === 0) { + return html` +
    + +

    ${t('projects.files.empty')}

    +
    + `; + } + return html` + + + + + + + + + ${canWrite ? html`` : nothing} + + + + ${this._entries.map(e => this._renderRow(e))} + +
    ${t('projects.files.col.name')}${t('projects.files.col.created')}${t('projects.files.col.modified')}${t('projects.files.col.size')}
    + `; + } + + _renderModal() { + if (!this._modal) return nothing; + const isMkdir = this._modal.mode === 'mkdir'; + return html` +
    { if (e.target === e.currentTarget) this._modal = null; }}> +
    +
    + + + ${isMkdir ? t('projects.files.modal.mkdir') : t('projects.files.modal.rename', { name: this._modal.target.name })} + + +
    +
    this._submitModal(e)}> +
    + + this._modal = { ...this._modal, name: e.target.value }} /> +
    +
    + + +
    +
    +
    +
    + `; + } + + render() { + if (!this.project?.root_path) return nothing; + const canWrite = !!this.project?.can_write; + return html` +
    { if (canWrite) { e.preventDefault(); this._drag = true; } }} + @dragleave=${() => this._drag = false} + @drop=${e => { e.preventDefault(); this._drag = false; if (canWrite) this._uploadFiles([...e.dataTransfer.files]); }}> +
    + ${this._renderToolbar()} + ${this._error ? html` +
    ${this._error}
    + ` : nothing} + ${this._drag ? html` +
    + ${t('projects.files.drop')} +
    + ` : this._renderTable()} +
    +
    + ${this._renderModal()} + `; + } +} diff --git a/web/components/sidebar.js b/web/components/sidebar.js index 08c9cc7..57f535f 100644 --- a/web/components/sidebar.js +++ b/web/components/sidebar.js @@ -112,9 +112,12 @@ export class AppSidebar extends I18nMixin(LightElement) { if (page === 'tasks') this._tasksSection = this._tasksSectionFromHash(); this._applyPage(page); }, 0); - // Poll inbox count independently of whether the page is open. + // Poll inbox count independently of whether the page is open. The 60 s + // interval is only a fallback: `inbox-changed` (pushed over the chat WS + // when any session raises/settles a pending item) refreshes it live. this._pollInbox(); - this._pollTimer = setInterval(() => this._pollInbox(), 10000); + this._pollTimer = setInterval(() => this._pollInbox(), 60000); + window.addEventListener('inbox-changed', () => this._pollInbox()); this._loadCollapsed(); this._loadDebugMode(); this._loadRecentProjects(); diff --git a/web/i18n/en.js b/web/i18n/en.js index fcba7f1..4c19197 100644 --- a/web/i18n/en.js +++ b/web/i18n/en.js @@ -221,7 +221,26 @@ export default { 'projects.share.access.write': 'Write', 'projects.share.access.readonly': 'Read-only', 'projects.share.access.readwrite':'Read & write', - 'projects.files.placeholder': 'The project files live here. Open the chat to work in this folder — a file explorer is coming.', + 'projects.tabs.files': 'Files', + 'projects.tabs.sharing': 'Sharing', + 'projects.files.col.name': 'Name', + 'projects.files.col.created': 'Created', + 'projects.files.col.modified': 'Modified', + 'projects.files.col.size': 'Size', + 'projects.files.empty': 'This folder is empty.', + 'projects.files.refresh': 'Refresh', + 'projects.files.drop': 'Drop files here to upload', + 'projects.files.btn.new_folder': 'New folder', + 'projects.files.btn.upload': 'Upload', + 'projects.files.uploading': 'Uploading…', + 'projects.files.action.rename': 'Rename', + 'projects.files.action.delete': 'Delete', + 'projects.files.confirm.delete_file': 'Delete "{name}"?', + 'projects.files.confirm.delete_dir': 'Delete the folder "{name}" and everything inside it?', + 'projects.files.modal.mkdir': 'New folder', + 'projects.files.modal.rename': 'Rename "{name}"', + 'projects.files.modal.name': 'Name', + 'projects.files.error.name': 'Enter a valid name (no slashes).', // ── Project detail ────────────────────────────────────────────────────────── 'project_board.back': 'Projects', diff --git a/web/i18n/fr.js b/web/i18n/fr.js index e328258..237add5 100644 --- a/web/i18n/fr.js +++ b/web/i18n/fr.js @@ -221,7 +221,26 @@ export default { 'projects.share.access.write': 'Écriture', 'projects.share.access.readonly': 'Lecture seule', 'projects.share.access.readwrite':'Lecture et écriture', - 'projects.files.placeholder': 'Les fichiers du projet vivent ici. Ouvrez la discussion pour travailler dans ce dossier — un explorateur de fichiers arrive bientôt.', + 'projects.tabs.files': 'Fichiers', + 'projects.tabs.sharing': 'Partage', + 'projects.files.col.name': 'Nom', + 'projects.files.col.created': 'Création', + 'projects.files.col.modified': 'Modification', + 'projects.files.col.size': 'Taille', + 'projects.files.empty': 'Ce dossier est vide.', + 'projects.files.refresh': 'Actualiser', + 'projects.files.drop': 'Déposez les fichiers ici pour les envoyer', + 'projects.files.btn.new_folder': 'Nouveau dossier', + 'projects.files.btn.upload': 'Envoyer', + 'projects.files.uploading': 'Envoi…', + 'projects.files.action.rename': 'Renommer', + 'projects.files.action.delete': 'Supprimer', + 'projects.files.confirm.delete_file': 'Supprimer « {name} » ?', + 'projects.files.confirm.delete_dir': 'Supprimer le dossier « {name} » et tout son contenu ?', + 'projects.files.modal.mkdir': 'Nouveau dossier', + 'projects.files.modal.rename': 'Renommer « {name} »', + 'projects.files.modal.name': 'Nom', + 'projects.files.error.name': 'Saisissez un nom valide (sans barres obliques).', // ── Détail du projet ──────────────────────────────────────────────────────── 'project_board.back': 'Projets', diff --git a/web/i18n/it.js b/web/i18n/it.js index 36598d2..2ce1a1a 100644 --- a/web/i18n/it.js +++ b/web/i18n/it.js @@ -245,7 +245,26 @@ export default { 'projects.share.access.write': 'Scrittura', 'projects.share.access.readonly': 'Sola lettura', 'projects.share.access.readwrite':'Lettura e scrittura', - 'projects.files.placeholder': 'Qui vivono i file del progetto. Apri la chat per lavorare in questa cartella — un file explorer è in arrivo.', + 'projects.tabs.files': 'File', + 'projects.tabs.sharing': 'Condivisione', + 'projects.files.col.name': 'Nome', + 'projects.files.col.created': 'Creazione', + 'projects.files.col.modified': 'Ultima modifica', + 'projects.files.col.size': 'Dimensione', + 'projects.files.empty': 'Questa cartella è vuota.', + 'projects.files.refresh': 'Aggiorna', + 'projects.files.drop': 'Trascina qui i file per caricarli', + 'projects.files.btn.new_folder': 'Nuova cartella', + 'projects.files.btn.upload': 'Carica', + 'projects.files.uploading': 'Caricamento…', + 'projects.files.action.rename': 'Rinomina', + 'projects.files.action.delete': 'Elimina', + 'projects.files.confirm.delete_file': 'Eliminare "{name}"?', + 'projects.files.confirm.delete_dir': 'Eliminare la cartella "{name}" e tutto il suo contenuto?', + 'projects.files.modal.mkdir': 'Nuova cartella', + 'projects.files.modal.rename': 'Rinomina "{name}"', + 'projects.files.modal.name': 'Nome', + 'projects.files.error.name': 'Inserisci un nome valido (senza barre).', // ── Dettaglio progetto ────────────────────────────────────────────────────── 'project_board.back': 'Progetti', diff --git a/web/lib/chat-session.js b/web/lib/chat-session.js index d6229b1..6dd5a1a 100644 --- a/web/lib/chat-session.js +++ b/web/lib/chat-session.js @@ -405,6 +405,7 @@ export class ChatSession extends LightElement { case 'approval_resolved': { const { request_id, tool_call_id, approved } = msg; + window.dispatchEvent(new CustomEvent('inbox-changed')); this._updatePendingWrite(request_id, { status: approved ? 'approved' : 'rejected' }); if (tool_call_id != null) { if (approved) { @@ -419,6 +420,17 @@ export class ChatSession extends LightElement { break; } + case 'approval_requested': + case 'clarification_requested': + case 'clarification_resolved': + case 'elicitation_requested': + case 'elicitation_resolved': + // Inbox lifecycle from any of this user's sessions (chat, cron, + // background): nudge listeners (sidebar badge, inbox page) to refresh + // immediately instead of waiting for the next poll. + window.dispatchEvent(new CustomEvent('inbox-changed')); + break; + case 'agent_question': // Link the question form to the tool card by updating status + storing request_id. this._updateTool(msg.tool_call_id, { From e70c4a90f31e19f54232afc8a2596e41282abe13 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 18:52:03 +0100 Subject: [PATCH 03/13] file viewer, docs, ws: add image/media preview path, projects doc, ws wiring Show file gains image and video display for capable agents. Docs add projects.md and update index. Wire ws file-watch in project-board. Minor fs tool and CLAUDE.md updates. --- CLAUDE.md | 26 +++++++--- crates/skald-core/src/tools/fs/mod.rs | 6 +-- crates/skald-core/src/tools/show_file.rs | 66 +++++++++++++++++++----- docs/index.md | 8 ++- docs/projects.md | 54 +++++++++++++++++++ src/frontend/api/files.rs | 31 +++++++++++ src/frontend/api/ws.rs | 2 + web/components/projects/project-board.js | 13 +++-- 8 files changed, 176 insertions(+), 30 deletions(-) create mode 100644 docs/projects.md diff --git a/CLAUDE.md b/CLAUDE.md index 87e742e..ce18eed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -89,7 +89,7 @@ Two rules keep the boundary real, and both are enforced by the compiler: | `crates/skald-core/src/approval/` | Approval rules engine | | `crates/skald-core/src/clarification/` | `ClarificationManager`: background-session question/answer | | `crates/skald-core/src/elicitation/` | `ElicitationManager` + bridge: MCP server-initiated input (`elicitation/create`), surfaced in the Inbox; secrets never logged/persisted | -| `crates/skald-core/src/inbox.rs` | `Inbox`: unified façade for pending approvals + clarifications + elicitations (wraps ApprovalManager, ClarificationManager, ElicitationManager) | +| `crates/skald-core/src/inbox.rs` | `Inbox`: unified façade for pending approvals + clarifications + elicitations (wraps ApprovalManager, ClarificationManager, ElicitationManager). The managers already emit the `*Requested`/`*Resolved` lifecycle events on the per-user bus; `ws.rs` forwards them to every connected client of that user regardless of `source`, so the web UI updates live (see `sidebar.js` row) | | `crates/skald-core/src/llm/` | LLM client abstraction (OpenAI-compat, Anthropic, Ollama…). OpenAI-compatible provider *types* are runtime data, not code: `providers/declared.rs` loads `providers.yaml` at boot (see Config); only non-OpenAI-compatible or bespoke providers (anthropic, ollama, openai, openrouter) stay native. **Retriability** (`llm_call.rs::is_retriable_llm_error`) keys on the real HTTP status via `llm_client::http_status` (a structured `LlmError { status }` from the client, else a `reqwest::Error` in the chain), **not** a substring of the message — a model id/token count containing "404"/"401" no longer mis-classifies; 401/403/404/422 don't retry, 400/429/5xx/network do | | `crates/skald-core/src/transcribe/` | Transcription providers | | `crates/skald-core/src/image_generate/` | Image generation providers | @@ -105,14 +105,14 @@ Two rules keep the boundary real, and both are enforced by the compiler: The schema is split into two buckets (§5.1), and the split is the point: -- **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `plugin_access` + `plugin_user_configs`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `oauth_providers`, `role_capabilities`, `shared_folders` + `shared_folder_members`. The MCP tables back the Connectors model (§7/§14/§15 — see its own section); `oauth_providers` (accessor `db/oauth_providers.rs`) holds one row per identity provider (Google…) — endpoints + `client_id`/`client_secret` + `redirect_uri`, admin-owned household secrets (§4/§15b), never a per-user token. The last two (accessor `db/shared_folders.rs`) are the **membership** of the on-disk shared folders (§6): a junction table so a member can be read-only (`can_write`) and so the container mount topology + the `shared/{X}` fs routing both query it. FK `shared_folder_members.user_id → users(id)` is registry→registry (same file), which is allowed — unlike an owner→registry key. -- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `projects`, `project_tickets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`). `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. +- **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `plugin_access` + `plugin_user_configs`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `oauth_providers`, `role_capabilities`, `shared_folders` + `shared_folder_members`, `projects` + `project_members`. The MCP tables back the Connectors model (§7/§14/§15 — see its own section); `oauth_providers` (accessor `db/oauth_providers.rs`) holds one row per identity provider (Google…) — endpoints + `client_id`/`client_secret` + `redirect_uri`, admin-owned household secrets (§4/§15b), never a per-user token. The last two pairs are junction-backed membership: `shared_folder_members` (accessor `db/shared_folders.rs`) for the on-disk shared folders (§6), `project_members` (accessor `db/project_members.rs`) for projects (see the Projects section) — both let a member be read-only (`can_write`) and both drive the container mount topology + the fs routing. Their FKs are registry→registry (same file), which is allowed — unlike an owner→registry key. +- **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`). `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. (`projects`/`project_tickets` were owner tables in the single-user past: projects are shareable now, so `projects` + `project_members` are registry tables and `project_tickets` is gone.) Schema is greenfield (no migrations, §0), but a purely **additive** column lands on an existing DB in place: `db::ensure_column` runs `ALTER TABLE … ADD COLUMN` and swallows the "duplicate column" error, a no-op on a fresh DB where the `CREATE TABLE` already has the column. Used for the OAuth columns on `mcp_catalog` / `mcp_user_servers` so a dev box need not be wiped for an additive change (a full recreate is still valid). -**No foreign key in the owner bucket may point at a registry table.** SQLite cannot enforce a key across files, not even through `ATTACH`, and sqlx turns on `PRAGMA foreign_keys`: the `CREATE TABLE` succeeds and every `INSERT` fails. `db::tests::owner_tables_stand_alone_with_foreign_keys_on` enforces this by running the owner schema against a database holding nothing else, then inserting a row into each table. Two keys crossed and were fixed: `chat_history.model_db_id` (dropped — write-only, and `llm_requests.model_name` already records the model) and `project_tickets.job_id` (fixed by moving `projects`/`project_tickets` into the owner bucket). +**No foreign key in the owner bucket may point at a registry table.** SQLite cannot enforce a key across files, not even through `ATTACH`, and sqlx turns on `PRAGMA foreign_keys`: the `CREATE TABLE` succeeds and every `INSERT` fails. `db::tests::owner_tables_stand_alone_with_foreign_keys_on` enforces this by running the owner schema against a database holding nothing else, then inserting a row into each table. One key crossed and was fixed: `chat_history.model_db_id` (dropped — write-only, and `llm_requests.model_name` already records the model). -**Memory namespace (blueprint §5).** `memory_docs` (accessor `db/memory_docs.rs` — `get`/`upsert`/`list`/`search`(FTS)/`delete`) backs a virtual note store surfaced through the fs-tools, **not** the disk. Two sibling roots (not the blueprint's nested `memory/{userid}` + `memory/shared`): `user-memory/…` routes to the caller's own pool (`ToolContext::pool`), `shared-memory/…` to the system pool (a singleton captured in `fs::register_all`). `tools/fs/classify_memory()` decides on the raw first path component (a `..` in the tail clamps inside the store, never escapes to disk); `read_file`/`write_file`/`list_files`/`edit_file`/`insert_at_line`/`replace_lines`/`search_file` override `run_with` to route memory paths (each extracting a pure transform shared with its on-disk `execute`) and leave every other path on disk. Approval (seeded in `seed_fs_path_rules`): `user-memory/*` is `@fs_any allow` (private, frictionless); `shared-memory/*` is `@fs_read allow` + `@fs_write require` — reads free, **writes need approval** so the agent can't silently push one person's data into shared memory. `grep_files` stays disk-only (regex-across-tree ≠ FTS); ranked full-text recall over notes is a separate tool, `memory_search` (`tools/fs/memory_search.rs`), over the `memory_docs` FTS index — allowed by a path-less rule (it takes `query`, not `path`). +**Memory namespace (blueprint §5).** `memory_docs` (accessor `db/memory_docs.rs` — `get`/`upsert`/`list`/`search`(FTS)/`delete`) backs a virtual note store surfaced through the fs-tools, **not** the disk. Two sibling roots (not the blueprint's nested `memory/{userid}` + `memory/shared`): `user-memory/…` routes to the caller's own pool (`ToolContext::pool`), `shared-memory/…` to the system pool (a singleton captured in `fs::register_all`). `tools/fs/classify_memory()` decides on the raw first path component (a `..` in the tail clamps inside the store, never escapes to disk); `read_file`/`write_file`/`list_files`/`edit_file`/`insert_at_line`/`replace_lines`/`search_file` override `run_with` to route memory paths (each extracting a pure transform shared with its on-disk `execute`) and leave every other path on disk. The HTTP surface routes them the same way: `GET /api/file` classifies **before** `resolve_view_path` and serves the note from `memory_docs` (caller's pool / system pool), so the file viewer opens `user-memory/…` and `shared-memory/…` like any file, and `show_file_to_user` accepts memory paths too (existence-checked on the right pool). Approval (seeded in `seed_fs_path_rules`): `user-memory/*` is `@fs_any allow` (private, frictionless); `shared-memory/*` is `@fs_read allow` + `@fs_write require` — reads free, **writes need approval** so the agent can't silently push one person's data into shared memory. `grep_files` stays disk-only (regex-across-tree ≠ FTS); ranked full-text recall over notes is a separate tool, `memory_search` (`tools/fs/memory_search.rs`), over the `memory_docs` FTS index — allowed by a path-less rule (it takes `query`, not `path`). **Memory injection into the prompt**: `MessageBuilder::load_inject_memory` routes each `meta.inject_memory` entry — `user-memory/…` → owner pool, `shared-memory/…` → the shared (`system.db`) pool, both via `memory_docs::get`; anything else (`data/…`, `$WD/…`) is a disk read. The shared pool is threaded `ChatSessionManager` → handler → `MessageBuilder`. `assistant` and `project-coordinator` inject `user-memory/index.md` + `shared-memory/index.md`. @@ -133,6 +133,7 @@ The agent sees **one namespace**, routed on the first path component. The choke | `user-memory/…` | SQLite `ctx.pool` (`{userid}.db`) | `classify_memory` → `memory_docs` | | `shared-memory/…` | SQLite `system.db` | `classify_memory` → `memory_docs` | | `shared/{X}/…` | host `{WD}/shared/{X}` (if a member) | `UserFs::host_base_and_tail` | +| `projects/{O}/{S}/…` | host `{WD}/projects/{owner_userid}/{S}` (if a member) | `UserFs::host_base_and_tail` | | `~/…`, relative | host `{WD}/homes/{userid}` | `UserFs::host_base_and_tail` | Two views, **one storage**: the fs-tools run **host-side** in the Skald process on `{WD}/homes/{userid}` + `{WD}/shared/{X}`; `execute_cmd` runs **inside the container** (`docker exec -w skald-{userid} sh -c …`, via `ExecuteCmd::run_with`) on the same paths bind-mounted (`homes/{userid}`→`/root`, `shared/{X}`→`/root/shared/{X}`, read-only when `can_write=0`). A file written in the container appears to the host fs-tools and vice versa. @@ -141,6 +142,16 @@ Two views, **one storage**: the fs-tools run **host-side** in the Skald process The threading: `UserContext.fs` (built by `container::build_user_fs` at login, snapshotting shared memberships) → `ChatSessionManager` → `ChatSessionHandler.fs` → `ToolContext.fs`. **Admin CRUD is wired** (`src/frontend/api/shared_folders.rs` — `GET/POST /api/shared-folders`, `PATCH/DELETE /api/shared-folders/{id}`, `POST`/`DELETE .../members[/{user_id}]`; UI `shared-folders.js`): a create/describe/delete + per-member `can_write` surface, and each mutation calls a best-effort `remount(user)` that rebuilds the affected user's fs + container mounts **in place** — so a membership change lands without a re-login (blueprint §6's "admin CRUD" + "membership refresh without re-login" TODOs, now closed; it still settles at next login/boot if the live remount fails). `execute_cmd` /stop is robust: the command runs under `setsid -w` in its own process-group (leader pid recorded in a container pidfile), and a `KillReaper` drop-guard reaps that group on /stop **or** timeout via a detached `docker exec` that walks `/proc` and kills members by **positive pid** (the container's dash mishandles `kill -`); the pidfile is passed positionally (`$1`), and the container's `--init` (tini) reaps the killed processes so no zombies accumulate. **Per-user MCP connectors now run inside this container** (§7) — the container infra enabled it; see the MCP connectors section. +## Projects + +A **project** is a shareable, self-service workspace: a folder at `{WD}/projects/{owner_userid}/{slug}` plus membership in the registry. `projects` (accessor `db/projects.rs` — slug is immutable, `UNIQUE(owner_user_id, slug)`) + `project_members` (junction with `can_write`; the owner is always a write-member, so a private project = one member). Sharing is **not** admin-gated: the owner and any write-member can add/remove/re-grant members and edit metadata; only the owner can delete. Each membership mutation remounts the affected users' containers in place (`Skald::refresh_user_mounts`). The mount appears in the agent namespace as `projects/{owner_username}/{slug}` (host keys on the stable userid, agent path on the username) — read-only members get a read-only bind mount in the container. + +**API** (`src/frontend/api/projects.rs`): `GET/POST /api/projects`, `GET/PUT/DELETE /api/projects/{id}`, `POST /api/projects/{id}/members`, `DELETE .../members/{user_id}`, `POST /api/projects/{id}/session`. `ProjectDetail` carries `root_path` — the agent path of the folder, computed server-side (owner username ≠ `owner_name`, which may be a display name) — the explorer's root. A `project-{id}` chat source provisions the `project-coordinator` agent with a project `RunContext` (`provisioning_for_source` → `skald_core::projects::build_project_run_context`: `project_root` + a system block with name/description/folder/members); every member keeps their **own private** `project-{id}` session — only the folder is shared. + +**UI** (`web/components/projects/`): `index.js` (`` host — hash-routed: `#projects`, `#projects/{id}`, `#projects/{id}/sharing`, back/forward-aware), `project-list.js` (card grid + create/edit/delete modal), `project-board.js` (`` — the detail page: header with **Open chat**, then a **Files / Sharing** tab bar using the `.project-tab-bar` styles in `css/projects/board.css`), `project-files.js` (`` — the explorer). The mobile app has its own read-only `shared/projects-page.js` (list → open project chat). + +**The explorer** (`project-files.js`): one directory at a time via `GET /api/files/dir?path=…` (new endpoint in `src/frontend/api/files.rs`: immediate children with `name/path/is_dir/size/created_at/modified_at`, dirs-first; same `resolve_view_path` scoping as `/api/file`). Breadcrumb rooted at the project (`/` = `root_path`); file click → `window.openFile` (existing viewer); folder click → navigate. **Live**: it subscribes the open directory on the existing `/api/file/watch` socket (`web/lib/file-watcher.js` singleton — `notify` NonRecursive on a dir reports its direct children) and reloads debounced 300 ms, so files created by other members or by the agent in-container appear without a refresh. Write actions (new folder, upload incl. drag&drop, rename, delete) are shown only to `can_write` members and ride the existing `/api/file` endpoints — `POST` gained `dir:true` (mkdir), `DELETE` handles directories (`remove_dir_all`), and binary upload is the new `POST /api/file/upload?path=…` (raw body, 256 MiB `DefaultBodyLimit`). **Server-side write gate**: all `/api/file` write handlers now call `UserFs::can_write_to(agent_path)` (core-api) — home → true, `shared/`/`projects/` → the membership's `can_write`, `docs/` → false — closing the host-side bypass of the read-only bind mount (the container mount only gates in-container writes). + ## MCP connectors (blueprint §7/§14/§15) MCP servers are surfaced to users as **"Connectors"** (UI naming; `mcp`/schema stays neutral, §0.1). The old single owner table `mcp_servers`, the agent-facing `register_mcp`/`delete_mcp` tools, and the `mcp` kinds of `list_items`/`toggle_item` are **gone**. Connectors are now admin-curated and user-activated through the Connectors UI/API — never written by the agent, which closes the §14 RCE vector (prompt-injection → agent writes+registers a local script → arbitrary code on the box). @@ -257,7 +268,7 @@ Create `agents//meta.json` and `agents//AGENT.md`. The agent is discover ## Documentation -`docs/` is **not developer documentation** — it's written for the in-app LLM, not for a human reading the repo, and is mounted read-only into every user's container at `~/docs/` (see the Filesystem & containers section: `docs_host` on `UserFs`, `DOCS_DIR` in `container/mod.rs`). It explains the software's UX (plugins, and eventually agents/connectors/memory/roles/…) in plain terms, in English, so the assistant can help a non-technical user configure things instead of guessing. `docs/index.md` is the entry point; `docs/plugins/.md` covers each built-in plugin. The three `type: chat` agents (`assistant`, `kid`, `project-coordinator`) are told in their `AGENT.md` to read `docs/index.md` when a user asks how the software works. Keep it in sync when plugins or major UX-facing behavior change — it goes stale like any other doc. +`docs/` is **not developer documentation** — it's written for the in-app LLM, not for a human reading the repo, and is mounted read-only into every user's container at `~/docs/` (see the Filesystem & containers section: `docs_host` on `UserFs`, `DOCS_DIR` in `container/mod.rs`). It explains the software's UX (plugins, and eventually agents/connectors/memory/roles/…) in plain terms, in English, so the assistant can help a non-technical user configure things instead of guessing. `docs/index.md` is the entry point (general index of feature pages); `docs/plugins/.md` covers each built-in plugin. The three `type: chat` agents (`assistant`, `kid`, `project-coordinator`) are told in their `AGENT.md` to read `docs/index.md` when a user asks how the software works. **Standing rule: every change that impacts the UX must update `docs/` in the same change** — a new/renamed feature page plus the `docs/index.md` index entry. It goes stale like any other doc, except users actually see this one. ## Config @@ -294,7 +305,7 @@ All extend `LightElement` from `web/lib/base.js` (Lit). `ChatSession` (`web/lib/ | `copilot.js` | `` | The chat surface (`_wsSource='web'`): full/dock roving layout, welcome hero empty state, privacy chip, composer with model pill, slash-command autocomplete | | `shared/chat-page.js` | `` | Mobile chat (`_wsSource='mobile'`) | | `copilot-render.js` | (helpers) | `renderMsg`, `renderTool`, `renderDiff`, etc. — shared by copilot and chat-page | -| `sidebar.js` | `` | Nav sidebar; role-driven (`ui_mode`); polls `/api/inbox` every 10 s for badge | +| `sidebar.js` | `` | Nav sidebar; role-driven (`ui_mode`); inbox badge is **live** — the chat WS forwards the inbox lifecycle events (`approval_requested/resolved`, `clarification_*`, `elicitation_*`) regardless of `source`, `chat-session.js` re-dispatches them as the `inbox-changed` window event, and the sidebar (+ `agent-inbox.js`) refreshes on it; a 60 s poll remains as fallback | | `topbar.js` | `` | Top nav bar; per-user avatar color hashed from the username | | `dashboard-page.js` | `` | `#dashboard` — status hero, LLM stats charts, pending inbox, quick guide | | `shared/file-viewer-base.js` | `FileViewerBase` (base) | Shared file-viewer engine (fetch, kind detection, markdown/PDF/SVG/LaTeX, watcher, `_renderBody`); driven by `_show`/`_hide`. Extended by desktop + mobile | @@ -310,6 +321,7 @@ All extend `LightElement` from `web/lib/base.js` (Lit). `ChatSession` (`web/lib/ | `plugin-detail.js` | `` | `#plugin-detail?id=` — one plugin's admin page: instance-config form (`config_schema`) + per-user access checklist (plugin twin of `connector-detail.js`) | | `plugin-page-host.js` | `` | Host for plugin-contributed pages (`#plugin//`): dynamic-imports the fragment module, registers its element, mounts it with `plugin-id` | | `shared-folders.js` | `` | `#shared-folders` — admin-only CRUD for on-disk shared folders (§6): create/describe/delete + per-member read-only/read-write grants; description feeds the assistant's `__SHARED_FOLDERS__` context | +| `projects/` | `` | `#projects` — host + list + board; the board is tabbed (**Files** explorer with live watcher + write actions, **Sharing** members), deep-linked `#projects/{id}[/sharing]`. See the Projects section | | `connector-detail.js` | `` | A connector's own page (`#connector?name=X`): env/secret form + Test, the **OAuth login panel** (sign in → paste code → complete, §15), global enable + per-user access grants | | `shared/connector-common.js` | (helpers) | Shared Connectors vocabulary: `statusOf` (incl. `needs_login` for a pending OAuth row), `STATUS_LABEL`, schema normalization, `jf` fetch | | `llm-providers.js` | `` | LLM provider management | diff --git a/crates/skald-core/src/tools/fs/mod.rs b/crates/skald-core/src/tools/fs/mod.rs index 2c1a778..375032c 100644 --- a/crates/skald-core/src/tools/fs/mod.rs +++ b/crates/skald-core/src/tools/fs/mod.rs @@ -52,7 +52,7 @@ pub const USER_MEMORY_ROOT: &str = "user-memory"; pub const SHARED_MEMORY_ROOT: &str = "shared-memory"; /// Which memory store a path resolves to. -pub(crate) enum MemScope { +pub enum MemScope { /// `user-memory/…` → the caller's own pool (`ToolContext::pool`). User, /// `shared-memory/…` → the shared system pool. @@ -61,7 +61,7 @@ pub(crate) enum MemScope { /// A path that falls inside the virtual memory namespace: the store it belongs to /// and the note key **relative to that store's root** (the root prefix stripped). -pub(crate) struct MemRef { +pub struct MemRef { pub scope: MemScope, pub rel: String, } @@ -75,7 +75,7 @@ pub(crate) struct MemRef { /// disk path. The tail is then normalized (resolving `.`/`..`) and clamped at the /// store root, so a memory path stays within its store and an absolute path is /// always disk. -pub(crate) fn classify_memory(user_path: &str) -> Option { +pub fn classify_memory(user_path: &str) -> Option { let mut parts = user_path.trim_start_matches("./").splitn(2, ['/', '\\']); let scope = match parts.next()? { USER_MEMORY_ROOT => MemScope::User, diff --git a/crates/skald-core/src/tools/show_file.rs b/crates/skald-core/src/tools/show_file.rs index 61d6103..1822fdc 100644 --- a/crates/skald-core/src/tools/show_file.rs +++ b/crates/skald-core/src/tools/show_file.rs @@ -1,17 +1,20 @@ use std::sync::Arc; use serde_json::{Value, json}; +use sqlx::SqlitePool; use core_api::user_fs::SharedFs; use crate::chat_hub::ChatHub; +use crate::db::memory_docs; use crate::events::{GlobalEvent, ServerEvent}; use crate::session::handler::{InterfaceTool, ToolFuture}; use crate::tools::fs; use crate::tools::tool_names::SHOW_FILE_TO_USER; -/// Build a `show_file_to_user` InterfaceTool bound to a `ChatHub`, a source and the -/// caller's [`SharedFs`] (their per-user filesystem view). +/// Build a `show_file_to_user` InterfaceTool bound to a `ChatHub`, a source, the +/// caller's [`SharedFs`] (their per-user filesystem view) and the two memory +/// pools (the caller's own + the shared system one). /// /// Injected only for SPA clients (web copilot + mobile) at the WebSocket entry /// point, so Telegram — which has its own `send_attachment` — never sees it. @@ -19,11 +22,20 @@ use crate::tools::tool_names::SHOW_FILE_TO_USER; /// The path is resolved through the caller's own workspace (`resolve_view_path`): /// `~/…`, `shared/{X}/…`, `projects/{O}/{S}/…`, a bare relative path, or a /// container-absolute `/root/…` — anything outside the container view is refused. +/// A path under a memory root (`user-memory/…`, `shared-memory/…`) is a virtual +/// note instead: it is looked up in `memory_docs` on the matching pool — the +/// viewer's `GET /api/file` applies the same routing, so it round-trips. /// It then emits a `ServerEvent::OpenFile` carrying the **canonical agent path**, so -/// the file-viewer page fetches the same file back through `/api/file` (which applies -/// the identical per-user resolution). The frontend renders every kind in the viewer -/// (HTML live in an origin-isolated iframe; LaTeX compiled to PDF server-side). -pub fn make_tool(hub: Arc, source: String, fs: SharedFs) -> InterfaceTool { +/// the file-viewer page fetches the same file back through `/api/file`. The +/// frontend renders every kind in the viewer (HTML live in an origin-isolated +/// iframe; LaTeX compiled to PDF server-side). +pub fn make_tool( + hub: Arc, + source: String, + fs: SharedFs, + user_pool: SqlitePool, + shared_pool: SqlitePool, +) -> InterfaceTool { let definition = json!({ "type": "function", "function": { @@ -34,7 +46,8 @@ pub fn make_tool(hub: Arc, source: String, fs: SharedFs) -> InterfaceTo to PDF automatically on the server). HTML files open in a \ new browser tab. Use this to surface a file you created or \ found so the user can look at it directly. One file per call. \ - The file must already exist on disk. \ + The file must already exist on disk — or as a memory note \ + (`user-memory/…`, `shared-memory/…`). \ IMPORTANT for LaTeX: always pass the `.tex` source, never a \ pre-built `.pdf` of a document you have the `.tex` for. The \ `.tex` is compiled on the server and the view live-reloads \ @@ -49,8 +62,9 @@ pub fn make_tool(hub: Arc, source: String, fs: SharedFs) -> InterfaceTo "type": "string", "description": "Path of the file to show, in your own workspace: relative to your \ home (e.g. `report.md` or `~/report.md`), a `shared//…` or \ - `projects///…` path, or an absolute container path \ - (`/root/…`). Paths outside your workspace are refused." + `projects///…` path, a memory note \ + (`user-memory/…`, `shared-memory/…`), or an absolute container \ + path (`/root/…`). Paths outside your workspace are refused." } }, "required": ["path"] @@ -59,14 +73,42 @@ pub fn make_tool(hub: Arc, source: String, fs: SharedFs) -> InterfaceTo }); let handler = Arc::new(move |args: Value| -> ToolFuture { - let hub = Arc::clone(&hub); - let source = source.clone(); - let fs = fs.clone(); + let hub = Arc::clone(&hub); + let source = source.clone(); + let fs = fs.clone(); + let user_pool = user_pool.clone(); + let shared_pool = shared_pool.clone(); Box::pin(async move { let path = args["path"] .as_str() .ok_or_else(|| anyhow::anyhow!("show_file_to_user: missing required parameter 'path'"))?; + // Virtual memory namespace → a `memory_docs` note, not a disk file. + // The viewer serves it through the same routing (see GET /api/file), + // so confirming existence is all that's needed here. + if let Some(mem) = fs::classify_memory(path) { + if mem.rel.is_empty() { + anyhow::bail!("show_file_to_user: '{path}' is a memory folder, not a file"); + } + let (pool, root) = match mem.scope { + fs::MemScope::User => (&user_pool, fs::USER_MEMORY_ROOT), + fs::MemScope::Shared => (&shared_pool, fs::SHARED_MEMORY_ROOT), + }; + let exists = memory_docs::get(pool, &mem.rel).await + .map_err(|e| anyhow::anyhow!("show_file_to_user: {e}"))? + .is_some(); + if !exists { + anyhow::bail!("show_file_to_user: file not found: {path}"); + } + let display = format!("{root}/{}", mem.rel); + hub.emit(GlobalEvent { + source: Some(source), + session_id: None, + event: ServerEvent::OpenFile { path: display.clone() }, + }); + return Ok(format!("Opened {display} in the user's viewer.")); + } + // Resolve against the caller's workspace snapshot: gives the host path to // stat and the canonical agent path the viewer will fetch back. let user_fs = fs.load(); diff --git a/docs/index.md b/docs/index.md index 1b04cb9..f7750a7 100644 --- a/docs/index.md +++ b/docs/index.md @@ -4,7 +4,13 @@ This folder is written for **you, the assistant**, not for the human directly. I Keep answers grounded in what's actually enabled and configured for this instance — check with the relevant tool (e.g. list installed/enabled plugins) rather than assuming everything described here is turned on. A feature documented here may not be enabled on this particular instance. -This index will grow over time. Right now it covers plugins; more sections (agents, connectors, memory, security groups, shared folders, projects…) will be added later. +This index will grow over time. Right now it covers projects and plugins; more sections (agents, connectors, memory, security groups, shared folders…) will be added later. + +## Features + +| Document | What it covers | +| --- | --- | +| [projects.md](projects.md) | Projects: shared folders with their own assistant chat, a live file explorer, and member sharing | ## Plugins diff --git a/docs/projects.md b/docs/projects.md new file mode 100644 index 0000000..f5fb241 --- /dev/null +++ b/docs/projects.md @@ -0,0 +1,54 @@ +# Projects + +A **project** is a shared workspace: a folder on the server plus its own chat with the assistant. Members of the group can work on the same files — directly, or by asking the assistant in the project chat — without giving anyone access to their private home folder. + +Examples: a household budget, a holiday plan, a shared recipe collection, a small work document base. + +## Creating a project + +1. Open **Projects** in the sidebar. +2. Click **New Project**, give it a name and an optional description, save. + +You become the project's owner. Only you can delete the project; everything else (editing the description, sharing) can also be done by members you grant read & write access. + +## The project page + +Opening a project shows its page, with two tabs (the current tab is part of the address, so you can bookmark or share the link): + +- **Files** — the project's file explorer (see below). +- **Sharing** — who can access the project. + +The header also has an **Open chat** button: it opens the project's conversation with the assistant. The assistant already knows the project folder and works directly inside it — creating documents, searching, summarizing. Each member has their **own private** conversation about the project; only the files are shared. + +## The Files tab + +A file explorer rooted at the project folder: + +- The **breadcrumb** on top shows where you are, relative to the project root (`/`, then `/folder`, `/folder/subfolder`). Click any segment to jump back. +- Files and folders are listed as a table: icon, name, creation date, last-modified date, size. +- Click a **file** to open it in the file viewer (Markdown rendered, images, PDFs, text…). +- Click a **folder** to navigate into it. +- The listing **updates by itself**: if another member or the assistant creates, renames or deletes a file while you're looking at a folder, the change appears within a second — no refresh needed. + +If you have write access you can also, from the toolbar or each row: + +- **New folder** — create a subfolder in the current location. +- **Upload** — send files from your device into the current folder (or just drag & drop them onto the list). +- **Rename** and **Delete** — from the icons on each row. Deleting a folder removes everything inside it, after a confirmation. + +Read-only members see the same explorer and can open every file, but the write actions are hidden (and refused by the server anyway). + +## The Sharing tab + +Lists every member with their access level. The owner and any read & write member can: + +- **Add a member** — pick a person and their access: *Read* (browse and open files only) or *Read & write* (can also create, edit, delete and share). +- **Change access** or **remove** a member (the owner can't be removed). + +Access changes apply immediately — no need for the other person to log out. + +## Notes + +- A private project is simply a project with one member (you). Share it later whenever you want. +- Renaming a project does not move its folder, so links and the assistant's context keep working. +- Deleting a project removes its folder for everyone — there is no undo. diff --git a/src/frontend/api/files.rs b/src/frontend/api/files.rs index 79424f4..dbd86df 100644 --- a/src/frontend/api/files.rs +++ b/src/frontend/api/files.rs @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize}; use std::sync::Arc; use core_api::user_fs::UserFs; +use skald_core::db::memory_docs; use skald_core::skald::Skald; use skald_core::latex::CompileError; use skald_core::tools::fs as fs_tools; @@ -141,6 +142,12 @@ pub struct FileQuery { /// `application/pdf`. Compilation failures yield `422 Unprocessable Entity` /// with the textual `latexmk` log in the body, so the caller can fall back to /// showing the raw source. +/// +/// A path under a virtual memory root (`user-memory/…`, `shared-memory/…`) is +/// served from the `memory_docs` table — the caller's own pool for the private +/// root, the system pool for the shared one — exactly like the fs-tools route +/// them (see [`fs_tools::classify_memory`]). Raw content only: no LaTeX +/// compilation (notes are not on disk). pub async fn get_file( State(state): State>, Extension(auth): Extension, @@ -150,6 +157,30 @@ pub async fn get_file( Ok(c) => c, Err(e) => return e.into_response(), }; + + // Virtual memory namespace → SQLite, not disk. + if let Some(mem) = fs_tools::classify_memory(&q.path) { + let pool = match mem.scope { + fs_tools::MemScope::User => Arc::clone(&ctx.pool), + fs_tools::MemScope::Shared => state.db().clone(), + }; + return match memory_docs::get(&pool, &mem.rel).await { + Ok(Some(doc)) => { + let mut response = doc.content.into_response(); + response.headers_mut().insert( + header::CONTENT_TYPE, + HeaderValue::from_static(content_type_for(&q.path)), + ); + if q.force_download { + set_attachment(&mut response, &basename(&q.path)); + } + response + } + Ok(None) => (StatusCode::NOT_FOUND, format!("File not found: {}", q.path)).into_response(), + Err(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), + }; + } + let user_fs = ctx.fs.load(); let abs = match fs_tools::resolve_view_path(user_fs.as_ref(), &q.path) { Ok((abs, _)) => abs, diff --git a/src/frontend/api/ws.rs b/src/frontend/api/ws.rs index 5d162e2..7538ea9 100644 --- a/src/frontend/api/ws.rs +++ b/src/frontend/api/ws.rs @@ -412,6 +412,8 @@ async fn handle_socket(mut socket: WebSocket, skald: Arc, source: String, Arc::clone(&chat_hub), source.clone(), ctx.fs.clone(), + ctx.pool.as_ref().clone(), + skald.db().as_ref().clone(), ), ], ..Default::default() diff --git a/web/components/projects/project-board.js b/web/components/projects/project-board.js index 37846f1..8aded50 100644 --- a/web/components/projects/project-board.js +++ b/web/components/projects/project-board.js @@ -222,17 +222,16 @@ export class ProjectBoardSection extends LightElement { _renderTabs() { const tab = (id, icon, label) => html` -
  • - -
    • + `; return html` -
      +
      ${tab('files', 'bi-folder2-open', t('projects.tabs.files'))} ${tab('sharing', 'bi-people', t('projects.tabs.sharing'))} -
    + `; } From 6f0461f7f557fea9c8800bfe2aa926dafe36730e Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 19:19:29 +0100 Subject: [PATCH 04/13] uploads: centralise via ChatHubApi::save_upload, refactor handlers Extract shared upload seam in skald-core, move Telegram and web handlers to use it. Simplify media attachment routing. Clean up unused deps and dead code. --- CLAUDE.md | 4 +- crates/core-api/src/chat_hub.rs | 16 +- crates/core-api/src/message_meta.rs | 7 +- crates/core-api/src/user_fs.rs | 6 + crates/plugin-telegram-bot/src/attachments.rs | 48 ++---- crates/plugin-telegram-bot/src/handlers.rs | 27 ++- crates/plugin-telegram-bot/src/lib.rs | 8 - crates/skald-core/src/chat_hub/mod.rs | 37 +++++ crates/skald-core/src/lib.rs | 1 + .../skald-core/src/session/handler/media.rs | 81 +++++---- .../src/session/handler/message_builder.rs | 10 +- crates/skald-core/src/session/handler/mod.rs | 9 +- crates/skald-core/src/uploads.rs | 154 ++++++++++++++++++ src/frontend/api/uploads.rs | 128 +++------------ src/frontend/server.rs | 28 +--- 15 files changed, 350 insertions(+), 214 deletions(-) create mode 100644 crates/skald-core/src/uploads.rs diff --git a/CLAUDE.md b/CLAUDE.md index ce18eed..58f0224 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -195,9 +195,9 @@ For a per-user connector whose credential is produced by **pairing** (`auth.type ## Multimodal attachments -Uploads (`POST /api/{source}/uploads`) are saved per-user under `data/uploads/{userid}/{session_id}/` (older rows may still reference the pre-namespacing `data/uploads/{session_id}/` layout — both stay readable), streamed to disk with a 256 MiB cap, with the sniffed magic-byte MIME preferred over the client claim; `/data/*` is served behind the same session-cookie gate as `/api`. Attachment metadata travels as structured JSON in `chat_history.metadata` — never as persisted text. +Uploads go through **one centralized seam** — `ChatHub::save_upload` (behind `ChatHubApi::save_upload`, backed by `skald_core::uploads::save_to_home`) — so every surface persists identically and no two callers can drift on placement (the class of bug where the agent was handed a path it couldn't reach). The seam writes into the **caller's container home** under `uploads/{session_id}/` (agent path `uploads/{session}/{name}`, the `UPLOADS_SUBDIR` const in `core-api/user_fs.rs`), collision-dedupes the name, and prefers the sniffed magic-byte MIME over the client claim. The **web** handler (`POST /api/{source}/uploads`) buffers each field with a 256 MiB cap then calls the seam; the **Telegram** plugin downloads bytes then calls the same seam via `handle.chat_hub().save_upload("telegram", …)`. Because the file lands in the home (bind-mounted at `/root`), it is reachable by the fs-tools, `execute_cmd`, and the file viewer (`GET /api/file`, per-user via `resolve_view_path`) — there is **no** `/data` static route anymore (removed: it was `require_auth`-only, not ownership-scoped, and also exposed internal server state under `data/`). Attachment metadata travels as structured JSON in `chat_history.metadata` — never as persisted text. -At context-build time (`MessageBuilder`), attachments of the **current turn** (the user/agent rows following the last completed assistant reply, including across in-flight tool rounds) are partitioned by `session/handler/media.rs`: when the resolved model's `LlmEntry.capabilities` include the modality (`vision` → `image_url` parts, `video` → `video_url` parts), the file is inlined as a base64 data-URL content part — but only if it canonicalizes under `data/uploads/`, its sniffed MIME is in the allowlist, and it fits the budgets (4 files / 10 MiB image / 32 MiB video / 48 MiB total per turn). Everything else — older turns, other kinds, any failed check — keeps the textual `[SYSTEM INFO]` path block, so a non-vision model produces a byte-identical payload to before. `OpenAiClient` forwards parts verbatim; `AnthropicClient` translates `image_url` data URLs to `image` blocks (video unsupported; Anthropic models get `vision` by editing the model row's capabilities — no catalog refresh writes them). On LLM fallback mid-round, messages are rebuilt with the replacement model's capabilities. +At context-build time (`MessageBuilder`), attachments of the **current turn** (the user/agent rows following the last completed assistant reply, including across in-flight tool rounds) are partitioned by `session/handler/media.rs`: when the resolved model's `LlmEntry.capabilities` include the modality (`vision` → `image_url` parts, `video` → `video_url` parts), the file is inlined as a base64 data-URL content part — but only if it resolves (through the caller's `UserFs`, via `resolve_host_path`) under the home's `uploads/` dir, its sniffed MIME is in the allowlist, and it fits the budgets (4 files / 10 MiB image / 32 MiB video / 48 MiB total per turn). Everything else — older turns, other kinds, any failed check — keeps the textual `[SYSTEM INFO]` path block, so a non-vision model produces a byte-identical payload to before. `OpenAiClient` forwards parts verbatim; `AnthropicClient` translates `image_url` data URLs to `image` blocks (video unsupported; Anthropic models get `vision` by editing the model row's capabilities — no catalog refresh writes them). On LLM fallback mid-round, messages are rebuilt with the replacement model's capabilities. ## Token streaming & reasoning display diff --git a/crates/core-api/src/chat_hub.rs b/crates/core-api/src/chat_hub.rs index a9f1849..280bc94 100644 --- a/crates/core-api/src/chat_hub.rs +++ b/crates/core-api/src/chat_hub.rs @@ -5,7 +5,7 @@ use tokio::sync::broadcast; use crate::events::GlobalEvent; use crate::interface_tool::InterfaceTool; -use crate::message_meta::MessageMetadata; +use crate::message_meta::{Attachment, MessageMetadata}; // ── SendMessageOptions ──────────────────────────────────────────────────────── @@ -59,6 +59,20 @@ pub trait ChatHubApi: Send + Sync { opts: SendMessageOptions, ) -> anyhow::Result<()>; + /// Persist an uploaded file for `source_id` into the owner's + /// `~/uploads/{session}/` and return its [`Attachment`] (home-relative agent + /// path). Channel adapters (e.g. the Telegram plugin) call this instead of + /// writing files themselves, so the core owns *where* uploads land and every + /// surface produces a path the agent can actually reach. The recognized + /// magic-byte MIME wins over the caller-claimed `client_mime`. + async fn save_upload( + &self, + source_id: &str, + file_name: &str, + client_mime: Option, + bytes: &[u8], + ) -> anyhow::Result; + /// Create a new session for the source, discarding the previous one. async fn clear(&self, source_id: &str) -> anyhow::Result; diff --git a/crates/core-api/src/message_meta.rs b/crates/core-api/src/message_meta.rs index e14c25f..cde40b1 100644 --- a/crates/core-api/src/message_meta.rs +++ b/crates/core-api/src/message_meta.rs @@ -12,9 +12,10 @@ use serde::{Deserialize, Serialize}; -/// One file attached by the user to a message. `path` is relative to the project -/// root (e.g. `data/uploads/123/file.pdf`) so it is both servable under `/data/…` -/// and resolvable by the filesystem tools. +/// One file attached by the user to a message. `path` is a home-relative agent +/// path (e.g. `uploads/123/file.pdf`) — the caller's container home is its root, +/// so the fs-tools, `execute_cmd`, the file viewer (`/api/file`) and the media +/// inliner all resolve it to the same physical file. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Attachment { pub path: String, diff --git a/crates/core-api/src/user_fs.rs b/crates/core-api/src/user_fs.rs index d8e8cd4..6cb57dc 100644 --- a/crates/core-api/src/user_fs.rs +++ b/crates/core-api/src/user_fs.rs @@ -22,6 +22,12 @@ use std::path::{Component, Path, PathBuf}; use std::sync::{Arc, RwLock}; +/// The subdirectory of a user's home where chat uploads are saved +/// (`{home}/uploads/{session_id}/…`, reachable by the agent as `uploads/…`). +/// Shared by the upload handler (write path) and the media inliner (containment +/// root) so the two anchors can never drift. +pub const UPLOADS_SUBDIR: &str = "uploads"; + /// One shared folder mounted into a user's container. #[derive(Debug, Clone)] pub struct SharedMount { diff --git a/crates/plugin-telegram-bot/src/attachments.rs b/crates/plugin-telegram-bot/src/attachments.rs index da67595..19269a5 100644 --- a/crates/plugin-telegram-bot/src/attachments.rs +++ b/crates/plugin-telegram-bot/src/attachments.rs @@ -1,7 +1,6 @@ use std::path::Path; use anyhow::Result; -use core_api::message_meta::Attachment; use teloxide::net::Download; use teloxide::prelude::*; @@ -10,9 +9,9 @@ use teloxide::prelude::*; /// # Extending /// Add a new variant here, then handle it in: /// 1. `handlers::classify_message` — detect the message type and build the variant -/// 2. `TelegramAttachment::download_and_save` — fetch bytes and persist to disk, -/// returning an [`Attachment`] -/// (return `Ok(None)` if no file is involved) +/// 2. `TelegramAttachment::download` — fetch the bytes (return `Ok(None)` if no +/// file is involved); the caller persists them +/// via the shared `ChatHubApi::save_upload` seam /// 3. `TelegramAttachment::system_info_message` — describe a file-less variant /// (Location) for the LLM pub(crate) enum TelegramAttachment { @@ -36,20 +35,16 @@ pub(crate) enum TelegramAttachment { } impl TelegramAttachment { - /// Downloads the attachment from Telegram, writes it to `base_dir//`, - /// and returns the saved [`Attachment`] (shared with the web/mobile path so the - /// copilot UI renders it identically). Returns `None` for attachment types that - /// carry no binary content (e.g. Location). - /// - /// The returned `path` is made relative to the process working directory (the - /// project root) when possible, so it is both servable under `/data/…` and - /// resolvable by the filesystem tools — matching web uploads. - pub(crate) async fn download_and_save( + /// Downloads the attachment's bytes from Telegram, returning + /// `(file_name, mimetype, bytes)`. Persistence is **not** done here: the caller + /// hands the bytes to the shared upload seam (`ChatHubApi::save_upload`), which + /// saves them into the user's home under `uploads/{session}/…` and produces the + /// [`Attachment`] — the same path every surface uses, so the agent can reach it. + /// Returns `None` for attachment types that carry no binary content (e.g. Location). + pub(crate) async fn download( &self, - bot: &Bot, - base_dir: &Path, - chat_id: i64, - ) -> Result> { + bot: &Bot, + ) -> Result, Vec)>> { let (file_id, file_name, mimetype): (&str, String, Option) = match self { Self::Document { file_id, file_name, mime_type, .. } => (file_id, file_name.clone(), mime_type.clone()), @@ -58,28 +53,11 @@ impl TelegramAttachment { Self::Location { .. } => return Ok(None), }; - let dir = base_dir.join(chat_id.to_string()); - tokio::fs::create_dir_all(&dir).await?; - let tg_file = bot.get_file(teloxide::types::FileId(file_id.to_string())).await?; let mut bytes = Vec::new(); bot.download_file(&tg_file.path, &mut bytes).await?; - let path = dir.join(&file_name); - tokio::fs::write(&path, &bytes).await?; - - // Prefer a project-root-relative path so `/data/…` serving works. - let rel = std::env::current_dir() - .ok() - .and_then(|cwd| path.strip_prefix(&cwd).ok().map(Path::to_path_buf)) - .unwrap_or_else(|| path.clone()); - - Ok(Some(Attachment { - path: rel.to_string_lossy().to_string(), - name: file_name, - mimetype, - filesize: Some(bytes.len() as u64), - })) + Ok(Some((file_name, mimetype, bytes))) } /// Builds the `[TELEGRAM SYSTEM INFO]` message injected into the conversation history. diff --git a/crates/plugin-telegram-bot/src/handlers.rs b/crates/plugin-telegram-bot/src/handlers.rs index 6942c25..8f992d1 100644 --- a/crates/plugin-telegram-bot/src/handlers.rs +++ b/crates/plugin-telegram-bot/src/handlers.rs @@ -512,17 +512,32 @@ async fn handle_attachment( bot.send_chat_action(chat_id, ChatAction::UploadDocument).await.ok(); - let saved = match attachment.download_and_save(&bot, &shared.uploads_dir, chat_id.0).await { - Ok(s) => s, + let downloaded = match attachment.download(&bot).await { + Ok(d) => d, Err(e) => { - error!(error = %e, "telegram: failed to save attachment"); - bot.send_message(chat_id, "⚠️ Could not save the attachment.").await.ok(); + error!(error = %e, "telegram: failed to download attachment"); + bot.send_message(chat_id, "⚠️ Could not download the attachment.").await.ok(); return; } }; - match saved { - Some(att) => { + match downloaded { + Some((file_name, mimetype, bytes)) => { + // Persist through the shared upload seam so the file lands in the user's + // home (`uploads/{session}/…`) with an agent-reachable path — identical + // to a web upload. + let att = match handle + .chat_hub() + .save_upload("telegram", &file_name, mimetype, &bytes) + .await + { + Ok(a) => a, + Err(e) => { + error!(error = %e, "telegram: failed to save attachment"); + bot.send_message(chat_id, "⚠️ Could not save the attachment.").await.ok(); + return; + } + }; info!(chat_id = chat_id.0, path = %att.path, "telegram: attachment saved, forwarding to LLM"); let caption = match &attachment { TelegramAttachment::Document { caption, .. } => caption.clone(), diff --git a/crates/plugin-telegram-bot/src/lib.rs b/crates/plugin-telegram-bot/src/lib.rs index 5820fe8..bc3adb1 100644 --- a/crates/plugin-telegram-bot/src/lib.rs +++ b/crates/plugin-telegram-bot/src/lib.rs @@ -26,7 +26,6 @@ /// `ApprovalApi`. use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; @@ -106,7 +105,6 @@ pub(crate) struct TgShared { pub(crate) transcribe: Arc, pub(crate) tts: Arc, pub(crate) location: Arc, - pub(crate) uploads_dir: PathBuf, // ── Pairing / bindings (config-table-backed, cached in memory) ── pub(crate) bindings: RwLock, @@ -272,11 +270,6 @@ impl Plugin for TelegramPlugin { anyhow::bail!("telegram: token is empty — set it via the plugins API"); } - let uploads_dir = std::env::current_dir() - .unwrap_or_default() - .join("uploads") - .join("telegram"); - // Load bindings from the config table (or default if absent). let telegram_config = auth::load_config(&*ctx.config).await .unwrap_or_default(); @@ -293,7 +286,6 @@ impl Plugin for TelegramPlugin { transcribe: Arc::clone(&ctx.transcribe), tts: Arc::clone(&ctx.tts_provider), location: Arc::clone(&ctx.location), - uploads_dir, bindings: RwLock::new(telegram_config), pending_approvals: Mutex::new(HashMap::new()), pending_questions: Mutex::new(HashMap::new()), diff --git a/crates/skald-core/src/chat_hub/mod.rs b/crates/skald-core/src/chat_hub/mod.rs index ce9e54b..a5daf7d 100644 --- a/crates/skald-core/src/chat_hub/mod.rs +++ b/crates/skald-core/src/chat_hub/mod.rs @@ -4,6 +4,7 @@ use std::sync::{Arc, OnceLock, Weak}; use std::time::Duration; use async_trait::async_trait; +use core_api::message_meta::Attachment; use sqlx::SqlitePool; use tokio::sync::{Mutex, broadcast, mpsc}; use tokio_util::sync::CancellationToken; @@ -234,6 +235,32 @@ impl ChatHub { self.session_mgr.get_or_create_handler(session_id).await } + /// Persist an uploaded file for `source_id` into the owner's home + /// (`~/uploads/{session}/`) and return its [`Attachment`]. The single entry + /// point every surface (web handler, channel plugins) routes through, so + /// uploads can't drift on placement or on the recorded agent path — see + /// [`crate::uploads::save_to_home`]. Resolves the source's active session (so + /// the upload shares the directory the following message references). + pub async fn save_upload( + &self, + source_id: &str, + file_name: &str, + client_mime: Option, + bytes: &[u8], + ) -> anyhow::Result { + let handler = self.session_handler(source_id).await?; + let fs = handler.user_fs(); + let att = crate::uploads::save_to_home( + &fs, + handler.session_id, + file_name, + client_mime, + bytes, + ) + .await?; + Ok(att) + } + /// Returns the handler for a specific `session_id`, creating one lazily if needed. /// Used to resolve a pending tool against the session that actually owns it, /// independent of any source's "active" session. @@ -781,6 +808,16 @@ impl ChatHubApi for ChatHub { self.send_message(source_id, prompt, opts).await } + async fn save_upload( + &self, + source_id: &str, + file_name: &str, + client_mime: Option, + bytes: &[u8], + ) -> anyhow::Result { + self.save_upload(source_id, file_name, client_mime, bytes).await + } + async fn clear(&self, source_id: &str) -> anyhow::Result { self.clear(source_id).await } diff --git a/crates/skald-core/src/lib.rs b/crates/skald-core/src/lib.rs index 6c20a0f..044e754 100644 --- a/crates/skald-core/src/lib.rs +++ b/crates/skald-core/src/lib.rs @@ -48,4 +48,5 @@ pub mod tool_discovery; pub mod tools; pub mod transcribe; pub mod tts; +pub mod uploads; pub mod users; diff --git a/crates/skald-core/src/session/handler/media.rs b/crates/skald-core/src/session/handler/media.rs index 7894614..77480fd 100644 --- a/crates/skald-core/src/session/handler/media.rs +++ b/crates/skald-core/src/session/handler/media.rs @@ -10,8 +10,9 @@ //! Promotion is deliberately strict: an attachment is inlined only when ALL of //! these hold — //! - the model has the modality's capability; -//! - the file lives under `data/uploads/`, canonicalized (attachments saved -//! anywhere else, e.g. by the Telegram plugin, stay textual); +//! - the file lives under the caller's `~/uploads/` (where the upload handler +//! saves it), resolved through their per-user filesystem — attachments stored +//! anywhere else stay textual; //! - the sniffed magic bytes match an allowed MIME — the client-supplied //! `mimetype` is never trusted; //! - the per-file and per-turn byte/count budgets are not exhausted. @@ -26,7 +27,7 @@ use tracing::debug; use core_api::message_meta::Attachment; use core_api::tool::MediaRef; -use core_api::user_fs::UserFs; +use core_api::user_fs::{UserFs, UPLOADS_SUBDIR}; /// Max media parts inlined per turn. const MAX_MEDIA_PER_TURN: usize = 4; @@ -108,22 +109,21 @@ pub struct MediaPartition { } /// Splits a message's attachments into inline media parts and leftovers. -/// Files are resolved against the process working directory. -pub async fn partition(attachments: &[Attachment], capabilities: &[String]) -> MediaPartition { - let base = std::env::current_dir().unwrap_or_default(); - partition_under(attachments, capabilities, &base).await -} - -/// [`partition`] with an explicit base directory (tests). -pub async fn partition_under( +/// +/// Each attachment path is resolved through the caller's per-user [`UserFs`] — +/// the same resolver the fs-tools use, fail-closed on traversal / workspace +/// escape — and inlined only when it lands under their `~/uploads/` directory, +/// where the upload handler saves them. Attachments stored anywhere else (a +/// path outside the home, or another surface's directory) stay textual. +pub async fn partition( attachments: &[Attachment], capabilities: &[String], - base: &Path, + fs: &UserFs, ) -> MediaPartition { let capable = MODALITIES .iter() .any(|m| capabilities.iter().any(|c| c == m.capability)); - let root = std::fs::canonicalize(base.join("data").join("uploads")).ok(); + let root = std::fs::canonicalize(fs.home_host.join(UPLOADS_SUBDIR)).ok(); if !capable || root.is_none() { return MediaPartition { parts: Vec::new(), rest: attachments.to_vec() }; } @@ -138,7 +138,7 @@ pub async fn partition_under( rest.push(a.clone()); continue; } - match try_inline(a, capabilities, base, &root, total).await { + match try_inline(a, capabilities, fs, &root, total).await { Some((part, bytes)) => { total += bytes; parts.push(part); @@ -150,16 +150,17 @@ pub async fn partition_under( } /// Promotes one uploaded attachment to a content part, or `None` when any check -/// fails (logged at debug level; the caller keeps it on the textual path). -/// Containment is against the uploads `root`; the rest is [`promote`]. +/// fails (logged at debug level; the caller keeps it on the textual path). The +/// agent path is resolved through the per-user filesystem (fail-closed) and then +/// re-checked to land under the uploads `root`; the rest is [`promote`]. async fn try_inline( a: &Attachment, capabilities: &[String], - base: &Path, + fs: &UserFs, root: &Path, used_total: u64, ) -> Option<(Value, u64)> { - let abs = tokio::fs::canonicalize(base.join(&a.path)).await.ok()?; + let abs = crate::tools::fs::resolve_host_path(fs, &a.path).ok()?; if !abs.starts_with(root) { debug!(path = %a.path, "media not inlined: outside the uploads root"); return None; @@ -205,7 +206,7 @@ async fn promote( } /// Inline media a tool produced (e.g. `read_file` on an image) as content parts, -/// for the current turn only. Mirrors [`partition_under`] but contains against the +/// for the current turn only. Mirrors [`partition`] but contains against the /// caller's **workspace roots** (home + shared + projects + docs) rather than the /// uploads dir — the tool already resolved + contained the path, so this is a /// fail-closed re-check against a symlink swap since the read (§6). Same per-file, @@ -395,11 +396,13 @@ mod tests { #[tokio::test] async fn partition_inlines_png_for_vision_model() { let tmp = std::env::temp_dir().join(format!("skald-media-{}", uuid::Uuid::new_v4())); - let dir = tmp.join("data/uploads/u/1"); + let home = tmp.join("homes/u1"); + let dir = home.join("uploads/1"); tokio::fs::create_dir_all(&dir).await.unwrap(); tokio::fs::write(dir.join("a.png"), png_bytes()).await.unwrap(); + let fs = fs_home(&home); - let p = partition_under(&[att("data/uploads/u/1/a.png")], &caps(&["vision"]), &tmp).await; + let p = partition(&[att("uploads/1/a.png")], &caps(&["vision"]), &fs).await; assert!(p.rest.is_empty()); assert_eq!(p.parts.len(), 1); let url = p.parts[0]["image_url"]["url"].as_str().unwrap(); @@ -411,27 +414,30 @@ mod tests { #[tokio::test] async fn partition_gates_on_capability_and_containment() { let tmp = std::env::temp_dir().join(format!("skald-media-{}", uuid::Uuid::new_v4())); - let dir = tmp.join("data/uploads/u/1"); + let home = tmp.join("homes/u1"); + let dir = home.join("uploads/1"); tokio::fs::create_dir_all(&dir).await.unwrap(); tokio::fs::write(dir.join("a.png"), png_bytes()).await.unwrap(); - tokio::fs::write(tmp.join("secret.png"), png_bytes()).await.unwrap(); + // A real image inside the home but OUTSIDE the uploads dir. + tokio::fs::write(home.join("secret.png"), png_bytes()).await.unwrap(); + let fs = fs_home(&home); // No capability → everything stays textual. - let p = partition_under(&[att("data/uploads/u/1/a.png")], &caps(&[]), &tmp).await; + let p = partition(&[att("uploads/1/a.png")], &caps(&[]), &fs).await; assert_eq!(p.rest.len(), 1); assert!(p.parts.is_empty()); // vision capability does not unlock video parts. - let p = partition_under(&[att("data/uploads/u/1/a.png")], &caps(&["video"]), &tmp).await; + let p = partition(&[att("uploads/1/a.png")], &caps(&["video"]), &fs).await; assert_eq!(p.rest.len(), 1); - // A real image outside the uploads root is never read inline. - let p = partition_under(&[att("secret.png")], &caps(&["vision"]), &tmp).await; + // A real image in the home but outside the uploads dir is never inlined. + let p = partition(&[att("secret.png")], &caps(&["vision"]), &fs).await; assert_eq!(p.rest.len(), 1); assert!(p.parts.is_empty()); - // Traversal out of the root is rejected. - let p = partition_under(&[att("data/uploads/../../secret.png")], &caps(&["vision"]), &tmp).await; + // Traversal out of the workspace is rejected fail-closed. + let p = partition(&[att("uploads/../../secret.png")], &caps(&["vision"]), &fs).await; assert_eq!(p.rest.len(), 1); assert!(p.parts.is_empty()); @@ -441,15 +447,16 @@ mod tests { #[tokio::test] async fn partition_enforces_count_budget() { let tmp = std::env::temp_dir().join(format!("skald-media-{}", uuid::Uuid::new_v4())); - let dir = tmp.join("data/uploads/u/1"); + let home = tmp.join("homes/u1"); + let dir = home.join("uploads/1"); tokio::fs::create_dir_all(&dir).await.unwrap(); let mut atts = Vec::new(); for i in 0..(MAX_MEDIA_PER_TURN + 2) { - let rel = format!("data/uploads/u/1/{i}.png"); tokio::fs::write(dir.join(format!("{i}.png")), png_bytes()).await.unwrap(); - atts.push(att(&rel)); + atts.push(att(&format!("uploads/1/{i}.png"))); } - let p = partition_under(&atts, &caps(&["vision"]), &tmp).await; + let fs = fs_home(&home); + let p = partition(&atts, &caps(&["vision"]), &fs).await; assert_eq!(p.parts.len(), MAX_MEDIA_PER_TURN); assert_eq!(p.rest.len(), 2); @@ -465,12 +472,14 @@ mod tests { #[tokio::test] async fn partition_inlines_pdf_as_file_part_for_document_model() { let tmp = std::env::temp_dir().join(format!("skald-media-{}", uuid::Uuid::new_v4())); - let dir = tmp.join("data/uploads/u/1"); + let home = tmp.join("homes/u1"); + let dir = home.join("uploads/1"); tokio::fs::create_dir_all(&dir).await.unwrap(); tokio::fs::write(dir.join("a.pdf"), pdf_bytes()).await.unwrap(); + let fs = fs_home(&home); // A document-capable model inlines the PDF as the OpenAI `file` part shape. - let p = partition_under(&[att("data/uploads/u/1/a.pdf")], &caps(&["document"]), &tmp).await; + let p = partition(&[att("uploads/1/a.pdf")], &caps(&["document"]), &fs).await; assert!(p.rest.is_empty()); assert_eq!(p.parts.len(), 1); assert_eq!(p.parts[0]["type"], "file"); @@ -479,7 +488,7 @@ mod tests { assert!(fd.starts_with("data:application/pdf;base64,"), "{fd}"); // vision alone does not unlock PDFs. - let p = partition_under(&[att("data/uploads/u/1/a.pdf")], &caps(&["vision"]), &tmp).await; + let p = partition(&[att("uploads/1/a.pdf")], &caps(&["vision"]), &fs).await; assert_eq!(p.rest.len(), 1); assert!(p.parts.is_empty()); diff --git a/crates/skald-core/src/session/handler/message_builder.rs b/crates/skald-core/src/session/handler/message_builder.rs index 0667c49..1952a28 100644 --- a/crates/skald-core/src/session/handler/message_builder.rs +++ b/crates/skald-core/src/session/handler/message_builder.rs @@ -258,8 +258,14 @@ impl MessageBuilder { // textual path block, generated on the fly and never // persisted as content. let (text, media) = match &entry.metadata { - Some(meta) if !meta.attachments.is_empty() && idx >= media_turn_start => { - let partition = super::media::partition(&meta.attachments, capabilities).await; + Some(meta) + if !meta.attachments.is_empty() + && idx >= media_turn_start + && self.fs.is_some() => + { + let fs = self.fs.as_deref().expect("guarded by is_some()"); + let partition = + super::media::partition(&meta.attachments, capabilities, fs).await; ( format!( "{}{}", diff --git a/crates/skald-core/src/session/handler/mod.rs b/crates/skald-core/src/session/handler/mod.rs index 829c274..59b2e18 100644 --- a/crates/skald-core/src/session/handler/mod.rs +++ b/crates/skald-core/src/session/handler/mod.rs @@ -20,7 +20,7 @@ use crate::config::DatetimeConfig; use crate::db::{chat_history, chat_sessions_stack}; use crate::events::ServerEvent; use core_api::message_meta::MessageMetadata; -use core_api::user_fs::SharedFs; +use core_api::user_fs::{SharedFs, UserFs}; use crate::llm::LlmManager; use crate::mcp::McpProvider; use crate::image_generate::ImageGeneratorManager; @@ -414,6 +414,13 @@ impl ChatSessionHandler { } } + /// The caller's current filesystem snapshot (home + shared folders + projects + /// + docs). Cheap — clones an `Arc`. Used by upload persistence to place files + /// in the owner's home. + pub fn user_fs(&self) -> Arc { + self.fs.load() + } + /// Override the session used for scratchpad reads/writes. /// Called by the cron runner for async tasks so they share the parent's scratchpad. pub fn set_scratchpad_session_id(&self, id: i64) { diff --git a/crates/skald-core/src/uploads.rs b/crates/skald-core/src/uploads.rs new file mode 100644 index 0000000..910b9c2 --- /dev/null +++ b/crates/skald-core/src/uploads.rs @@ -0,0 +1,154 @@ +//! Centralized upload persistence. +//! +//! The single place any surface — the web `POST /uploads` handler, a channel +//! plugin like Telegram, or a future one — turns received file bytes into a +//! saved [`Attachment`]. Keeping placement + naming + metadata here means no two +//! callers can drift on *where* an upload lands or *what* path is recorded (the +//! bug this fixes: uploads that the agent then couldn't reach). +//! +//! Files are written into the user's private container home under +//! `uploads/{session_id}/…`. That single agent path is resolved identically by +//! every consumer — the fs-tools, `execute_cmd` (the home is bind-mounted at +//! `/root`), the file viewer (`/api/file`) and the media inliner — through the +//! same per-user [`UserFs`]. + +use std::path::{Path, PathBuf}; + +use core_api::message_meta::Attachment; +use core_api::user_fs::{UserFs, UPLOADS_SUBDIR}; + +use crate::session::handler::media::sniff_mime; + +/// Persist `bytes` as a file named `file_name` into the user's +/// `~/uploads/{session_id}/`, returning the resulting [`Attachment`] whose `path` +/// is the home-relative agent path. The recognized magic-byte MIME wins over the +/// caller-claimed `client_mime`. +/// +/// The caller owns byte transport (streaming a multipart body, downloading from +/// an API) and any size cap; this owns placement, collision-safe naming, MIME +/// sniffing and the metadata shape. +pub async fn save_to_home( + fs: &UserFs, + session_id: i64, + file_name: &str, + client_mime: Option, + bytes: &[u8], +) -> std::io::Result { + let dir_host = fs + .home_host + .join(UPLOADS_SUBDIR) + .join(session_id.to_string()); + tokio::fs::create_dir_all(&dir_host).await?; + + let (abs_path, final_name) = unique_target(&dir_host, &sanitize_filename(file_name)); + tokio::fs::write(&abs_path, bytes).await?; + + // The sniffed type wins over the client claim when we recognize the bytes. + let mimetype = sniff_mime(&bytes[..bytes.len().min(16)]) + .map(str::to_string) + .or(client_mime); + + Ok(Attachment { + path: format!("{UPLOADS_SUBDIR}/{session_id}/{final_name}"), + name: final_name, + mimetype, + filesize: Some(bytes.len() as u64), + }) +} + +/// Reduces an arbitrary client filename to a safe basename: directory components +/// are dropped and an empty/`.`/`..` result falls back to `"file"`. +fn sanitize_filename(raw: &str) -> String { + let base = Path::new(raw) + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("") + .trim(); + if base.is_empty() || base == "." || base == ".." { + "file".to_string() + } else { + base.to_string() + } +} + +/// Returns a non-colliding `(absolute_path, final_name)` inside `dir`. If `name` +/// already exists, inserts `_1`, `_2`, … before the extension. +fn unique_target(dir: &Path, name: &str) -> (PathBuf, String) { + let candidate = dir.join(name); + if !candidate.exists() { + return (candidate, name.to_string()); + } + let path = Path::new(name); + let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(name); + let ext = path.extension().and_then(|s| s.to_str()); + for n in 1.. { + let next = match ext { + Some(ext) => format!("{stem}_{n}.{ext}"), + None => format!("{stem}_{n}"), + }; + let candidate = dir.join(&next); + if !candidate.exists() { + return (candidate, next); + } + } + unreachable!("unique_target loop always returns") +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A throwaway [`UserFs`] whose private home is `home`. + fn fs_home(home: &Path) -> UserFs { + UserFs::new( + "u1", + home.to_path_buf(), + "skald-u1", + PathBuf::from("/root"), + vec![], + vec![], + None, + ) + } + + fn pdf_bytes() -> Vec { + let mut v = b"%PDF-1.7\n".to_vec(); + v.extend_from_slice(&[0x00; 32]); + v + } + + #[tokio::test] + async fn saves_into_home_uploads_with_agent_path_and_sniffs_mime() { + let tmp = std::env::temp_dir().join(format!("skald-upload-{}", uuid::Uuid::new_v4())); + let home = tmp.join("homes/u1"); + tokio::fs::create_dir_all(&home).await.unwrap(); + let fs = fs_home(&home); + + // A wrong client MIME is overridden by the sniffed PDF signature. + let att = save_to_home(&fs, 7, "cv.pdf", Some("application/octet-stream".into()), &pdf_bytes()) + .await + .unwrap(); + + assert_eq!(att.path, "uploads/7/cv.pdf"); + assert_eq!(att.name, "cv.pdf"); + assert_eq!(att.mimetype.as_deref(), Some("application/pdf")); + assert_eq!(att.filesize, Some(pdf_bytes().len() as u64)); + // Physically lands under the home's uploads dir (reachable by the agent). + assert!(home.join("uploads/7/cv.pdf").exists()); + + // A second upload of the same name never overwrites — it is de-duped. + let att2 = save_to_home(&fs, 7, "cv.pdf", None, &pdf_bytes()).await.unwrap(); + assert_eq!(att2.path, "uploads/7/cv_1.pdf"); + assert!(home.join("uploads/7/cv_1.pdf").exists()); + + let _ = tokio::fs::remove_dir_all(&tmp).await; + } + + #[test] + fn sanitize_strips_directory_components() { + assert_eq!(sanitize_filename("../../etc/passwd"), "passwd"); + assert_eq!(sanitize_filename("a/b/c.txt"), "c.txt"); + assert_eq!(sanitize_filename(".."), "file"); + assert_eq!(sanitize_filename(""), "file"); + } +} diff --git a/src/frontend/api/uploads.rs b/src/frontend/api/uploads.rs index fced831..f75a42d 100644 --- a/src/frontend/api/uploads.rs +++ b/src/frontend/api/uploads.rs @@ -1,36 +1,36 @@ -use std::path::{Path as StdPath, PathBuf}; use std::sync::Arc; use axum::{ Json, Extension, extract::{Multipart, Path, State}, }; -use tokio::io::AsyncWriteExt; use core_api::message_meta::Attachment; -use skald_core::session::handler::media::sniff_mime; use skald_core::skald::Skald; -use skald_core::tools::fs as fs_tools; use super::{ApiError, guard::AuthUser, require_context}; use super::sessions::SourcePath; -/// Max bytes accepted for a single uploaded file; anything larger is cut off -/// mid-stream, the partial file removed, and the request answered 413. +/// Max bytes accepted for a single uploaded file; anything larger is refused 413. const MAX_UPLOAD_BYTES: u64 = 256 * 1024 * 1024; /// `POST /api/{source}/uploads` /// -/// Accepts a `multipart/form-data` body with one or more file fields and saves -/// each under `data/uploads/{user_id}/{session_id}/` (per-user namespaced, so -/// colliding session ids across users never share a directory). Bytes are -/// streamed straight to disk (`field.chunk()` → file), never buffered whole in -/// RAM — the route disables the default body-size limit (see router) and -/// enforces [`MAX_UPLOAD_BYTES`] itself. When the magic bytes are recognized, -/// the sniffed MIME wins over the client-supplied `Content-Type`. +/// Accepts a `multipart/form-data` body with one or more file fields and persists +/// each through the shared upload seam ([`skald_core::chat_hub::ChatHub::save_upload`]), +/// which saves into the caller's container home under `uploads/{session_id}/` — +/// so a single agent path (`uploads/{session_id}/…`) is reachable by the fs-tools, +/// by `execute_cmd` (the home is bind-mounted at `/root`), and by the file viewer +/// alike. The web handler and every channel plugin go through that one seam, so no +/// two surfaces can drift on *where* an upload lands. /// -/// Returns the saved [`Attachment`]s (project-root-relative path, name, MIME, -/// size) so the client can show chips and echo them back when sending the message. +/// Each field is read with the [`MAX_UPLOAD_BYTES`] cap enforced during accumulation +/// (an over-cap field is refused before anything is written); the route disables the +/// default body-size limit (see router). The seam sniffs the magic bytes and prefers +/// them over the client-supplied `Content-Type`. +/// +/// Returns the saved [`Attachment`]s (home-relative agent path, name, MIME, size) so +/// the client can show chips and echo them back when sending the message. pub async fn upload( State(skald): State>, Extension(auth): Extension, @@ -38,13 +38,6 @@ pub async fn upload( mut multipart: Multipart, ) -> Result>, ApiError> { let ctx = require_context(&skald, &auth.user_id).await?; - // Resolve (creating if needed) the source's session so uploads land in the - // directory the message will reference. - let session_id = ctx.chat_hub.session_handler(&p.source).await?.session_id; - - let dir_rel = format!("data/uploads/{}/{session_id}", auth.user_id); - let dir_abs = fs_tools::resolve(&dir_rel)?; - tokio::fs::create_dir_all(&dir_abs).await?; let mut saved: Vec = Vec::new(); @@ -53,93 +46,26 @@ pub async fn upload( { // Only fields carrying a filename are file uploads; skip plain text fields. let Some(orig_name) = field.file_name().map(str::to_string) else { continue }; - let mimetype = field.content_type().map(str::to_string); + let client_mime = field.content_type().map(str::to_string); - let base_name = sanitize_filename(&orig_name); - let (abs_path, final_name) = unique_target(&dir_abs, &base_name); - - let mut file = tokio::fs::File::create(&abs_path).await - .map_err(|e| ApiError::from(anyhow::anyhow!("cannot create {}: {e}", abs_path.display())))?; - - let mut size: u64 = 0; - let mut too_large = false; + // Buffer the field, enforcing the size cap as we read so an over-limit + // upload is refused before any bytes are handed to the store. + let mut bytes: Vec = Vec::new(); while let Some(chunk) = field.chunk().await .map_err(|e| ApiError::bad_request(format!("upload read error: {e}")))? { - size += chunk.len() as u64; - if size > MAX_UPLOAD_BYTES { - too_large = true; - break; + if bytes.len() as u64 + chunk.len() as u64 > MAX_UPLOAD_BYTES { + return Err(ApiError::payload_too_large(format!( + "'{orig_name}' exceeds the {} MiB upload limit", + MAX_UPLOAD_BYTES / 1024 / 1024 + ))); } - file.write_all(&chunk).await?; - } - file.flush().await?; - drop(file); - - if too_large { - let _ = tokio::fs::remove_file(&abs_path).await; - return Err(ApiError::payload_too_large(format!( - "'{final_name}' exceeds the {} MiB upload limit", - MAX_UPLOAD_BYTES / 1024 / 1024 - ))); + bytes.extend_from_slice(&chunk); } - // The sniffed type wins over the client claim when we recognize the bytes. - let mimetype = sniff_head(&abs_path).await.map(String::from).or(mimetype); - - saved.push(Attachment { - path: format!("{dir_rel}/{final_name}"), - name: final_name, - mimetype, - filesize: Some(size), - }); + let att = ctx.chat_hub.save_upload(&p.source, &orig_name, client_mime, &bytes).await?; + saved.push(att); } Ok(Json(saved)) } - -/// Reads the first bytes of a saved upload and sniffs its real media type. -async fn sniff_head(path: &StdPath) -> Option<&'static str> { - let mut file = tokio::fs::File::open(path).await.ok()?; - let mut head = [0u8; 16]; - let n = tokio::io::AsyncReadExt::read(&mut file, &mut head).await.ok()?; - sniff_mime(&head[..n]) -} - -/// Reduces an arbitrary client filename to a safe basename: directory components -/// are dropped and an empty/`.`/`..` result falls back to `"file"`. -fn sanitize_filename(raw: &str) -> String { - let base = StdPath::new(raw) - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("") - .trim(); - if base.is_empty() || base == "." || base == ".." { - "file".to_string() - } else { - base.to_string() - } -} - -/// Returns a non-colliding `(absolute_path, final_name)` inside `dir`. If `name` -/// already exists, inserts `_1`, `_2`, … before the extension. -fn unique_target(dir: &StdPath, name: &str) -> (PathBuf, String) { - let candidate = dir.join(name); - if !candidate.exists() { - return (candidate, name.to_string()); - } - let path = StdPath::new(name); - let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or(name); - let ext = path.extension().and_then(|s| s.to_str()); - for n in 1.. { - let next = match ext { - Some(ext) => format!("{stem}_{n}.{ext}"), - None => format!("{stem}_{n}"), - }; - let candidate = dir.join(&next); - if !candidate.exists() { - return (candidate, next); - } - } - unreachable!("unique_target loop always returns") -} diff --git a/src/frontend/server.rs b/src/frontend/server.rs index 920e74a..2bf0af0 100644 --- a/src/frontend/server.rs +++ b/src/frontend/server.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::sync::Arc; use anyhow::{Context, Result}; @@ -78,7 +77,6 @@ impl WebServer { Arc::clone(&skald), api::guard::require_auth, )); - let skald_for_data = Arc::clone(&skald); // Resolve the app state first so the resulting `Router<()>` can host the // stateless plugin routers via `nest`. @@ -108,29 +106,21 @@ impl WebServer { )); router = router.nest(&format!("/api/plugin/{id}"), gated); } - // Serve the data/ directory under /data/ (accessible via URL), behind the - // same session-cookie gate as /api — uploads are private user content. - let data_dir = Path::new(static_dir).parent().unwrap_or(Path::new(".")).join("data"); - // Static responses (SPA assets + /data) get `Cache-Control: no-cache`: - // the browser may store them but MUST revalidate before use, so after a - // self-rewrite/restart the client never serves a stale asset (no heuristic + // User files are never served as static content: chat uploads live in the + // caller's container home and are fetched, per-user and access-checked, + // through `/api/file`. (The former `/data` static mount was removed — it + // was gated by `require_auth` only, not ownership, so it also exposed + // internal server state under `data/`.) + // + // Static responses (the SPA assets) get `Cache-Control: no-cache`: the + // browser may store them but MUST revalidate before use, so after a + // rebuild/restart the client never serves a stale asset (no heuristic // caching). Revalidation yields cheap 304s (the body is already on disk). // `/api` is deliberately left without this header (dynamic, not cached). let static_assets = || ServiceBuilder::new().layer(SetResponseHeaderLayer::overriding( header::CACHE_CONTROL, HeaderValue::from_static("no-cache"), )); - let data_service = ServiceBuilder::new() - .layer(axum::middleware::from_fn_with_state( - skald_for_data, - api::guard::require_auth, - )) - .layer(SetResponseHeaderLayer::overriding( - header::CACHE_CONTROL, - HeaderValue::from_static("no-cache"), - )) - .service(ServeDir::new(&data_dir)); - router = router.nest_service("/data", data_service); router = router.fallback_service(static_assets().service(ServeDir::new(static_dir))); // Negotiated gzip/brotli compression (Accept-Encoding). Matters most for // the mobile WebView, whose HTTP traffic is reverse-proxied byte-for-byte From d5f80dfcb3bd6e082f0545d2b90364409ea98c16 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 19:33:12 +0100 Subject: [PATCH 05/13] container: improve mount reconciliation and error handling --- crates/skald-core/src/container/mod.rs | 59 ++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/crates/skald-core/src/container/mod.rs b/crates/skald-core/src/container/mod.rs index e47e26d..e8e1f46 100644 --- a/crates/skald-core/src/container/mod.rs +++ b/crates/skald-core/src/container/mod.rs @@ -55,6 +55,16 @@ pub const CONTAINER_HOME: &str = "/root"; /// before force-killing — enough for a shell or MCP `docker exec` child to exit. const STOP_GRACE: Duration = Duration::from_secs(10); +/// Grace window at **app shutdown** ([`ContainerManager::stop_all`]). Deliberately +/// short: a healthy container (tini as PID 1, via `--init`) exits within ~100 ms of +/// SIGTERM, so this only bounds the pathological case — an old container whose PID 1 +/// is `sleep infinity` (created before `--init`) ignores SIGTERM entirely (the kernel +/// applies no default signal disposition to PID 1) and would otherwise burn the full +/// 10 s `STOP_GRACE` before SIGKILL, once **per user, in sequence**. `ensure`'s init +/// self-heal recreates such containers with tini on the next boot; this cap keeps the +/// shutdown fast in the meantime. +const SHUTDOWN_STOP_GRACE: Duration = Duration::from_secs(2); + /// The deterministic container name for a user — derivable without any manager, /// so `UserFs` can carry it and `execute_cmd` can exec into it directly. pub fn container_name(user_id: &str) -> String { @@ -203,16 +213,19 @@ impl ContainerManager { let want_user = host_uid_gid().map(|(uid, gid)| format!("{uid}:{gid}")); match container_state(name).await { - // Reuse only if it runs as the expected user; otherwise recreate below. - ContainerState::Running if user_matches(name, &want_user).await => return Ok(()), - ContainerState::Stopped if user_matches(name, &want_user).await => { + // Reuse only if it runs as the expected user AND has tini as PID 1; + // otherwise recreate below. + ContainerState::Running if reusable(name, &want_user).await => return Ok(()), + ContainerState::Stopped if reusable(name, &want_user).await => { docker(&["start", name]).await.context("docker start failed")?; return Ok(()); } ContainerState::Absent => {} - // Present but with a stale `--user` (e.g. an old root container): tear it - // down. The container holds no durable state — everything is in the bind - // mounts — so a recreate is safe. + // Present but stale — a mismatched `--user` (e.g. an old root container) or + // missing `--init` (an old container whose PID 1 is `sleep infinity`, which + // ignores SIGTERM and hangs `docker stop` for the full grace, see + // `SHUTDOWN_STOP_GRACE`): tear it down. The container holds no durable state + // — everything is in the bind mounts — so a recreate is safe. _ => { let _ = docker(&["rm", "-f", name]).await; } @@ -266,14 +279,25 @@ impl ContainerManager { } /// Stops every user's container (best-effort) at shutdown. + /// + /// Stops run **concurrently** and with a short [`SHUTDOWN_STOP_GRACE`], so total + /// shutdown time is bounded by one grace window regardless of how many users there + /// are — not `N × 10 s` as the old sequential, default-grace loop was (a pre-`--init` + /// container ignores SIGTERM and burns the full grace before SIGKILL). pub async fn stop_all(&self) -> Result<()> { let users = db::users::list(&self.system).await?; + let secs = SHUTDOWN_STOP_GRACE.as_secs().to_string(); + let mut set = tokio::task::JoinSet::new(); for user in &users { let name = container_name(&user.id); - if let Err(e) = docker(&["stop", &name]).await { - tracing::debug!(container = %name, error = %e, "container stop (ignored)"); - } + let secs = secs.clone(); + set.spawn(async move { + if let Err(e) = docker(&["stop", "-t", &secs, &name]).await { + tracing::debug!(container = %name, error = %e, "container stop (ignored)"); + } + }); } + while set.join_next().await.is_some() {} Ok(()) } @@ -354,6 +378,23 @@ async fn user_matches(name: &str, want: &Option) -> bool { } } +/// Whether a container was created with `--init` (tini as PID 1). `.HostConfig.Init` +/// is `true` only then; an old container (pre-`--init`) reports ``/`false`, so its +/// PID 1 is `sleep infinity`, which ignores SIGTERM and makes `docker stop` hang for +/// the full grace before SIGKILL. A `false` here triggers a recreate in [`ensure`]. +async fn init_matches(name: &str) -> bool { + docker(&["inspect", "-f", "{{.HostConfig.Init}}", name]) + .await + .map(|s| s.trim() == "true") + .unwrap_or(false) +} + +/// Whether an existing container can be reused as-is: right `--user` (§6 UID coherence) +/// **and** `--init` (fast, clean `docker stop`). A mismatch on either recreates it. +async fn reusable(name: &str, want_user: &Option) -> bool { + user_matches(name, want_user).await && init_matches(name).await +} + /// Gives the container's runtime `uid`/`gid` a passwd + shadow (+ group) entry, so /// tools that resolve the invoking user work despite the arbitrary numeric uid — and /// so `sudo` succeeds (without a shadow entry PAM's account phase fails with "account From 8befab42373f3df41cf8d5458e2860a77a4aeb5f Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 20:12:41 +0100 Subject: [PATCH 06/13] llm: add structured streaming support for Anthropic and OpenAI clients --- crates/llm-client/src/anthropic.rs | 42 +++++++++++++++++-- crates/llm-client/src/lib.rs | 15 ++++++- crates/llm-client/src/openai.rs | 14 ++++++- .../src/session/handler/llm_call.rs | 25 ++++++++++- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/crates/llm-client/src/anthropic.rs b/crates/llm-client/src/anthropic.rs index 1c8562a..b0b6cf1 100644 --- a/crates/llm-client/src/anthropic.rs +++ b/crates/llm-client/src/anthropic.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use tokio::sync::mpsc; use tracing::{debug, info, trace, warn}; -use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, headers_to_json, redact_key}; +use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, error_response_body, headers_to_json, redact_key}; const DEFAULT_BASE_URL: &str = "https://api.anthropic.com"; const ANTHROPIC_VERSION: &str = "2023-06-01"; @@ -203,6 +203,10 @@ impl AnthropicClient { }) } + /// Sends the request and returns the raw response **without** `error_for_status`, + /// so the tool-calling paths can read the error body and attach the request + /// payload to the `LlmError` (a `reqwest` status error discards the body). The + /// plain `chat` path keeps its own `error_for_status`. async fn send_request(&self, body: &Value) -> reqwest::Result { self.http .post(self.url()) @@ -211,8 +215,7 @@ impl AnthropicClient { .header("X-Title", core_api::APP_NAME) .json(body) .send() - .await? - .error_for_status() + .await } /// Joined `thinking` blocks of a content array, if any (extended thinking). @@ -252,6 +255,23 @@ impl AnthropicClient { let http_resp = self.send_request(&body).await?; let response_headers = headers_to_json(http_resp.headers()); + let status = http_resp.status(); + if !status.is_success() { + let resp_text = http_resp.text().await?; + return Err(crate::LlmError { + status: Some(status.as_u16()), + message: format!( + "anthropic: HTTP {status} from {url}\nbody: {resp_text}", + url = self.url(), + ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), + }.into()); + } /// One content block being accumulated by index. #[derive(Default)] @@ -562,7 +582,23 @@ impl ChatbotClient for AnthropicClient { let http_resp = self.send_request(&body).await?; let response_headers = headers_to_json(http_resp.headers()); + let status = http_resp.status(); let resp_text = http_resp.text().await?; + if !status.is_success() { + return Err(crate::LlmError { + status: Some(status.as_u16()), + message: format!( + "anthropic: HTTP {status} from {url}\nbody: {resp_text}", + url = self.url(), + ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), + }.into()); + } let resp: Value = serde_json::from_str(&resp_text) .map_err(|e| anyhow::anyhow!("anthropic: failed to parse response JSON: {e}\nbody: {resp_text}"))?; let response_body: Value = serde_json::from_str(&resp_text).unwrap_or(Value::Null); diff --git a/crates/llm-client/src/lib.rs b/crates/llm-client/src/lib.rs index 274fd29..1d44eed 100644 --- a/crates/llm-client/src/lib.rs +++ b/crates/llm-client/src/lib.rs @@ -66,6 +66,14 @@ pub fn headers_to_json(headers: &reqwest::header::HeaderMap) -> Value { Value::Object(map) } +/// Turns a raw error-response body into a JSON `Value` for the payload log: +/// the parsed JSON when the provider returned JSON (the common case — an +/// `{"error": …}` object), else the raw text wrapped as a JSON string so a +/// non-JSON body (HTML gateway page, plain text) is still preserved verbatim. +pub fn error_response_body(text: String) -> Value { + serde_json::from_str::(&text).unwrap_or(Value::String(text)) +} + /// Returns a redacted preview of an API key: first 7 chars + "***". pub fn redact_key(key: &str) -> String { if key.len() > 7 { @@ -82,12 +90,17 @@ pub fn redact_key(key: &str) -> String { /// substring-matching a formatted message — which mis-fires when a model id, token /// count or URL merely contains "401"/"404"/… (bug B6). Non-HTTP failures (network, /// JSON parse, cancellation) stay ordinary `anyhow` errors with no status. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct LlmError { /// HTTP status code, when the failure came from an HTTP response. pub status: Option, /// Human-readable detail (provider tag + body), used for logs and the UI. pub message: String, + /// Request/response payload captured at the failing call, so the debug log + /// can show what was actually sent even when the provider rejected it (e.g. + /// a 400). `None` for failures with no HTTP round-trip (network, cancellation, + /// parse) — those carry no body to surface. + pub raw_meta: Option, } impl std::fmt::Display for LlmError { diff --git a/crates/llm-client/src/openai.rs b/crates/llm-client/src/openai.rs index bfc3cc7..c9a655e 100644 --- a/crates/llm-client/src/openai.rs +++ b/crates/llm-client/src/openai.rs @@ -6,7 +6,7 @@ use serde_json::{Value, json}; use tokio::sync::mpsc; use tracing::{debug, info, trace, warn}; -use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, headers_to_json, redact_key}; +use crate::{ChatOptions, ChatResponse, ChatbotClient, LlmRawMeta, LlmTurn, Message, Role, SseDecoder, StreamDelta, ToolCall, error_response_body, headers_to_json, redact_key}; use core_api::APP_NAME; /// OpenAI ChatGPT client (also compatible with any OpenAI-spec endpoint). @@ -140,6 +140,12 @@ impl OpenAiClient { "openai: HTTP {status} from {url}\nbody: {resp_text}", url = self.url(), ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), }.into()); } @@ -348,6 +354,12 @@ impl ChatbotClient for OpenAiClient { "openai: HTTP {status} from {url}\nbody: {resp_text}", url = self.url(), ), + raw_meta: Some(LlmRawMeta { + request_headers: Some(request_headers), + request_body: Some(request_body), + response_headers: Some(response_headers), + response_body: Some(error_response_body(resp_text)), + }), }.into()); } diff --git a/crates/skald-core/src/session/handler/llm_call.rs b/crates/skald-core/src/session/handler/llm_call.rs index 70e099a..52c9982 100644 --- a/crates/skald-core/src/session/handler/llm_call.rs +++ b/crates/skald-core/src/session/handler/llm_call.rs @@ -13,7 +13,7 @@ use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; use tracing::{error, warn}; -use crate::chatbot::{ChatOptions, LlmTurn, StreamDelta}; +use crate::chatbot::{ChatOptions, LlmError, LlmTurn, StreamDelta}; use crate::db::llm_request_payloads; use crate::events::{ServerEvent, TokenDeltaKind}; use crate::llm::{LlmEntry, LlmStrength}; @@ -121,6 +121,27 @@ impl ChatSessionHandler { Err(e) => e, }; + // Persist the payload even on failure so the debug log shows the request + // that was rejected (e.g. a provider 400). Only the HTTP clients attach a + // body (`LlmError::raw_meta`); a network/parse/cancel error carries none. + // Fire-and-forget, keyed on the same `request_id` as the metadata row the + // logging wrapper wrote to system.db. + if let Some(meta) = e.downcast_ref::().and_then(|le| le.raw_meta.as_ref()) { + let row = llm_request_payloads::PayloadRow { + request_id: request_id.clone(), + request_json: meta.request_body.as_ref().map(|v| v.to_string()).unwrap_or_default(), + request_headers: meta.request_headers.as_ref().map(|v| v.to_string()), + response_json: meta.response_body.as_ref().map(|v| v.to_string()), + response_headers: meta.response_headers.as_ref().map(|v| v.to_string()), + }; + let pool = Arc::clone(&self.db); + tokio::spawn(async move { + if let Err(e) = llm_request_payloads::insert(&pool, row).await { + tracing::warn!(error = %e, "llm_request_payloads: failed to insert error payload"); + } + }); + } + error!(session_id = self.session_id, client = %cur_name, error = %e, "LLM call failed"); self.llm_manager.mark_failure(cur_name, &e.to_string()).await; @@ -226,7 +247,7 @@ mod tests { use crate::chatbot::LlmError; fn http_err(status: u16, message: &str) -> anyhow::Error { - LlmError { status: Some(status), message: message.to_string() }.into() + LlmError { status: Some(status), message: message.to_string(), ..Default::default() }.into() } #[test] From 0156bf681475982c8466bce739610294faf32d2b Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 20:50:31 +0100 Subject: [PATCH 07/13] llm: add Requesty provider, improve message builder, wire bundles --- crates/llm-client/src/openai.rs | 22 +- crates/skald-core/src/llm/providers/mod.rs | 1 + .../skald-core/src/llm/providers/requesty.rs | 197 ++++++++++++++++++ .../src/session/handler/message_builder.rs | 36 +++- crates/skald-core/src/skald/bundles.rs | 1 + 5 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 crates/skald-core/src/llm/providers/requesty.rs diff --git a/crates/llm-client/src/openai.rs b/crates/llm-client/src/openai.rs index c9a655e..6b8ee09 100644 --- a/crates/llm-client/src/openai.rs +++ b/crates/llm-client/src/openai.rs @@ -223,6 +223,26 @@ impl OpenAiClient { warn!(model = %options.model, ?output_tokens, "openai: response truncated (max_tokens reached)"); } + // Reassemble the streamed message for the payload log, so a streamed call + // leaves the same debugging trail as a buffered one — including + // reasoning_content and tool_calls, which previously existed only as + // transient deltas and never appeared in the logged body. Built here, + // before `turn` consumes the accumulators (clones are cheap vs. the round-trip). + let logged_tool_calls: Vec = tool_calls.iter() + .map(|(_idx, (id, name, args))| json!({ + "id": id, + "type": "function", + "function": { "name": name, "arguments": args }, + })) + .collect(); + let mut logged_message = json!({ "role": "assistant", "content": content.clone() }); + if let Some(rc) = &reasoning_content { + logged_message["reasoning_content"] = rc.clone().into(); + } + if !logged_tool_calls.is_empty() { + logged_message["tool_calls"] = Value::Array(logged_tool_calls); + } + let turn = if !tool_calls.is_empty() { let calls = tool_calls .into_values() @@ -242,7 +262,7 @@ impl OpenAiClient { // streamed call leaves the same debugging trail as a buffered one. let response_body = json!({ "streamed": true, - "choices": [{ "finish_reason": finish }], + "choices": [{ "finish_reason": finish, "message": logged_message }], "usage": usage, }); let raw_meta = LlmRawMeta { diff --git a/crates/skald-core/src/llm/providers/mod.rs b/crates/skald-core/src/llm/providers/mod.rs index c9d0829..d6b6b3a 100644 --- a/crates/skald-core/src/llm/providers/mod.rs +++ b/crates/skald-core/src/llm/providers/mod.rs @@ -3,6 +3,7 @@ pub mod declared; pub mod ollama; pub mod openai; pub mod openrouter; +pub mod requesty; // Re-export so existing code that uses `providers::ServiceType` / `providers::RemoteLlmModelInfo` keeps working. pub use crate::provider::ServiceType; diff --git a/crates/skald-core/src/llm/providers/requesty.rs b/crates/skald-core/src/llm/providers/requesty.rs new file mode 100644 index 0000000..601d06f --- /dev/null +++ b/crates/skald-core/src/llm/providers/requesty.rs @@ -0,0 +1,197 @@ +use anyhow::{Result, anyhow}; + +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm, fetch_openai_models}; +use crate::llm::{LlmModelRecord, LlmProviderRecord}; +use crate::provider::{ApiProvider, BuiltLlmClient, ProviderField, ProviderUiMeta, ReasoningMode, ServiceType}; + +/// Requesty router base URL. +const BASE_URL: &str = "https://router.requesty.ai/v1"; + +/// Reasoning effort values Requesty passes through to supporting models +/// (OpenAI o-series, Anthropic extended thinking, DeepSeek-R, etc.). +const REASONING_EFFORTS: &[&str] = &["minimal", "low", "medium", "high"]; + +pub struct RequestyProvider { + /// Lazy: a `reqwest::Client` can only be built after the process installs + /// a crypto provider (done by the shell at startup), so we defer until + /// first use — same pattern as `DeclaredProvider`. + http: std::sync::OnceLock, +} + +impl RequestyProvider { + pub fn new() -> Self { + Self { http: std::sync::OnceLock::new() } + } + + fn http(&self) -> &reqwest::Client { + self.http.get_or_init(reqwest::Client::new) + } + + async fn fetch_catalog(&self, api_key: &str) -> Result> { + let raw = fetch_openai_models(self.http(), BASE_URL, Some(api_key), "Requesty").await?; + Ok(raw.iter().filter_map(map_model).collect()) + } +} + +/// Maps one raw JSON model object from Requesty's `GET /v1/models` response +/// to `RemoteLlmModelInfo`. The endpoint returns flat fields (not nested +/// objects): `context_window`, `max_output_tokens`, `input_price`, +/// `output_price` (all per-token USD), and `supports_*` booleans. +fn map_model(m: &serde_json::Value) -> Option { + let id = m["id"].as_str()?.to_string(); + + let context_length = m["context_window"].as_u64(); + let max_completion_tokens = m["max_output_tokens"].as_u64(); + + // Prices are per-token USD → convert to per-million. + let price_input_per_million = m["input_price"].as_f64().map(|v| v * 1_000_000.0); + let price_output_per_million = m["output_price"].as_f64().map(|v| v * 1_000_000.0); + + let vision = m["supports_vision"].as_bool().unwrap_or(false); + let reasoning = m["supports_reasoning"].as_bool().unwrap_or(false); + + let mut capabilities = vec!["function_calling".to_string()]; + if vision { capabilities.push("vision".to_string()); } + if reasoning { capabilities.push("reasoning".to_string()); } + capabilities.sort(); + capabilities.dedup(); + + Some(RemoteLlmModelInfo { + name: id.clone(), + id, + context_length, + max_completion_tokens, + knowledge_cutoff: None, + capabilities, + vision: Some(vision), + price_input_per_million, + price_output_per_million, + reasoning: None, + }) +} + +#[async_trait::async_trait] +impl ApiProvider for RequestyProvider { + fn type_id(&self) -> &'static str { "requesty" } + fn display_name(&self) -> &'static str { "Requesty" } + fn supported_types(&self) -> &'static [ServiceType] { + &[ServiceType::Llm] + } + + async fn list_llm_models(&self, record: &LlmProviderRecord) -> Result>> { + let api_key = record.api_key.as_deref() + .ok_or_else(|| anyhow!("provider '{}': api_key required for requesty model listing", record.name))?; + Ok(Some(self.fetch_catalog(api_key).await?)) + } + + fn reasoning_mode(&self, _model_id: &str, capabilities: &[String]) -> Option { + if capabilities.iter().any(|c| c == "reasoning") { + Some(ReasoningMode::ValueSet { + values: REASONING_EFFORTS.iter().map(|s| s.to_string()).collect(), + default: Some("medium".to_string()), + }) + } else { + None + } + } + + fn reasoning_request(&self, value: &serde_json::Value) -> Option { + value.as_str().map(|s| serde_json::json!({ "reasoning_effort": s })) + } + + fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { + Some(build_openai_llm(self, BASE_URL, record, model, false)) + } + + fn ui_meta(&self) -> ProviderUiMeta { + ProviderUiMeta { + type_id: "requesty", + display_name: "Requesty", + description: Some("Requesty AI gateway — 300+ models from OpenAI, Anthropic, Google and more"), + color: "#10b981", + icon: "bi-shuffle", + lists_models: true, + fields: &[ + ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, + ], + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn map_full_model() { + let m = serde_json::json!({ + "id": "anthropic/claude-opus-4-8", + "object": "model", + "created": 1715367049, + "owned_by": "anthropic", + "context_window": 1048576, + "max_output_tokens": 128000, + "input_price": 0.000015, + "output_price": 0.000075, + "supports_vision": true, + "supports_reasoning": true + }); + let info = map_model(&m).unwrap(); + assert_eq!(info.id, "anthropic/claude-opus-4-8"); + assert_eq!(info.context_length, Some(1_048_576)); + assert_eq!(info.max_completion_tokens, Some(128_000)); + assert_eq!(info.price_input_per_million, Some(15.0)); + assert_eq!(info.price_output_per_million, Some(75.0)); + assert_eq!(info.vision, Some(true)); + assert!(info.capabilities.contains(&"vision".to_string())); + assert!(info.capabilities.contains(&"reasoning".to_string())); + assert!(info.capabilities.contains(&"function_calling".to_string())); + } + + #[test] + fn map_bare_model() { + // Some models may omit the enriched fields entirely. + let m = serde_json::json!({ + "id": "experimental/model-x", + "object": "model", + "owned_by": "test" + }); + let info = map_model(&m).unwrap(); + assert_eq!(info.id, "experimental/model-x"); + assert_eq!(info.context_length, None); + assert_eq!(info.max_completion_tokens, None); + assert_eq!(info.price_input_per_million, None); + assert_eq!(info.vision, Some(false)); + assert!(!info.capabilities.contains(&"vision".to_string())); + } + + #[test] + fn map_non_vision_non_reasoning() { + let m = serde_json::json!({ + "id": "deepseek/deepseek-chat", + "context_window": 64000, + "supports_vision": false, + "supports_reasoning": false + }); + let info = map_model(&m).unwrap(); + assert_eq!(info.context_length, Some(64_000)); + assert_eq!(info.vision, Some(false)); + assert!(!info.capabilities.contains(&"reasoning".to_string())); + } + + #[test] + fn reasoning_request_maps_effort() { + let p = RequestyProvider::new(); + let req = |v: &str| p.reasoning_request(&serde_json::json!(v)).unwrap(); + assert_eq!(req("high"), serde_json::json!({ "reasoning_effort": "high" })); + assert_eq!(req("minimal"), serde_json::json!({ "reasoning_effort": "minimal" })); + assert!(p.reasoning_request(&serde_json::json!(42)).is_none()); + } + + #[test] + fn reasoning_mode_from_capabilities() { + let p = RequestyProvider::new(); + assert!(p.reasoning_mode("any", &["reasoning".to_string()]).is_some()); + assert!(p.reasoning_mode("any", &[]).is_none()); + } +} diff --git a/crates/skald-core/src/session/handler/message_builder.rs b/crates/skald-core/src/session/handler/message_builder.rs index 1952a28..6e4c78c 100644 --- a/crates/skald-core/src/session/handler/message_builder.rs +++ b/crates/skald-core/src/session/handler/message_builder.rs @@ -17,6 +17,14 @@ use crate::tools::tool_names as tn; /// that have `inject_skills` enabled (the default). const SKILLS_INDEX_PATH: &str = "skills/index.md"; +/// Stand-in for a tool-call turn's `reasoning_content` when none was recorded. +/// DeepSeek's thinking mode 400s if an assistant turn that made tool calls is +/// replayed with an absent or empty `reasoning_content` (it "must be passed back"), +/// and a bare tool call sometimes arrives with no reasoning at all — so the field +/// must always be present and non-empty. Neutral text: it stands in for the model's +/// own prior chain-of-thought. +const REASONING_ROUNDTRIP_PLACEHOLDER: &str = "(no reasoning recorded for this step)"; + /// OS description (type + version), computed once — it does not change at runtime. fn os_description() -> &'static str { static OS: std::sync::OnceLock = std::sync::OnceLock::new(); @@ -298,11 +306,14 @@ impl MessageBuilder { if tool_calls.is_empty() { let mut msg = json!({ "role": "assistant", "content": entry.content }); - if let Some(rc) = &entry.reasoning_content { + // A plain (non-tool) assistant turn does not need its reasoning + // round-tripped; echo it only when we actually have some, and never + // as "" (DeepSeek rejects an empty reasoning_content). + if let Some(rc) = entry.reasoning_content.as_deref().filter(|s| !s.is_empty()) { // Echo under both names: DeepSeek expects "reasoning_content", // MiniMax M3 and others expect "reasoning". - msg["reasoning_content"] = rc.clone().into(); - msg["reasoning"] = rc.clone().into(); + msg["reasoning_content"] = rc.into(); + msg["reasoning"] = rc.into(); } out.push(msg); } else { @@ -323,12 +334,19 @@ impl MessageBuilder { "content": entry.content, "tool_calls": tc_array, }); - if let Some(rc) = &entry.reasoning_content { - // Echo under both names: DeepSeek expects "reasoning_content", - // MiniMax M3 and others expect "reasoning". - msg["reasoning_content"] = rc.clone().into(); - msg["reasoning"] = rc.clone().into(); - } + // DeepSeek thinking mode: an assistant turn that made tool calls + // must carry a NON-EMPTY reasoning_content back on the request that + // continues from its tool result, or the API 400s ("reasoning_content + // in the thinking mode must be passed back"). DeepSeek sometimes + // streams a bare tool call with no reasoning, so the stored value can + // be absent/empty — backfill a placeholder, since both an absent and + // an empty field are rejected on replay. Echoed under both names + // (MiniMax M3 uses "reasoning"); harmless for providers that ignore it. + let rc = entry.reasoning_content.as_deref() + .filter(|s| !s.is_empty()) + .unwrap_or(REASONING_ROUNDTRIP_PLACEHOLDER); + msg["reasoning_content"] = rc.into(); + msg["reasoning"] = rc.into(); out.push(msg); for tc in &tool_calls { diff --git a/crates/skald-core/src/skald/bundles.rs b/crates/skald-core/src/skald/bundles.rs index 47cc11e..7cd13c3 100644 --- a/crates/skald-core/src/skald/bundles.rs +++ b/crates/skald-core/src/skald/bundles.rs @@ -64,6 +64,7 @@ impl Models { provider_registry.register_builtin(crate::llm::providers::openai::OpenAiProvider); provider_registry.register_builtin(crate::llm::providers::anthropic::AnthropicProvider::new()); provider_registry.register_builtin(crate::llm::providers::openrouter::OpenRouterProvider::new()); + provider_registry.register_builtin(crate::llm::providers::requesty::RequestyProvider::new()); provider_registry.register_builtin(crate::llm::providers::ollama::OllamaProvider::new()); // OpenAI-compatible providers are runtime data (providers.yaml), not code. for p in crate::llm::providers::declared::load(std::path::Path::new( From e71990347d3755f2d752e99da70da3240237eeea Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 20:58:04 +0100 Subject: [PATCH 08/13] inbox: improve pending items display and live updates --- web/components/shared/inbox-page.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/web/components/shared/inbox-page.js b/web/components/shared/inbox-page.js index 2680261..ee0f167 100644 --- a/web/components/shared/inbox-page.js +++ b/web/components/shared/inbox-page.js @@ -17,6 +17,16 @@ export class InboxPage extends LightElement { this._expanded = new Set(); } + connectedCallback() { + super.connectedCallback(); + // Live refresh: the always-mounted keeps the chat WS connected + // regardless of the active tab, and it re-dispatches inbox lifecycle events + // from any of this user's sessions as the `inbox-changed` window event. + // Reload immediately when visible instead of waiting for the next poll. + this.__onInboxChanged = () => { if (this.visible) this._load(); }; + window.addEventListener('inbox-changed', this.__onInboxChanged); + } + updated(changed) { if (!changed.has('visible')) return; if (this.visible) { @@ -30,11 +40,13 @@ export class InboxPage extends LightElement { disconnectedCallback() { super.disconnectedCallback(); this._stopPolling(); + window.removeEventListener('inbox-changed', this.__onInboxChanged); } _startPolling() { this._stopPolling(); - this._pollTimer = setInterval(() => this._load(), 8000); + // Fallback only — pushes via `inbox-changed` keep the page fresh. + this._pollTimer = setInterval(() => this._load(), 60000); } _stopPolling() { From 7769b6689da502c31dfa8660033ec8b86642c355 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 22:32:43 +0100 Subject: [PATCH 09/13] list_files with_metadata mode, deeper JSON outline, MCP connector descriptions in prompt - list_files: new with_metadata parameter returns {path, line_count?, size} per entry (both disk and memory-docs) so the agent can spot large files worth outlining before reading - ast_outline: replaced flat tree-sitter JSON walker with a recursive one that shows nested keys at every depth, with inline scalar values and container summaries - message_builder: format active MCP connectors as a table with description instead of a bare bullet list - read_file description now hints to use get_ast_outline first - tools.md: agent guidance to outline before reading --- agents/common/tools.md | 4 + crates/skald-core/src/db/memory_docs.rs | 31 ++++ .../src/session/handler/message_builder.rs | 8 +- crates/skald-core/src/tools/ast_outline.rs | 166 ++++++++++++++++-- crates/skald-core/src/tools/fs/list_files.rs | 133 +++++++++++++- crates/skald-core/src/tools/fs/read_file.rs | 2 + 6 files changed, 325 insertions(+), 19 deletions(-) diff --git a/agents/common/tools.md b/agents/common/tools.md index 9c6e6c4..4e83ef2 100644 --- a/agents/common/tools.md +++ b/agents/common/tools.md @@ -1,3 +1,7 @@ # Tools Scratchpad notes (`update_scratchpad`) are shared across all agents in the session and injected into every agent's context. Not persisted across sessions. Keep values concise. For a **private** task list that sub-agents should *not* see, use `write_todos` instead. + +## Understanding code before you read it + +When you need to understand source code you don't already know, reach for `get_ast_outline` **before** `read_file` — especially on a large file. It returns the file's structure and the line range of every definition at a fraction of the tokens. Then `read_file` only the ranges you actually need. Reading a whole unfamiliar file wastes context; outline first, read narrow. (`list_files` with `with_metadata=true` reports each file's size and line count, so you can spot which files are worth outlining.) diff --git a/crates/skald-core/src/db/memory_docs.rs b/crates/skald-core/src/db/memory_docs.rs index 662752f..ff2b616 100644 --- a/crates/skald-core/src/db/memory_docs.rs +++ b/crates/skald-core/src/db/memory_docs.rs @@ -34,6 +34,16 @@ pub struct MemoryHit { pub snippet: String, } +/// A directory listing row carrying cheap size metadata. `line_count` and +/// `byte_len` are computed in SQL (`LENGTH` / newline count) so the note body +/// never leaves the database. +#[derive(Debug, Clone, sqlx::FromRow)] +pub struct MemoryEntryMeta { + pub path: String, + pub line_count: i64, + pub byte_len: i64, +} + const SELECT: &str = "SELECT id, path, content, created_at, updated_at FROM memory_docs"; /// Fetch one note by its exact path. @@ -84,6 +94,27 @@ pub async fn list(pool: &SqlitePool, prefix: &str) -> Result> { Ok(rows) } +/// Like [`list`], but each row also carries a line count and byte length, +/// computed in SQL so the body is never transferred. Line count matches the +/// on-disk convention: an empty note is 0 lines, otherwise newline-count + 1. +pub async fn list_with_metadata(pool: &SqlitePool, prefix: &str) -> Result> { + let pattern = format!("{}%", escape_like(prefix)); + let rows = sqlx::query_as::<_, MemoryEntryMeta>( + "SELECT path, + CASE WHEN content = '' THEN 0 + ELSE LENGTH(content) - LENGTH(REPLACE(content, char(10), '')) + 1 + END AS line_count, + LENGTH(CAST(content AS BLOB)) AS byte_len + FROM memory_docs + WHERE path LIKE ? ESCAPE '\\' + ORDER BY updated_at DESC", + ) + .bind(pattern) + .fetch_all(pool) + .await?; + Ok(rows) +} + /// Full-text search over note bodies and paths, best match first. `query` is /// FTS5 MATCH syntax; `snippet` is a short excerpt of the body with the matched /// terms wrapped in `[` … `]`. diff --git a/crates/skald-core/src/session/handler/message_builder.rs b/crates/skald-core/src/session/handler/message_builder.rs index 6e4c78c..025e3e0 100644 --- a/crates/skald-core/src/session/handler/message_builder.rs +++ b/crates/skald-core/src/session/handler/message_builder.rs @@ -618,9 +618,13 @@ impl MessageBuilder { } if !active.is_empty() { - out.push_str("\n**Active** — tools callable as `mcp____`:\n"); + out.push_str("\n**Active** — tools callable as `mcp____`:\n\n"); + out.push_str("| Server | Description |\n|--------|-------------|\n"); for name in &active { - out.push_str(&format!("- `{name}`\n")); + let desc = descriptions.get(*name) + .and_then(|d| d.as_deref()) + .unwrap_or("—"); + out.push_str(&format!("| `{name}` | {desc} |\n")); } } diff --git a/crates/skald-core/src/tools/ast_outline.rs b/crates/skald-core/src/tools/ast_outline.rs index 13d6e9d..e0800b0 100644 --- a/crates/skald-core/src/tools/ast_outline.rs +++ b/crates/skald-core/src/tools/ast_outline.rs @@ -15,12 +15,14 @@ impl Tool for AstOutline { fn category(&self) -> crate::tools::ToolCategory { crate::tools::ToolCategory::Filesystem } fn description(&self) -> &str { - "Return the structural outline of a source file: top-level definitions (functions, classes, \ - structs, methods, traits, interfaces, etc.) without their bodies. \ - Each entry is formatted as 'START-END | : ' where START and END are 1-based \ - line numbers of the full definition — same column format as read_file, so you can pass \ - START/END directly to read_file's start_line/end_line to read just that definition. \ - Much cheaper than reading the full file when you only need to understand the shape of the code. \ + "Start here when you need to understand a source file you don't already know — especially a large one. \ + Returns the file's structural outline: top-level definitions (functions, classes, structs, methods, \ + traits, interfaces, etc.) without their bodies, so you grasp the whole shape at a fraction of the \ + tokens of reading it. \ + Each entry is formatted as 'START-END | : ' where START and END are 1-based line numbers \ + of the full definition — same column format as read_file, so you pass START/END straight to \ + read_file's start_line/end_line to read just the definition you care about. \ + Typical flow: outline first, then read only the ranges you need — far cheaper than reading the whole file. \ Supported: .rs .py .js .mjs .ts .tsx .go .java .c .h .cpp .cc .hpp .swift .lua .rb .sh .ex .exs \ .kt .json .toml .yaml .yml .html .css .md .sql" } @@ -67,7 +69,7 @@ impl Tool for AstOutline { "rb" => outline_ts(path, ts_ruby(), "Ruby"), "sh" | "bash" => outline_ts(path, ts_bash(), "Bash"), "ex" | "exs" => outline_ts(path, ts_elixir(), "Elixir"), - "json" => outline_ts(path, ts_json(), "JSON"), + "json" => outline_json(path), "yaml" | "yml" => outline_ts(path, ts_yaml(), "YAML"), "html" => outline_ts(path, ts_html(), "HTML"), "css" => outline_ts(path, ts_css(), "CSS"), @@ -292,15 +294,6 @@ fn ts_elixir() -> LangConfig { } } -fn ts_json() -> LangConfig { - LangConfig { - language: tree_sitter_json::LANGUAGE.into(), - def_kinds: &["pair"], - name_field: "key", - container_kinds: &[], - } -} - fn ts_yaml() -> LangConfig { LangConfig { language: tree_sitter_yaml::LANGUAGE.into(), @@ -331,6 +324,147 @@ fn ts_css() -> LangConfig { } } +// ── JSON outline (dedicated tree-sitter walker: nested keys) ──────────────── +// +// The generic `collect_nodes` only descends through `container_kinds`, which for +// JSON tops out at the first level of the root object (and never enters arrays +// of objects). This walker recurses through the parse tree instead: it lists +// every key at every depth, shows scalar values inline, and expands nested +// objects/arrays. Line ranges keep the read_file contract (`START-END | …`). + +const JSON_VALUE_KINDS: &[&str] = + &["object", "array", "string", "number", "true", "false", "null"]; + +fn outline_json(path: &str) -> Result { + let source = read_to_string(path)?; + let mut parser = tree_sitter::Parser::new(); + let language: tree_sitter::Language = tree_sitter_json::LANGUAGE.into(); + parser.set_language(&language) + .map_err(|e| anyhow::anyhow!("tree-sitter language load error: {e}"))?; + let tree = parser.parse(source.as_bytes(), None) + .ok_or_else(|| anyhow::anyhow!("tree-sitter parse returned None for {path}"))?; + + let mut out = format!("--- JSON outline: {path} ---\n\n"); + // document → single top-level value (object or array). + if let Some(top) = json_first_value(tree.root_node()) { + json_walk(top, &source, 0, &mut out); + } + Ok(out) +} + +/// First JSON value child of `document` (skips comments/whitespace nodes). +fn json_first_value(document: tree_sitter::Node) -> Option { + for i in 0..document.child_count() { + let c = document.child(i as u32).unwrap(); + if JSON_VALUE_KINDS.contains(&c.kind()) { + return Some(c); + } + } + None +} + +/// Emit one line per entry of an object/array, recursing into nested containers. +/// Scalars are shown inline; scalar array elements are summarised by the array's +/// header only (not listed) to stay readable on large value arrays. +fn json_walk(node: tree_sitter::Node, source: &str, depth: usize, out: &mut String) { + const MAX_JSON_DEPTH: usize = 16; + if depth > MAX_JSON_DEPTH { + return; + } + match node.kind() { + "object" => { + for i in 0..node.child_count() { + let pair = node.child(i as u32).unwrap(); + if pair.kind() != "pair" { + continue; + } + let (Some(key), Some(val)) = ( + pair.child_by_field_name("key"), + pair.child_by_field_name("value"), + ) else { + continue; + }; + json_emit(&json_key_text(key, source), val, pair, source, depth, out); + } + } + "array" => { + let mut idx = 0usize; + for i in 0..node.child_count() { + let el = node.child(i as u32).unwrap(); + if !JSON_VALUE_KINDS.contains(&el.kind()) { + continue; + } + let this = idx; + idx += 1; + // Only expand container elements; scalars are covered by the count. + if el.kind() == "object" || el.kind() == "array" { + json_emit(&format!("[{this}]"), el, el, source, depth, out); + } + } + } + _ => {} + } +} + +/// Emit one entry line (`name: `) spanning `span`'s rows, +/// then recurse when the value is itself a container. +fn json_emit( + name: &str, + val: tree_sitter::Node, + span: tree_sitter::Node, + source: &str, + depth: usize, + out: &mut String, +) { + let start = span.start_position().row + 1; + let end = span.end_position().row + 1; + let indent = " ".repeat(depth); + let desc = json_value_desc(val, source); + out.push_str(&format!("{start:>4}-{end:>4} | {indent}{name}: {desc}\n")); + if val.kind() == "object" || val.kind() == "array" { + json_walk(val, source, depth + 1, out); + } +} + +/// Short descriptor of a value: `{N keys}`, `[N items]`, or the scalar literal. +fn json_value_desc(node: tree_sitter::Node, source: &str) -> String { + match node.kind() { + "object" => { + let n = json_count(node, &["pair"]); + format!("{{{n} {}}}", if n == 1 { "key" } else { "keys" }) + } + "array" => { + let n = json_count(node, JSON_VALUE_KINDS); + format!("[{n} {}]", if n == 1 { "item" } else { "items" }) + } + _ => { + let raw = source.get(node.byte_range()).unwrap_or(""); + let one = raw.split_whitespace().collect::>().join(" "); + truncate_label(&one, MAX_LABEL_SHORT) + } + } +} + +/// Number of direct children whose kind is in `kinds`. +fn json_count(node: tree_sitter::Node, kinds: &[&str]) -> usize { + let mut n = 0; + for i in 0..node.child_count() { + if kinds.contains(&node.child(i as u32).unwrap().kind()) { + n += 1; + } + } + n +} + +/// Object key text with the surrounding double-quotes stripped. +fn json_key_text(key: tree_sitter::Node, source: &str) -> String { + let raw = source.get(key.byte_range()).unwrap_or(""); + raw.strip_prefix('"') + .and_then(|s| s.strip_suffix('"')) + .unwrap_or(raw) + .to_string() +} + // ── text-based fallbacks (crates incompatible with tree-sitter 0.26) ─────── fn outline_kotlin(path: &str) -> Result { diff --git a/crates/skald-core/src/tools/fs/list_files.rs b/crates/skald-core/src/tools/fs/list_files.rs index 0957b9a..075459a 100644 --- a/crates/skald-core/src/tools/fs/list_files.rs +++ b/crates/skald-core/src/tools/fs/list_files.rs @@ -36,6 +36,8 @@ impl Tool for ListFiles { Skips .git, target, node_modules, .cache. \ Returns a JSON array of paths relative to the requested directory. \ Use depth=1 for immediate contents only, depth=2-3 for moderate exploration. \ + Set with_metadata=true to instead return objects {path, line_count?, size} — handy for spotting a \ + large file worth outlining with get_ast_outline before you read it. \ Listing under user-memory/ (private) or shared-memory/ (shared) lists your memory notes instead of disk." } @@ -54,6 +56,10 @@ impl Tool for ListFiles { "dirs_only": { "type": "boolean", "description": "If true, return only directories and omit files (default false)." + }, + "with_metadata": { + "type": "boolean", + "description": "If true, return objects {path, line_count?, size} instead of bare path strings. size is human-readable; line_count is included only for text files (omitted for binaries and very large files). Use it to decide whether to get_ast_outline a large file before reading it." } } }) @@ -71,6 +77,7 @@ impl Tool for ListFiles { /// under the prefix is returned, keyed relative to the requested directory. fn run_with<'a>(&'a self, ctx: &ToolContext, args: Value) -> Box { let path = args["path"].as_str().unwrap_or("").to_string(); + let with_metadata = args["with_metadata"].as_bool().unwrap_or(false); let Some(m) = classify_memory(&path) else { return match super::rewrite_to_host(&ctx.fs, &path, args) { Ok(args) => self.run(args), @@ -87,6 +94,21 @@ impl Tool for ListFiles { // Treat `rel` as a directory prefix: match `rel/…` (or everything at // the root), then strip it so results are relative to what was asked. let prefix = if rel.is_empty() || rel.ends_with('/') { rel } else { format!("{rel}/") }; + + if with_metadata { + let mut entries: Vec = crate::db::memory_docs::list_with_metadata(&pool, &prefix) + .await? + .into_iter() + .map(|e| FileEntry { + path: e.path.strip_prefix(&prefix).unwrap_or(&e.path).to_string(), + line_count: Some(e.line_count.max(0) as usize), + size: Some(human_size(e.byte_len.max(0) as u64)), + }) + .collect(); + entries.sort_by(|a, b| a.path.cmp(&b.path)); + return Ok(ToolResult::Text(serde_json::to_string(&entries)?)); + } + let entries = crate::db::memory_docs::list(&pool, &prefix).await?; let mut paths: Vec = entries.into_iter() .map(|e| e.path.strip_prefix(&prefix).unwrap_or(&e.path).to_string()) @@ -100,12 +122,81 @@ impl Tool for ListFiles { let user_path = args["path"].as_str().unwrap_or("."); let max_depth = args["depth"].as_u64().unwrap_or(3) as usize; let dirs_only = args["dirs_only"].as_bool().unwrap_or(false); + let with_metadata = args["with_metadata"].as_bool().unwrap_or(false); let dir = resolve(user_path)?; let mut paths: Vec = Vec::new(); walk(&dir, &dir, 0, max_depth, dirs_only, &mut paths)?; paths.sort(); - Ok(serde_json::to_string(&paths)?) + + if !with_metadata { + return Ok(serde_json::to_string(&paths)?); + } + let entries: Vec = paths.into_iter() + .map(|rel| file_entry(&dir.join(&rel), rel)) + .collect(); + Ok(serde_json::to_string(&entries)?) + } +} + +/// A `with_metadata` listing row. Field order (declaration order) is the wire +/// order; `line_count` and `size` are omitted when unavailable. +#[derive(serde::Serialize)] +struct FileEntry { + path: String, + #[serde(skip_serializing_if = "Option::is_none")] + line_count: Option, + #[serde(skip_serializing_if = "Option::is_none")] + size: Option, +} + +/// Largest file we'll read to count lines; bigger files report `size` only, so +/// a metadata listing never turns into a full read of the tree. +const LINE_COUNT_SIZE_CAP: u64 = 2 * 1024 * 1024; + +/// Build a metadata row for one on-disk path. `size` comes free from a stat; +/// `line_count` is read only for text files within the size cap. +fn file_entry(abs: &Path, rel: String) -> FileEntry { + let meta = std::fs::metadata(abs).ok(); + let len = meta.as_ref().map(|m| m.len()); + let is_file = meta.as_ref().map(|m| m.is_file()).unwrap_or(false); + let line_count = match len { + Some(l) if is_file && l <= LINE_COUNT_SIZE_CAP => count_lines_if_text(abs), + _ => None, + }; + FileEntry { path: rel, line_count, size: len.map(human_size) } +} + +/// Line count of a text file, or `None` if it reads as binary (contains a NUL). +fn count_lines_if_text(abs: &Path) -> Option { + let bytes = std::fs::read(abs).ok()?; + if bytes.contains(&0) { return None; } + Some(count_lines(&bytes)) +} + +/// Number of lines an editor would show: 0 for empty, else newline-count plus +/// one when the file does not end in a newline. +fn count_lines(bytes: &[u8]) -> usize { + if bytes.is_empty() { return 0; } + let nl = bytes.iter().filter(|&&b| b == b'\n').count(); + if bytes.last() == Some(&b'\n') { nl } else { nl + 1 } +} + +/// Human-readable byte size, `ls -h` style (base 1024): "512 B", "18 KB", "1.4 MB". +fn human_size(bytes: u64) -> String { + const UNITS: &[&str] = &["B", "KB", "MB", "GB", "TB"]; + let mut size = bytes as f64; + let mut unit = 0; + while size >= 1024.0 && unit < UNITS.len() - 1 { + size /= 1024.0; + unit += 1; + } + if unit == 0 { + format!("{bytes} B") + } else { + let s = format!("{size:.1}"); + let s = s.strip_suffix(".0").unwrap_or(&s); + format!("{s} {}", UNITS[unit]) } } @@ -138,3 +229,43 @@ fn walk(root: &Path, dir: &Path, depth: usize, max_depth: usize, dirs_only: bool } Ok(()) } + +#[cfg(test)] +mod meta_smoke { + use super::*; + const SP: &str = "/private/tmp/claude-501/-Users-dguiducci-projects-skald-circle/1cb4c456-6a62-4c67-abf8-bb93ef73e30c/scratchpad/lf"; + + #[test] + fn human_size_fmt() { + assert_eq!(human_size(0), "0 B"); + assert_eq!(human_size(512), "512 B"); + assert_eq!(human_size(18 * 1024), "18 KB"); + assert_eq!(human_size(1024 * 1024 + 400 * 1024), "1.4 MB"); + } + + #[test] + fn line_counts() { + assert_eq!(count_lines(b""), 0); + assert_eq!(count_lines(b"a\nb\nc\n"), 3); + assert_eq!(count_lines(b"no newline"), 1); + } + + #[test] + fn entries() { + let t = std::path::Path::new(SP).join("three.txt"); + let e = file_entry(&t, "three.txt".into()); + println!("three.txt -> {}", serde_json::to_string(&e).unwrap()); + assert_eq!(e.line_count, Some(3)); + + let o = std::path::Path::new(SP).join("one.txt"); + let e = file_entry(&o, "one.txt".into()); + println!("one.txt -> {}", serde_json::to_string(&e).unwrap()); + assert_eq!(e.line_count, Some(1)); + + let b = std::path::Path::new(SP).join("blob.bin"); + let e = file_entry(&b, "blob.bin".into()); + println!("blob.bin -> {}", serde_json::to_string(&e).unwrap()); + assert_eq!(e.line_count, None); // binary: size only + assert!(e.size.is_some()); + } +} diff --git a/crates/skald-core/src/tools/fs/read_file.rs b/crates/skald-core/src/tools/fs/read_file.rs index 83b0d92..e0a8caa 100644 --- a/crates/skald-core/src/tools/fs/read_file.rs +++ b/crates/skald-core/src/tools/fs/read_file.rs @@ -78,6 +78,8 @@ impl Tool for ReadFile { Use instead of cat/head/tail in the terminal. \ Returns text prefixed as ' N | line'. When calling edit_file, copy the text after '| ' exactly. \ For large files use start_line/end_line to read in chunks — files over ~2000 lines should never be read whole. \ + For an unfamiliar source file, call get_ast_outline first to get each definition's line range, then read \ + just the range you need instead of the whole file. \ Use limit to cap output when end_line is unknown. \ Paths under user-memory/ (private) or shared-memory/ (shared) read a note from your memory instead of disk." } From aa4f31ec64e0806833d9b35fad7ab35e0c388a82 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 22:34:09 +0100 Subject: [PATCH 10/13] Fix memory-docs line-count SQL edge case, remove stale tmp-referencing test - memory_docs list_with_metadata: use SQL substr to detect trailing newline instead of counting newlines and adding 1 unconditionally - list_files: remove meta_smoke tests that referenced a non-existent scratchpad directory --- crates/skald-core/src/db/memory_docs.rs | 3 +- crates/skald-core/src/tools/fs/list_files.rs | 40 -------------------- 2 files changed, 2 insertions(+), 41 deletions(-) diff --git a/crates/skald-core/src/db/memory_docs.rs b/crates/skald-core/src/db/memory_docs.rs index ff2b616..b0f49fd 100644 --- a/crates/skald-core/src/db/memory_docs.rs +++ b/crates/skald-core/src/db/memory_docs.rs @@ -102,7 +102,8 @@ pub async fn list_with_metadata(pool: &SqlitePool, prefix: &str) -> Result( "SELECT path, CASE WHEN content = '' THEN 0 - ELSE LENGTH(content) - LENGTH(REPLACE(content, char(10), '')) + 1 + ELSE LENGTH(content) - LENGTH(REPLACE(content, char(10), '')) + + CASE WHEN substr(content, -1, 1) = char(10) THEN 0 ELSE 1 END END AS line_count, LENGTH(CAST(content AS BLOB)) AS byte_len FROM memory_docs diff --git a/crates/skald-core/src/tools/fs/list_files.rs b/crates/skald-core/src/tools/fs/list_files.rs index 075459a..80b654a 100644 --- a/crates/skald-core/src/tools/fs/list_files.rs +++ b/crates/skald-core/src/tools/fs/list_files.rs @@ -229,43 +229,3 @@ fn walk(root: &Path, dir: &Path, depth: usize, max_depth: usize, dirs_only: bool } Ok(()) } - -#[cfg(test)] -mod meta_smoke { - use super::*; - const SP: &str = "/private/tmp/claude-501/-Users-dguiducci-projects-skald-circle/1cb4c456-6a62-4c67-abf8-bb93ef73e30c/scratchpad/lf"; - - #[test] - fn human_size_fmt() { - assert_eq!(human_size(0), "0 B"); - assert_eq!(human_size(512), "512 B"); - assert_eq!(human_size(18 * 1024), "18 KB"); - assert_eq!(human_size(1024 * 1024 + 400 * 1024), "1.4 MB"); - } - - #[test] - fn line_counts() { - assert_eq!(count_lines(b""), 0); - assert_eq!(count_lines(b"a\nb\nc\n"), 3); - assert_eq!(count_lines(b"no newline"), 1); - } - - #[test] - fn entries() { - let t = std::path::Path::new(SP).join("three.txt"); - let e = file_entry(&t, "three.txt".into()); - println!("three.txt -> {}", serde_json::to_string(&e).unwrap()); - assert_eq!(e.line_count, Some(3)); - - let o = std::path::Path::new(SP).join("one.txt"); - let e = file_entry(&o, "one.txt".into()); - println!("one.txt -> {}", serde_json::to_string(&e).unwrap()); - assert_eq!(e.line_count, Some(1)); - - let b = std::path::Path::new(SP).join("blob.bin"); - let e = file_entry(&b, "blob.bin".into()); - println!("blob.bin -> {}", serde_json::to_string(&e).unwrap()); - assert_eq!(e.line_count, None); // binary: size only - assert!(e.size.is_some()); - } -} From 42c0eaf2eccf6aba216bc438567449127111dd2d Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Wed, 22 Jul 2026 23:21:03 +0100 Subject: [PATCH 11/13] Live-refresh connectors after marketplace reinstall After a reinstall the catalog entry carries new llm_short_description, icon and code. Previously the running servers (global + per-user) kept their old metadata and code until the next login. - Add refresh_connector_after_reinstall on Skald: re-snapshots the description from the catalog, reconciles local files on per-user connectors, and restarts both the global and per-user servers - Add set_description db accessor for mcp_global_servers - user_row_spec_resolved now injects the live catalog description (over the bare name) so user-runtime connectors show the right blurb - marketplace install() fetches a fresh feed instead of the browse cache, so a reinstall reflects the changed manifest immediately --- .../skald-core/src/db/mcp_global_servers.rs | 13 ++++ crates/skald-core/src/mcp/mod.rs | 11 ++++ crates/skald-core/src/skald/accessors.rs | 61 +++++++++++++++++++ src/frontend/api/marketplace.rs | 15 ++++- 4 files changed, 99 insertions(+), 1 deletion(-) diff --git a/crates/skald-core/src/db/mcp_global_servers.rs b/crates/skald-core/src/db/mcp_global_servers.rs index ecc8af5..7635f8e 100644 --- a/crates/skald-core/src/db/mcp_global_servers.rs +++ b/crates/skald-core/src/db/mcp_global_servers.rs @@ -147,6 +147,19 @@ pub async fn set_enabled(pool: &SqlitePool, id: i64, enabled: bool) -> Result<() Ok(()) } +/// Re-snapshots the LLM-facing description on an enabled global server, without +/// touching its config/credentials — used when a marketplace **reinstall** rewrites +/// the catalog's `llm_short_description` and the running server's snapshot must catch +/// up (the caller then restarts the server so its in-RAM description updates too). +pub async fn set_description(pool: &SqlitePool, id: i64, description: Option<&str>) -> Result<()> { + sqlx::query("UPDATE mcp_global_servers SET description = ?1 WHERE id = ?2") + .bind(description) + .bind(id) + .execute(pool) + .await?; + Ok(()) +} + pub async fn delete(pool: &SqlitePool, id: i64) -> Result<()> { sqlx::query("DELETE FROM mcp_global_servers WHERE id = ?") .bind(id) diff --git a/crates/skald-core/src/mcp/mod.rs b/crates/skald-core/src/mcp/mod.rs index 15acc72..34397e2 100644 --- a/crates/skald-core/src/mcp/mod.rs +++ b/crates/skald-core/src/mcp/mod.rs @@ -669,6 +669,17 @@ pub async fn user_row_spec_resolved( if let Some(catalog_name) = row.catalog_name.as_deref() { if let Ok(Some(entry)) = crate::db::mcp_catalog::get_by_name(registry, catalog_name).await { spec.tool_titles = crate::db::mcp_catalog::parse_tool_titles(entry.tool_meta_json.as_deref()); + // The catalog's `description` is the connector's `llm_short_description` — + // the line the model reads when deciding whether to `activate_tools()` on + // this server (see `render_mcp_list`). `user_row_spec` only had the bare + // catalog name to fall back on; inject the real blurb here (this is the + // "the caller injects it if richer" the sync builder defers to), and keep + // the name when the catalog has none. Because it is read from the catalog + // live, a marketplace reinstall that rewrites the description is reflected + // the next time this spec is built. + if let Some(desc) = entry.description { + spec.description = Some(desc); + } } } if let (Some(provider), Some(deliver), Some(refresh)) = diff --git a/crates/skald-core/src/skald/accessors.rs b/crates/skald-core/src/skald/accessors.rs index 3656cf7..2dfaaad 100644 --- a/crates/skald-core/src/skald/accessors.rs +++ b/crates/skald-core/src/skald/accessors.rs @@ -133,6 +133,67 @@ impl Skald { } } } + + /// Pushes a marketplace **reinstall** into every live copy of the connector so + /// active sessions pick up the new metadata (`llm_short_description`) and code + /// without a re-login — the reinstall counterpart of the §6/§7 remount helpers. + /// The reinstall has already rewritten `mcp_catalog`; this reconnects what runs: + /// + /// - **Global runtime**: for each *enabled* `mcp_global_servers` row snapshotting + /// this catalog entry, re-snapshot its `description` from the catalog and restart + /// it, so the running server's in-RAM description (and code) catches up. + /// - **Per-user runtimes**: for each live user who has this connector *startable*, + /// re-copy its files/deps into the container (`prepare_local_connector` — a hash + /// no-op when the source is unchanged) and restart that one server. The rebuilt + /// spec now carries the fresh catalog description (see `user_row_spec_resolved`). + /// + /// Best-effort: the catalog write already committed, so a Docker/MCP hiccup here + /// must not fail the reinstall — anything not refreshed settles at the user's next + /// login. A fresh install (nothing live yet) is a cheap no-op: no row matches. + pub async fn refresh_connector_after_reinstall(&self, catalog_name: &str) { + // The metadata the reinstall just wrote — the source of truth to push out. + let entry = match crate::db::mcp_catalog::get_by_name(self.db(), catalog_name).await { + Ok(Some(e)) => e, + Ok(None) => return, + Err(e) => { + tracing::warn!(connector = %catalog_name, error = %e, "reinstall refresh: catalog lookup failed"); + return; + } + }; + + // 1. Global runtime. + if let Ok(globals) = crate::db::mcp_global_servers::all_enabled(self.db()).await { + for g in globals.iter().filter(|g| g.catalog_name.as_deref() == Some(catalog_name)) { + if let Err(e) = crate::db::mcp_global_servers::set_description(self.db(), g.id, entry.description.as_deref()).await { + tracing::warn!(connector = %catalog_name, error = %e, "reinstall refresh: failed to update global description"); + continue; + } + match crate::db::mcp_global_servers::get(self.db(), g.id).await { + Ok(Some(row)) => { + let spec = crate::mcp::global_row_spec(&row); + if let Err(e) = self.mcp().start_server(spec).await { + tracing::warn!(connector = %catalog_name, error = %e, "reinstall refresh: failed to restart global server"); + } + } + _ => tracing::warn!(connector = %catalog_name, "reinstall refresh: global row vanished before restart"), + } + } + } + + // 2. Per-user runtimes — restart this one connector for each live user who runs it. + for ctx in self.rt_user_contexts().all_live().await { + let rows = crate::db::mcp_user_servers::all_startable(&ctx.pool).await.unwrap_or_default(); + let Some(row) = rows.into_iter().find(|r| r.catalog_name.as_deref() == Some(catalog_name)) else { + continue; + }; + let container = crate::container::container_name(&ctx.user_id); + crate::mcp::prepare_local_connector(self.db(), &ctx.user_id, &container, &row).await; + let spec = crate::mcp::user_row_spec_resolved(&row, &container, self.db()).await; + if let Err(e) = ctx.user_mcp.start_server(spec).await { + tracing::warn!(user = %ctx.user_id, connector = %catalog_name, error = %e, "reinstall refresh: failed to restart per-user connector"); + } + } + } pub fn sessions(&self) -> &Arc { &self.rt.sessions } pub fn config(&self) -> &Arc { &self.rt.config } pub fn config_properties(&self) -> &[core_api::ConfigSet] { &self.rt.config_properties } diff --git a/src/frontend/api/marketplace.rs b/src/frontend/api/marketplace.rs index 8b2d36e..f298906 100644 --- a/src/frontend/api/marketplace.rs +++ b/src/frontend/api/marketplace.rs @@ -660,7 +660,12 @@ pub async fn install( ) -> Result, ApiError> { require_cap(&skald, &auth.user_id, role_capabilities::MANAGE_CATALOG).await?; - let feed = feed(false).await?; + // An install (or reinstall) always pulls the **current** feed, never the 300 s + // browse cache: a reinstall exists precisely to pick up a changed manifest + // (new `llm_short_description`, icon, code), so reading a stale snapshot would + // silently reapply the old metadata. Browsing the list stays cached; the + // mutating path fetches fresh. + let feed = feed(true).await?; let h = feed .iter() .find(|h| h.entry.id == body.id) @@ -789,6 +794,14 @@ pub async fn install( ) .await?; + // Push the (re)installed metadata + code into anything already running it, so a + // reinstall lands live instead of waiting for each user's next login: enabled + // global servers re-snapshot the description and restart; each live user who + // activated it gets its files/deps reconciled and the connector restarted with + // the fresh `llm_short_description`. A first-time install matches nothing live + // and is a cheap no-op. + skald.refresh_connector_after_reinstall(&body.id).await; + Ok(Json(json!({ "id": id, "name": h.entry.id, From f1bae3e84f7ceaebb35cf5a16f92824b1933ce78 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Thu, 23 Jul 2026 17:51:11 +0100 Subject: [PATCH 12/13] ci: include commands/ in packaged tarball --- ci/package.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/package.sh b/ci/package.sh index 374cd17..7eddc44 100755 --- a/ci/package.sh +++ b/ci/package.sh @@ -16,7 +16,7 @@ # --output Directory where the .tar.gz will be written # # The tarball contains everything needed to run (or uninstall) Skald Circle: -# bin/skald, bin/skald-setup, web/, agents/, skills/, docs/, +# bin/skald, bin/skald-setup, web/, agents/, commands/, skills/, docs/, # default.config.yaml, providers.yaml, requirements.txt, # requirements-optional.txt, run.sh, update.sh, uninstall.sh @@ -92,6 +92,7 @@ chmod 755 "$STAGING/bin/skald" "$STAGING/bin/skald-setup" # ── Copy runtime assets ─────────────────────────────────────────────────────── cp -r web "$STAGING/web" cp -r agents "$STAGING/agents" +cp -r commands "$STAGING/commands" cp -r skills "$STAGING/skills" cp -r docs "$STAGING/docs" cp default.config.yaml "$STAGING/default.config.yaml" From 798e55951b6cea6eb5437342938093fa98ac0f0e Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Thu, 23 Jul 2026 18:03:28 +0100 Subject: [PATCH 13/13] =?UTF-8?q?ui:=20shape=20simplified=20interface=20?= =?UTF-8?q?=E2=80=94=20hide=20reasoning,=20show=20projects?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- web/components/copilot-render.js | 7 ++++--- web/components/sidebar.js | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/web/components/copilot-render.js b/web/components/copilot-render.js index 67e53ee..42b5d0a 100644 --- a/web/components/copilot-render.js +++ b/web/components/copilot-render.js @@ -513,8 +513,9 @@ export function renderAttachmentChips(host, attachments, { removable = false } = * across re-renders, so a user-expanded block stays open while tokens stream * into it (live) and in past history items alike. */ -function renderReasoning(msg) { +function renderReasoning(host, msg) { if (!msg.reasoning) return nothing; + if (host?._me?.ui_mode === 'simple') return nothing; return html`
    ${t('chat.reasoning')} @@ -531,7 +532,7 @@ export function renderMsg(host, msg) { return html`
    ${msg.failed ? failedBadge() : nothing} - ${renderReasoning(msg)} + ${renderReasoning(host, msg)} ${unsafeHTML(renderMarkdown(msg.content))} ${msg.input_tokens != null ? html`
    ↑${msg.input_tokens.toLocaleString()} tok  ↓${msg.output_tokens?.toLocaleString()} tok
    ` : nothing}
    `; @@ -539,7 +540,7 @@ export function renderMsg(host, msg) { return html`
    ${msg.failed ? failedBadge() : nothing} - ${renderReasoning(msg)} + ${renderReasoning(host, msg)} ${unsafeHTML(renderMarkdown(msg.content))} ${msg.streaming ? html`` : nothing} ${msg.input_tokens != null && !msg.streaming ? html`
    ↑${msg.input_tokens.toLocaleString()} tok  ↓${msg.output_tokens?.toLocaleString()} tok
    ` : nothing} diff --git a/web/components/sidebar.js b/web/components/sidebar.js index 57f535f..ffb4abf 100644 --- a/web/components/sidebar.js +++ b/web/components/sidebar.js @@ -62,6 +62,11 @@ const GROUPS = [ const COLLAPSE_KEY = 'sidebar-collapsed'; +// The projects NAV entry, surfaced in the simplified interface too (projects are +// membership-gated, not capability-gated, so a simple-mode member can own/share +// them just like anyone else). +const PROJECTS_NAV = NAV.find((i) => i.id === 'projects'); + export class AppSidebar extends I18nMixin(LightElement) { static properties = { @@ -437,7 +442,8 @@ export class AppSidebar extends I18nMixin(LightElement) { } render() { - // Simplified interface (role attrs `ui_mode: "simple"`): chat + inbox only, + // Simplified interface (role attrs `ui_mode: "simple"`): chat, inbox and + // projects (self-service workspaces — membership-gated, not capability-gated), // ungrouped. Hiding links is not access control — every route stays // capability-gated server-side; this only shapes the nav for less technical // members. @@ -452,7 +458,11 @@ export class AppSidebar extends I18nMixin(LightElement) { `;