ldraney/claude-custom

Fork 0

refactor: project-scoped context loading in session-start-context.sh #144

Merged

forgejo_admin merged 1 commit from 143-refactor-session-start-context-sh-projec into main

2026-03-22 06:56:31 +00:00

forgejo_admin commented

2026-03-21 20:27:01 +00:00

Contributor

Summary

Replaces the project detection mechanism with a single /repos/{name} endpoint lookup and scopes all downstream context (plan TOCs, semantic search, board items, project context) to the detected project. Falls back to current cross-project behavior when no project is detected. Maintains fail-open pattern throughout.

Changes

hooks/session-start-context.sh:
- 20a: Replaced project detection (iterating all projects with URL normalization) with a single GET /repos/{repo-name} call. Extracts repo name via basename. Moved detection early (after platform detection) so detected_project_slug is available for all scoping.
- 20b: Plan TOC injection now checks detected_project_slug first -- matching project gets full TOC, non-matching projects get one-liner. Falls back to board activity filter when no project detected.
- 20c: Semantic search URL gets &project=${detected_project_slug} appended when a project is detected.
- 20d: Board items show detailed titles (in_progress, qa, needs_approval) for matching board, one-line summary counts for others. Aggregated counts when no project detected.
- 20e: Replaced raw HTML dump (first 40 lines of stripped project page) with structured TOC-based context: project name, user stories pointer, page sections list, targeted access instructions.

Test Plan

bash -n hooks/session-start-context.sh passes
Smoke tested with claude-custom repo as cwd: correctly detects pal-e-agency via /repos/claude-custom, shows structured TOC context, scoped board items with detailed titles, scoped plan TOCs
Smoke tested with /tmp (non-git directory): correctly falls back to unscoped cross-project behavior
Verified fail-open: /repos/nonexistent-repo returns 404, detected_project_slug stays empty, full cross-project loading occurs
Net change: +148/-125 lines. Token output reduced by eliminating raw HTML dump.

Review Checklist

bash -n syntax check passes
Fail-open pattern maintained (|| true on all fallible commands)
set -eo pipefail preserved
No changes to personality injection, SOP loading, core SOPs, template reference, or bug tracking instructions
Repos endpoint response shape verified: {"slug": "...", "project": {"slug": "...", "name": "..."}}
Both project-detected and no-project-detected paths tested end-to-end

Closes #143

## Summary Replaces the project detection mechanism with a single `/repos/{name}` endpoint lookup and scopes all downstream context (plan TOCs, semantic search, board items, project context) to the detected project. Falls back to current cross-project behavior when no project is detected. Maintains fail-open pattern throughout. ## Changes - `hooks/session-start-context.sh`: - **20a**: Replaced project detection (iterating all projects with URL normalization) with a single `GET /repos/{repo-name}` call. Extracts repo name via `basename`. Moved detection early (after platform detection) so `detected_project_slug` is available for all scoping. - **20b**: Plan TOC injection now checks `detected_project_slug` first -- matching project gets full TOC, non-matching projects get one-liner. Falls back to board activity filter when no project detected. - **20c**: Semantic search URL gets `&project=${detected_project_slug}` appended when a project is detected. - **20d**: Board items show detailed titles (in_progress, qa, needs_approval) for matching board, one-line summary counts for others. Aggregated counts when no project detected. - **20e**: Replaced raw HTML dump (first 40 lines of stripped project page) with structured TOC-based context: project name, user stories pointer, page sections list, targeted access instructions. ## Test Plan - `bash -n hooks/session-start-context.sh` passes - Smoke tested with `claude-custom` repo as cwd: correctly detects `pal-e-agency` via `/repos/claude-custom`, shows structured TOC context, scoped board items with detailed titles, scoped plan TOCs - Smoke tested with `/tmp` (non-git directory): correctly falls back to unscoped cross-project behavior - Verified fail-open: `/repos/nonexistent-repo` returns 404, `detected_project_slug` stays empty, full cross-project loading occurs - Net change: +148/-125 lines. Token output reduced by eliminating raw HTML dump. ## Review Checklist - [x] `bash -n` syntax check passes - [x] Fail-open pattern maintained (`|| true` on all fallible commands) - [x] `set -eo pipefail` preserved - [x] No changes to personality injection, SOP loading, core SOPs, template reference, or bug tracking instructions - [x] Repos endpoint response shape verified: `{"slug": "...", "project": {"slug": "...", "name": "..."}}` - [x] Both project-detected and no-project-detected paths tested end-to-end ## Related - Closes #143

forgejo_admin added 1 commit

2026-03-21 20:27:01 +00:00

refactor: project-scoped context loading in session-start-context.sh d2c01cfd83

Replace the project detection mechanism with a single /repos/{name} endpoint
lookup instead of iterating all projects with URL normalization. Scope plan
TOCs, semantic search, and board items to the detected project when available,
falling back to current cross-project behavior when no project is detected.

Changes (issue #143):
- 20a: Project detection via /repos/{repo-name} endpoint
- 20b: Plan TOC injection scoped to matching project
- 20c: Semantic search scoped with &project= parameter
- 20d: Board items detailed for matching board, summary for others
- 20e: Structured project context (TOC sections + user stories pointer)
  replacing raw HTML dump

Closes #143

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-21 20:27:36 +00:00

Author

Contributor

Self-Review

Reviewed the diff against all five sub-tasks in issue #143. No blocking issues found.

20a (Project Detection): Clean single-endpoint lookup via /repos/{repo-name}. Uses basename to extract repo name from git remote URL, curl -sf with || true for fail-open. Moved early in the function so detected_project_slug is available for all downstream scoping.

20b (Plan TOC Scoping): When detected_project_slug is set, only the matching project's plan gets full TOC injection. Non-matching plans get one-liner. When empty, falls back to original board activity filter. Correct priority ordering.

20c (Semantic Search Scoping): Conditionally appends &project=${detected_project_slug} to semantic search URL. Clean.

20d (Board Item Scoping): Matching board shows item titles grouped by column (in_progress, qa, needs_approval). Other boards show summary counts. No-project path preserves existing aggregated behavior.

20e (Structured Project Context): Fetches project page TOC, checks for user-stories anchor, lists all sections, provides get_section() pointers. Eliminates the raw HTML dump (40 lines of stripped HTML). Multiple fallback paths if TOC fetch fails.

Fail-open: All curl calls guarded with || true, all jq calls have validity checks. set -eo pipefail preserved. Final main 2>/dev/null || exit 0 untouched.

Untouched sections confirmed: Personality injection, SOP loading, core SOPs, template reference, bug tracking instructions -- no changes.

Smoke tested: Both project-detected (claude-custom -> pal-e-agency) and no-project-detected (/tmp) paths produce correct output.

## Self-Review Reviewed the diff against all five sub-tasks in issue #143. No blocking issues found. **20a (Project Detection):** Clean single-endpoint lookup via `/repos/{repo-name}`. Uses `basename` to extract repo name from git remote URL, `curl -sf` with `|| true` for fail-open. Moved early in the function so `detected_project_slug` is available for all downstream scoping. **20b (Plan TOC Scoping):** When `detected_project_slug` is set, only the matching project's plan gets full TOC injection. Non-matching plans get one-liner. When empty, falls back to original board activity filter. Correct priority ordering. **20c (Semantic Search Scoping):** Conditionally appends `&project=${detected_project_slug}` to semantic search URL. Clean. **20d (Board Item Scoping):** Matching board shows item titles grouped by column (in_progress, qa, needs_approval). Other boards show summary counts. No-project path preserves existing aggregated behavior. **20e (Structured Project Context):** Fetches project page TOC, checks for `user-stories` anchor, lists all sections, provides `get_section()` pointers. Eliminates the raw HTML dump (40 lines of stripped HTML). Multiple fallback paths if TOC fetch fails. **Fail-open:** All curl calls guarded with `|| true`, all jq calls have validity checks. `set -eo pipefail` preserved. Final `main 2>/dev/null || exit 0` untouched. **Untouched sections confirmed:** Personality injection, SOP loading, core SOPs, template reference, bug tracking instructions -- no changes. **Smoke tested:** Both project-detected (claude-custom -> pal-e-agency) and no-project-detected (/tmp) paths produce correct output.

forgejo_admin commented

2026-03-21 20:30:02 +00:00

Author

Contributor

PR #144 Review

DOMAIN REVIEW

Tech stack: Bash shell script (Claude Code SessionStart hook). Domain checks: shell safety patterns, fail-open behavior, jq usage, curl error handling, variable scoping.

20a -- Project detection via /repos/{name}: Clean replacement. The basename approach correctly handles both HTTPS and SSH git URLs. The three-step validation (non-empty remote, non-empty repo_name, valid JSON with .project.slug) is sound. All steps have || true or 2>/dev/null guards.

20b -- Plan TOC scoping: The priority logic is clear: detected project wins, then board activity, then no TOC. However, the old fail-open clause for plans with no plan_project_slug has been dropped (see NITS below).

20c -- Semantic search scoping: Simple &project= parameter append. Clean.

20d -- Board items scoping: The matched-board detailed view (showing in_progress, qa, needs_approval item titles) is a significant improvement over raw counts. The other_board_lines summary is well-structured. The else branch preserves old aggregated behavior exactly.

20e -- Structured project context: TOC-based approach is a clear win over the old raw HTML dump + sed strip. The user-stories anchor detection is a nice touch. Falls back gracefully when TOC fetch fails.

General observations:

set -eo pipefail is preserved (not shown in diff, but present in file header)
|| true pattern maintained on all fallible commands in new code
No secrets, credentials, or hardcoded values introduced
cwd_remote variable declaration correctly moved from old position (line 373) to new position (line ~68); old declaration and entire old project detection block are fully removed -- no shadowing
matched_board convention board-${detected_project_slug} correctly matches boards-config.sh naming convention

BLOCKERS

None.

This is an infrastructure shell hook, not application code with a test harness. The bash -n syntax check and manual smoke testing (project-detected, no-project, fail-open with nonexistent repo) documented in the PR body are the appropriate validation level for this type of change. No BLOCKER criteria are triggered.

NITS

Dropped fail-open for orphan plans (20b): The old code had a final elif [[ -z "$plan_project_slug" ]]; then inject_toc=true clause that ensured plans without a project slug would always get TOC injection (fail-open -- "can't match to board, fail open for this plan"). This clause is missing from BOTH the project-scoped path AND the unscoped elif fallback path in the new code. If any active plan lacks a project.slug, it will silently lose its TOC injection. Consider restoring this fail-open clause in the unscoped path at minimum:
```
elif [[ -z "$plan_project_slug" ]]; then
    inject_toc=true
fi
```
Triple jq parse of repo_response (20a): The response is piped through jq three times (validation, slug extraction, name extraction). Could be a single jq call extracting both values, e.g.:
```
eval "$(echo "$repo_response" | jq -r '
    "detected_project_slug=" + (.project.slug // ""),
    "detected_project_name=" + (.project.name // "")
')"
```
Not a correctness issue -- just unnecessary subprocess spawns on every session start.
Inconsistent "active" column definitions: The project-scoped board path (20d) counts in_progress + qa + needs_approval as "active" items. The unscoped path only counts in_progress and todo. This is likely intentional (scoped = more detail) but the asymmetry could confuse consumers of the output.
origin remote assumption: Both old and new code use git remote get-url origin. Per CLAUDE.md, pal-e-platform uses forgejo as its remote name, not origin. This is NOT a regression (same behavior as before), but worth tracking as a known limitation.

SOP COMPLIANCE

Branch named after issue: 143-refactor-session-start-context-sh-projec (truncated but starts with 143)
PR body has Summary, Changes, Test Plan, Related sections
Related references plan slug: PR body says "Closes #143" but does not reference plan-pal-e-agency by slug
No secrets committed
No unnecessary file changes (single file, all changes are in scope)
Commit messages are descriptive (PR title is descriptive)
bash -n syntax validation documented as passing

PROCESS OBSERVATIONS

Deployment frequency: This is a local hook -- deploys instantly on merge (symlinked). Low risk.
Change failure risk: Low-medium. The fail-open pattern means worst case is degraded context (less information injected), not broken sessions. The orphan-plan nit (item 1) is the only path where behavior silently degrades vs. the old code.
Token reduction: The PR correctly notes that replacing raw HTML dump with structured TOC reduces token output. This is a meaningful improvement for session start latency and context window usage.
Net API calls: The new /repos/{name} lookup adds 1 API call but the old project detection fetched /projects (all projects) + iterated + fetched individual project pages. Net reduction in API calls for the common case.

VERDICT: APPROVED

## PR #144 Review ### DOMAIN REVIEW **Tech stack:** Bash shell script (Claude Code SessionStart hook). Domain checks: shell safety patterns, fail-open behavior, `jq` usage, `curl` error handling, variable scoping. **20a -- Project detection via `/repos/{name}`:** Clean replacement. The `basename` approach correctly handles both HTTPS and SSH git URLs. The three-step validation (non-empty remote, non-empty repo_name, valid JSON with `.project.slug`) is sound. All steps have `|| true` or `2>/dev/null` guards. **20b -- Plan TOC scoping:** The priority logic is clear: detected project wins, then board activity, then no TOC. However, **the old fail-open clause for plans with no `plan_project_slug` has been dropped** (see NITS below). **20c -- Semantic search scoping:** Simple `&project=` parameter append. Clean. **20d -- Board items scoping:** The matched-board detailed view (showing `in_progress`, `qa`, `needs_approval` item titles) is a significant improvement over raw counts. The `other_board_lines` summary is well-structured. The `else` branch preserves old aggregated behavior exactly. **20e -- Structured project context:** TOC-based approach is a clear win over the old raw HTML dump + `sed` strip. The `user-stories` anchor detection is a nice touch. Falls back gracefully when TOC fetch fails. **General observations:** - `set -eo pipefail` is preserved (not shown in diff, but present in file header) - `|| true` pattern maintained on all fallible commands in new code - No secrets, credentials, or hardcoded values introduced - `cwd_remote` variable declaration correctly moved from old position (line 373) to new position (line ~68); old declaration and entire old project detection block are fully removed -- no shadowing - `matched_board` convention `board-${detected_project_slug}` correctly matches `boards-config.sh` naming convention ### BLOCKERS None. This is an infrastructure shell hook, not application code with a test harness. The `bash -n` syntax check and manual smoke testing (project-detected, no-project, fail-open with nonexistent repo) documented in the PR body are the appropriate validation level for this type of change. No BLOCKER criteria are triggered. ### NITS 1. **Dropped fail-open for orphan plans (20b):** The old code had a final `elif [[ -z "$plan_project_slug" ]]; then inject_toc=true` clause that ensured plans without a project slug would always get TOC injection (fail-open -- "can't match to board, fail open for this plan"). This clause is missing from BOTH the project-scoped path AND the unscoped `elif` fallback path in the new code. If any active plan lacks a `project.slug`, it will silently lose its TOC injection. Consider restoring this fail-open clause in the unscoped path at minimum: ```bash elif [[ -z "$plan_project_slug" ]]; then inject_toc=true fi ``` 2. **Triple `jq` parse of `repo_response` (20a):** The response is piped through `jq` three times (validation, slug extraction, name extraction). Could be a single `jq` call extracting both values, e.g.: ```bash eval "$(echo "$repo_response" | jq -r ' "detected_project_slug=" + (.project.slug // ""), "detected_project_name=" + (.project.name // "") ')" ``` Not a correctness issue -- just unnecessary subprocess spawns on every session start. 3. **Inconsistent "active" column definitions:** The project-scoped board path (20d) counts `in_progress + qa + needs_approval` as "active" items. The unscoped path only counts `in_progress` and `todo`. This is likely intentional (scoped = more detail) but the asymmetry could confuse consumers of the output. 4. **`origin` remote assumption:** Both old and new code use `git remote get-url origin`. Per CLAUDE.md, `pal-e-platform` uses `forgejo` as its remote name, not `origin`. This is NOT a regression (same behavior as before), but worth tracking as a known limitation. ### SOP COMPLIANCE - [x] Branch named after issue: `143-refactor-session-start-context-sh-projec` (truncated but starts with `143`) - [x] PR body has Summary, Changes, Test Plan, Related sections - [ ] Related references plan slug: PR body says "Closes #143" but does not reference `plan-pal-e-agency` by slug - [x] No secrets committed - [x] No unnecessary file changes (single file, all changes are in scope) - [x] Commit messages are descriptive (PR title is descriptive) - [x] `bash -n` syntax validation documented as passing ### PROCESS OBSERVATIONS - **Deployment frequency:** This is a local hook -- deploys instantly on merge (symlinked). Low risk. - **Change failure risk:** Low-medium. The fail-open pattern means worst case is degraded context (less information injected), not broken sessions. The orphan-plan nit (item 1) is the only path where behavior silently degrades vs. the old code. - **Token reduction:** The PR correctly notes that replacing raw HTML dump with structured TOC reduces token output. This is a meaningful improvement for session start latency and context window usage. - **Net API calls:** The new `/repos/{name}` lookup adds 1 API call but the old project detection fetched `/projects` (all projects) + iterated + fetched individual project pages. Net reduction in API calls for the common case. ### VERDICT: APPROVED