Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Copilot instructions for docketworks

Django 6 backend (`apps/`) + Vue 3 / TypeScript frontend (`frontend/`) job-management system for jobbing
shops. One installation per client. The codebase deliberately deviates from typical Django/Vue defaults —
the reasons are recorded as ADRs in `docs/adr/` (read the index first). Follow the rules below so your
suggestions and reviews match the architecture instead of fighting it. Deeper operational detail lives in
`AGENTS.md` / `CLAUDE.md` and `frontend/CLAUDE.md`.

## Architecture rules

- **Zero backwards compatibility (ADR 0017).** When a name, URL, or data shape changes, every caller
changes in the *same* PR and the old form disappears. No deprecation aliases, no `getattr` shims, no
"for safety" duplicate columns, no dual-read fallbacks. Old URLs return 404, not redirects.

- **Fix the data, not the consumer (ADR 0015).** When code hits malformed or unexpected data, the fix is
a migration that repairs the data — never a read-side fallback, `COALESCE`, or tolerate-bad-shape
branch. Consumers stay strict and trust the data model.

- **Fail early, and it is absolute.** Check the bad case first; validate required inputs up front and
crash if they're missing; never add a default that masks a config or data problem. Prefer an explicit
`else` on non-trivial `if`s. **An invalid state stops the operation.** Do not get clever and process the
valid parts while routing around a broken one — there is no "tolerate one bad item and keep going." A
broken symlink, a malformed row, a missing setting is a stop condition.

- **Never add a tolerance fallback to silence a failure.** Do not wrap a failing command in `|| true`, do
not write `try/except: pass`, do not default a missing value to make an error go away. These mask the
real problem instead of fixing it (ADR 0015). If a hard stop is too opaque, make the stop *clearer* (an
explicit error that names what's broken — ADR 0013), never make it *tolerant*.
- Worked example: in `scripts/server/release-utils.sh`, `release_is_referenced` aborts the
`cleanup_unreferenced_releases` sweep under `set -euo pipefail` if an instance's `current` symlink is
broken. The tempting "fix" is `readlink -f … || true` to limp past it. That is wrong: a broken
`current` symlink is an invalid state, so stopping is the correct outcome. Improve the *diagnosis*
(name the bad instance), not the tolerance.

- **Every caught exception is persisted, once (ADR 0019 + 0001).** Errors live in the `AppError` table,
not just stdout. Use the two-arm dedup pattern so a single failure is logged once as it unwinds:

```python
from apps.workflow.exceptions import AlreadyLoggedException
from apps.workflow.services.error_persistence import persist_app_error

try:
operation()
except AlreadyLoggedException:
raise # already persisted upstream — pass through unchanged
except Exception as exc:
err = persist_app_error(exc) # MANDATORY
raise AlreadyLoggedException(exc, err.id) from exc
```

- **Backend owns data; frontend owns presentation (ADR 0020).** Anything involving the DB, business
rules, or external systems is backend. Static UI constants, layout, and ergonomics are frontend. The
boundary is the *kind of value*, not the layer of code.

- **Frontend talks to the API only through the generated client (ADR 0021).** All HTTP goes through
`frontend/src/api/generated/api.ts`; types come from the OpenAPI schema (`z.infer<typeof schemas.X>`).
No raw `fetch`/`axios`, no manual response typing, no hand-editing generated files. A missing endpoint
is a backend request, never a frontend workaround. After a backend schema change, regenerate with
`npm run update-schema && npm run gen:api`.

- **Keep business logic in service classes; keep views thin.**

## Code-style gotchas

- Python is formatted with **Black (line length 88)** + **isort**, and type-checked under **strict MyPy**
(`bash scripts/check_mypy.sh` is authoritative). New code must be fully type-clean.
- **Never hand-edit `__init__.py`** — it's autogenerated; run `python scripts/update_init.py` after
adding or removing a Python module.
- **Never hand-add entries to `mypy-baseline.txt`** — it only shrinks. A `# type: ignore[code]` needs the
specific error code plus a justification comment on the same line.
- **Never hand-edit generated artifacts** — the generated API client (above) and `.codesight/` files are
regenerated by tooling.
- Keep migrations small and reviewable; prefer a schema change over a code workaround that masks a data
shape problem.
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ repos:
files: '\.py$'
types: [python]

- id: shellcheck
name: Lint shell scripts with shellcheck
entry: shellcheck
language: system
types: [shell]
args: [--external-sources]

- id: check-naive-local-dates
name: Forbid timezone.now().date() — use timezone.localdate()
entry: poetry run python scripts/check_naive_local_dates.py
Expand Down
7 changes: 7 additions & 0 deletions .shellcheckrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Follow `source`d files so shared libraries are analysed, not guessed at.
external-sources=true
# Resolve `source "$SCRIPT_DIR/common.sh"` relative to each script's own
# directory. Without this, every script that sources scripts/server/common.sh
# (or release-utils.sh) trips SC1091 "not following" and the downstream
# SC2153 "INSTANCES_DIR may not be assigned" false positives.
source-path=SCRIPTDIR
40 changes: 40 additions & 0 deletions apps/workflow/tests/test_build_id.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import os
from unittest import mock

from django.core.exceptions import ImproperlyConfigured
from django.test import SimpleTestCase

from docketworks.settings import _validate_sha, read_build_id

VALID_SHA = "0123456789abcdef0123456789abcdef01234567"


class ValidateShaTests(SimpleTestCase):
def test_accepts_40_char_hex_sha(self) -> None:
self.assertEqual(_validate_sha(VALID_SHA, "test"), VALID_SHA)

def test_strips_surrounding_whitespace(self) -> None:
self.assertEqual(_validate_sha(f" {VALID_SHA}\n", "test"), VALID_SHA)

def test_rejects_empty(self) -> None:
with self.assertRaises(ImproperlyConfigured):
_validate_sha("", "test")

def test_rejects_short_hash(self) -> None:
with self.assertRaises(ImproperlyConfigured):
_validate_sha("0123abc", "test")

def test_rejects_non_hex(self) -> None:
with self.assertRaises(ImproperlyConfigured):
_validate_sha("z" * 40, "test")


class ReadBuildIdTests(SimpleTestCase):
def test_returns_validated_env_sha(self) -> None:
with mock.patch.dict(os.environ, {"DOCKETWORKS_BUILD_SHA": VALID_SHA}):
self.assertEqual(read_build_id(), VALID_SHA)

def test_rejects_invalid_env_sha(self) -> None:
with mock.patch.dict(os.environ, {"DOCKETWORKS_BUILD_SHA": "not-a-sha"}):
with self.assertRaises(ImproperlyConfigured):
read_build_id()
181 changes: 160 additions & 21 deletions apps/workflow/tests/test_xero_instance_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,40 @@
SERVER_README = REPO_ROOT / "scripts" / "server" / "README.md"
PRODUCTION_SETUP_DOC = REPO_ROOT / "docs" / "instance-setup-production.md"
DEMO_SETUP_DOC = REPO_ROOT / "docs" / "instance-setup-demo.md"
DW_RUN_SCRIPT = REPO_ROOT / "scripts" / "server" / "dw-run.sh"
RELEASE_UTILS = REPO_ROOT / "scripts" / "server" / "release-utils.sh"
GUNICORN_TEMPLATE = (
REPO_ROOT
/ "scripts"
/ "server"
/ "templates"
/ "gunicorn-instance.service.template"
)
CELERY_WORKER_TEMPLATE = (
REPO_ROOT
/ "scripts"
/ "server"
/ "templates"
/ "celery-worker-instance.service.template"
)
CELERY_BEAT_TEMPLATE = (
REPO_ROOT
/ "scripts"
/ "server"
/ "templates"
/ "celery-beat-instance.service.template"
)
NGINX_TEMPLATE = (
REPO_ROOT / "scripts" / "server" / "templates" / "nginx-instance.conf.template"
)
BACKUP_TEMPLATE = (
REPO_ROOT
/ "scripts"
/ "server"
/ "templates"
/ "backup-db-instance.service.template"
)
SETTINGS_FILE = REPO_ROOT / "docketworks" / "settings.py"


class XeroInstanceTemplateTests(SimpleTestCase):
Expand Down Expand Up @@ -66,7 +100,7 @@ def test_instance_script_requires_and_loads_xero_app_fixture(self):

self.assertIn("xero-apps.json.template", content)
self.assertIn(
'call_command("loaddata", "apps/workflow/fixtures/xero_apps.json")',
"call_command('loaddata', '$XERO_APPS_FIXTURE')",
content,
)
self.assertIn(
Expand Down Expand Up @@ -141,9 +175,109 @@ def test_instance_script_only_seeds_missing_db_config(self) -> None:
def test_instance_script_rejects_seed_for_existing_checkout(self) -> None:
content = INSTANCE_SCRIPT.read_text()

self.assertIn('[[ -d "$INSTANCE_DIR/.git" && "$SEED" == "true" ]]', content)
self.assertIn('[[ "$IS_EXISTING" == "true" && "$SEED" == "true" ]]', content)
self.assertIn("--seed is only valid when creating a new instance", content)

def test_instance_script_uses_shared_releases_not_instance_checkouts(self) -> None:
content = INSTANCE_SCRIPT.read_text()

self.assertIn('source "$SCRIPT_DIR/release-utils.sh"', content)
self.assertIn('ensure_release "$TARGET_SHA"', content)
self.assertIn('switch_instance_release "$INSTANCE" "$TARGET_SHA"', content)
self.assertNotIn('git -C "$INSTANCE_DIR" init', content)
self.assertNotIn('git -C "$INSTANCE_DIR" checkout', content)
self.assertNotIn("Building frontend for instance", content)

def test_instance_secret_fixtures_are_instance_private(self) -> None:
content = INSTANCE_SCRIPT.read_text()

self.assertIn('local fixture_dir="$instance_dir/.fixtures"', content)
self.assertIn(
'local AI_PROVIDERS_FIXTURE="$INSTANCE_DIR/.fixtures/ai_providers.json"',
content,
)
self.assertIn(
'local XERO_APPS_FIXTURE="$INSTANCE_DIR/.fixtures/xero_apps.json"', content
)
self.assertNotIn(
"$instance_dir/apps/workflow/fixtures/ai_providers.json",
content,
)
self.assertNotIn(
"$instance_dir/apps/workflow/fixtures/xero_apps.json",
content,
)

def test_deploy_prepares_one_shared_release_for_all_targets(self) -> None:
content = DEPLOY_SCRIPT.read_text()

self.assertIn('TARGET_REF="origin/main"', content)
self.assertIn('TARGET_SHA="$(resolve_release_ref "$TARGET_REF")"', content)
self.assertIn('ensure_release "$TARGET_SHA"', content)
self.assertIn('switch_instance_release "$instance" "$TARGET_SHA"', content)
self.assertNotIn("Updating shared Python dependencies", content)
self.assertNotIn("Updating shared node_modules", content)
self.assertNotIn("Building frontend", content)
self.assertNotIn('git -C "$inst_dir" pull', content)

def test_release_utils_builds_immutable_release_artifacts(self) -> None:
content = RELEASE_UTILS.read_text()

self.assertIn("RELEASES_DIR", content)
self.assertIn("git -C '$LOCAL_REPO' archive '$sha'", content)
self.assertIn("printf '%s\\n' '$sha' > '$tmp_dir/.release-sha'", content)
self.assertIn("python3.12 -m venv '$tmp_dir/.venv'", content)
self.assertIn("npm run check:typed-router", content)
self.assertIn("npm run build", content)
self.assertIn("npm run manual:build", content)
self.assertIn("rm -rf node_modules", content)
self.assertIn("touch '$tmp_dir/.complete'", content)

def test_runtime_templates_use_current_release(self) -> None:
for template in [
GUNICORN_TEMPLATE,
CELERY_WORKER_TEMPLATE,
CELERY_BEAT_TEMPLATE,
]:
content = template.read_text()
self.assertIn(
"WorkingDirectory=/opt/docketworks/instances/__INSTANCE__/current",
content,
)
self.assertIn(
"/opt/docketworks/instances/__INSTANCE__/current/.venv/bin/", content
)
self.assertIn("Environment=PYTHONDONTWRITEBYTECODE=1", content)
self.assertNotIn("/opt/docketworks/.venv/bin/", content)

nginx = NGINX_TEMPLATE.read_text()
self.assertIn(
"/opt/docketworks/instances/__INSTANCE__/current/frontend/dist", nginx
)
self.assertIn("/opt/docketworks/instances/__INSTANCE__/mediafiles/", nginx)

backup = BACKUP_TEMPLATE.read_text()
self.assertIn(
"ExecStart=/opt/docketworks/instances/__INSTANCE__/current/scripts/backup_db.sh __INSTANCE__",
backup,
)

def test_dw_run_uses_current_release_and_instance_env(self) -> None:
content = DW_RUN_SCRIPT.read_text()

self.assertIn('CURRENT_DIR="$INSTANCE_DIR/current"', content)
self.assertIn("source '$CURRENT_DIR/.venv/bin/activate'", content)
self.assertIn("source '$INSTANCE_DIR/.env'", content)
self.assertIn("cd '$CURRENT_DIR'", content)
self.assertIn("PYTHONDONTWRITEBYTECODE=1", content)

def test_build_id_reads_release_sha_before_git(self) -> None:
content = SETTINGS_FILE.read_text()

self.assertIn('os.environ.get("DOCKETWORKS_BUILD_SHA"', content)
self.assertIn('release_sha_file = BASE_DIR / ".release-sha"', content)
self.assertIn('["git", "rev-parse", "HEAD"]', content)

def test_credentials_file_stays_root_owned_before_root_source(self) -> None:
common_content = COMMON_SCRIPT.read_text()
instance_content = INSTANCE_SCRIPT.read_text()
Expand Down Expand Up @@ -183,7 +317,7 @@ def test_credentials_file_stays_root_owned_before_root_source(self) -> None:

def test_node_major_parsing_accepts_patch_versions(self) -> None:
common_content = COMMON_SCRIPT.read_text()
deploy_content = DEPLOY_SCRIPT.read_text()
release_utils_content = RELEASE_UTILS.read_text()
server_setup_content = SERVER_SETUP_SCRIPT.read_text()

self.assertIn("node_major_from_nvmrc()", common_content)
Expand All @@ -193,16 +327,10 @@ def test_node_major_parsing_accepts_patch_versions(self) -> None:
common_content,
)
self.assertIn(
'REQUIRED_NODE_MAJOR="$(node_major_from_nvmrc '
'"$LOCAL_REPO/frontend/.nvmrc")',
deploy_content,
)
self.assertIn(
'REQUIRED_NODE_MAJOR="$(node_major_from_nvmrc '
'"$LOCAL_REPO/frontend/.nvmrc")',
server_setup_content,
"sed -nE 's/^[[:space:]]*v?([0-9]+).*/\\1/p' .nvmrc",
release_utils_content,
)
self.assertNotIn("tr -d 'v[:space:]'", deploy_content)
self.assertNotIn("tr -d 'v[:space:]'", release_utils_content)
self.assertNotIn("tr -d 'v[:space:]'", server_setup_content)

for nvmrc_value in ["18", "v18", "18.2.0", "v18.2.0", " v18.2.0"]:
Expand Down Expand Up @@ -257,16 +385,27 @@ def test_xero_default_user_id_docs_match_required_create_time_workflow(
self.assertNotIn("Copy the relevant user ID into credentials.env", docs)
self.assertNotIn("then run `instance.sh reconfigure`", docs)

def test_deploy_restores_typed_router_after_drift_detection(self) -> None:
content = DEPLOY_SCRIPT.read_text()
def test_release_cleanup_is_deploy_integrated_and_reachability_based(self) -> None:
deploy_content = DEPLOY_SCRIPT.read_text()
release_utils_content = RELEASE_UTILS.read_text()

self.assertIn("--cleanup-releases", deploy_content)
self.assertIn("cleanup_stale_release_builds", deploy_content)
self.assertIn('cleanup_unreferenced_releases "$TARGET_SHA"', deploy_content)
self.assertIn("release_is_referenced()", release_utils_content)
self.assertIn(
"server generated a different frontend/src/typed-router.d.ts",
content,
'read_env_value "$instance_dir/deploy-state.env" PREVIOUS_SHA',
release_utils_content,
)
self.assertIn(
'git -C "$instance_dir" restore --source=HEAD -- '
"frontend/src/typed-router.d.ts",
content,
self.assertIn("Removing unreferenced release", release_utils_content)

def test_typed_router_drift_is_checked_in_release_build(self) -> None:
deploy_content = DEPLOY_SCRIPT.read_text()
release_utils_content = RELEASE_UTILS.read_text()

self.assertIn("npm run check:typed-router", release_utils_content)
self.assertNotIn(
"server generated a different frontend/src/typed-router.d.ts",
deploy_content,
)
self.assertIn('FAILED_INSTANCES+=("$instance")', content)
self.assertNotIn("frontend/src/typed-router.d.ts", deploy_content)
Loading
Loading