From c5f7221fb2fc66cd17e45667df80678411416b9f Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Fri, 19 Dec 2025 18:34:14 +0100
Subject: [PATCH] fix(astrolabe): address code review feedback

CRITICAL FIXES:
- Fix tag parsing in workflow to strip "astrolabe-v" instead of "v"
  For tag astrolabe-v0.1.0, now correctly extracts "0.1.0"
- Add workflow filtering to only run on astrolabe-v* tags
  Prevents wasting CI resources on MCP/Helm releases

RECOMMENDED IMPROVEMENTS:
- Make Nextcloud server path configurable in Makefile
  Can now override: make appstore server_dir=/path/to/server
- Add dependency validation to Makefile
  Checks for composer, npm, php before building
- Add signing prerequisite validation
  Verifies server/occ, private key, and certificate exist
- Add dependency checks to all bump scripts
  Validates uv is installed before running cz bump

These changes improve local build experience and prevent common
errors with clear error messages and installation guidance.
---
 .github/workflows/appstore-build-publish.yml |  4 ++-
 scripts/bump-astrolabe.sh                    |  3 +++
 scripts/bump-helm.sh                         |  3 +++
 scripts/bump-mcp.sh                          |  3 +++
 third_party/astrolabe/Makefile               | 26 +++++++++++++++++---
 5 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/appstore-build-publish.yml b/.github/workflows/appstore-build-publish.yml
index 63b20f0..bd1165d 100644
--- a/.github/workflows/appstore-build-publish.yml
+++ b/.github/workflows/appstore-build-publish.yml
@@ -11,6 +11,8 @@ env:
 jobs:
   build-and-publish:
     runs-on: ubuntu-latest
+    # Only run on Astrolabe releases
+    if: startsWith(github.ref, 'refs/tags/astrolabe-v')
 
     steps:
       - name: Checkout code
@@ -19,7 +21,7 @@ jobs:
       - name: Get version from tag
         id: tag
         run: |
-          echo "TAG=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT
+          echo "TAG=${GITHUB_REF#refs/tags/astrolabe-v}" >> $GITHUB_OUTPUT
 
       - name: Validate version in info.xml matches tag
         working-directory: ${{ env.APP_DIR }}
diff --git a/scripts/bump-astrolabe.sh b/scripts/bump-astrolabe.sh
index 79e61cc..4a320d7 100755
--- a/scripts/bump-astrolabe.sh
+++ b/scripts/bump-astrolabe.sh
@@ -2,6 +2,9 @@
 # Bump Astrolabe app version
 set -e
 
+# Validate dependencies
+command -v uv >/dev/null 2>&1 || { echo "Error: uv not found. Install from https://docs.astral.sh/uv/"; exit 1; }
+
 cd third_party/astrolabe
 
 echo "Bumping Astrolabe version..."
diff --git a/scripts/bump-helm.sh b/scripts/bump-helm.sh
index 75d08e7..dd5411a 100755
--- a/scripts/bump-helm.sh
+++ b/scripts/bump-helm.sh
@@ -2,6 +2,9 @@
 # Bump Helm chart version
 set -e
 
+# Validate dependencies
+command -v uv >/dev/null 2>&1 || { echo "Error: uv not found. Install from https://docs.astral.sh/uv/"; exit 1; }
+
 cd charts/nextcloud-mcp-server
 
 echo "Bumping Helm chart version..."
diff --git a/scripts/bump-mcp.sh b/scripts/bump-mcp.sh
index 29bb045..6cc9a47 100755
--- a/scripts/bump-mcp.sh
+++ b/scripts/bump-mcp.sh
@@ -2,6 +2,9 @@
 # Bump MCP server version
 set -e
 
+# Validate dependencies
+command -v uv >/dev/null 2>&1 || { echo "Error: uv not found. Install from https://docs.astral.sh/uv/"; exit 1; }
+
 echo "Bumping MCP server version..."
 uv run cz bump --yes
 
diff --git a/third_party/astrolabe/Makefile b/third_party/astrolabe/Makefile
index bf9f188..f96e6e0 100644
--- a/third_party/astrolabe/Makefile
+++ b/third_party/astrolabe/Makefile
@@ -9,19 +9,31 @@ appstore_dir=$(build_dir)/artifacts
 package_name=$(appstore_dir)/$(app_name)
 cert_dir=$(HOME)/.nextcloud/certificates
 
+# Nextcloud server path (configurable via environment variable)
+server_dir?=../../server
+occ=$(server_dir)/occ
+
 # Signing
 private_key=$(cert_dir)/$(app_name).key
 certificate=$(cert_dir)/$(app_name).crt
-sign_cmd=php ../../server/occ integrity:sign-app --privateKey=$(private_key) --certificate=$(certificate)
+sign_cmd=php $(occ) integrity:sign-app --privateKey=$(private_key) --certificate=$(certificate)
 
 # Clean build artifacts
 .PHONY: clean
 clean:
 	rm -rf $(build_dir)
 
+# Validate required dependencies
+.PHONY: validate-deps
+validate-deps:
+	@command -v composer >/dev/null 2>&1 || { echo "Error: composer not found. Install from https://getcomposer.org/"; exit 1; }
+	@command -v npm >/dev/null 2>&1 || { echo "Error: npm not found. Install Node.js from https://nodejs.org/"; exit 1; }
+	@command -v php >/dev/null 2>&1 || { echo "Error: php not found. Install PHP 8.1 or higher."; exit 1; }
+	@echo "✓ All dependencies found"
+
 # Install PHP and Node dependencies
 .PHONY: install-deps
-install-deps:
+install-deps: validate-deps
 	composer install --no-dev --optimize-autoloader
 	npm ci
 
@@ -63,9 +75,17 @@ assemble: clean install-deps build-frontend
 		--exclude='src/' \
 		./ $(package_name)/
 
+# Validate signing prerequisites
+.PHONY: validate-signing
+validate-signing:
+	@test -f $(occ) || { echo "Error: Nextcloud server not found at $(server_dir)"; echo "Set server_dir variable: make appstore server_dir=/path/to/server"; exit 1; }
+	@test -f $(private_key) || { echo "Error: Private key not found at $(private_key)"; exit 1; }
+	@test -f $(certificate) || { echo "Error: Certificate not found at $(certificate)"; exit 1; }
+	@echo "✓ Signing prerequisites validated"
+
 # Create signed release tarball for App Store
 .PHONY: appstore
-appstore: assemble
+appstore: assemble validate-signing
 	# Sign the app
 	$(sign_cmd) --path=$(package_name)
 	# Create tarball