Lewati ke isi

Your First PR

Purpose

A walkthrough of the HUPH contribution workflow — how we branch, commit, test, and merge. Follow this page for your first change (and every change after) so your PR lands smoothly with the rest of the team.

Prerequisites

  • Local environment working (see setup)
  • git configured with your identity
  • Repo access (you should already be able to push to github.com/wabiwabo/huph)

Overview of the workflow

Text Only
main (protected)
  │
  ├─ spec → plan → branch → TDD loop → PR → review → merge
  │
  └─ Every feature follows this rhythm

Big features follow a spec → plan → implement flow with documents in docs/superpowers/specs/ and docs/superpowers/plans/. Small fixes (1-2 file changes, clear scope) can skip straight to a branch + PR.

Branch naming

Prefix Use for Example
feat/ New functionality feat/lead-capture-phase2a
fix/ Bug fixes fix/notifications-dedup
docs/ Documentation changes docs/revamp-pass-1
refactor/ Non-behavior-changing code reorganization refactor/extract-jwt-verifier
chore/ Deps, CI, tooling chore/bump-mkdocs-material

Create from current main:

Bash
git checkout main
git pull origin main
git checkout -b feat/<short-descriptive-name>

Commit message style (Conventional Commits)

All commits follow Conventional Commits with an optional scope. Examples from recent history:

  • feat(db): composite index on notifications dedup lookup
  • fix(notifications): dedup rapid-fire fanout within 30s window
  • docs(plan): API HTTP auth middleware implementation plan — 15 TDD tasks
  • docs(spec): API HTTP auth middleware design — defense in depth for Express
  • feat(db): composite index on (cluster_id, status, last_message_at)

Rules

  1. First line ≤ 72 characters — no period at end
  2. Scope is optional but preferred — api, admin, db, notifications, leads, auth, audit, etc.
  3. Present tense, imperative mood — "add X", not "added X"
  4. Body paragraph(s) for the why — what problem does this solve, what alternative was considered, what gotchas did you hit
  5. Footer: Co-Authored-By: lines for pair-programming or AI assistance (HUPH commits by convention include Co-Authored-By: Claude Opus 4.7 (1M context) when Claude was involved)

Example

Text Only
feat(leads): atomic phone-merge in Lead Capture Phase 2A

The leadStore.upsert uses INSERT ... ON CONFLICT with a CASE clause
that recomputes status inline from COALESCE'd values. Without the
inline recompute, concurrent writers can race and a stale
'incomplete' status overwrites a row that should be 'new'. Uses the
`xmax = 0` postgres trick to distinguish INSERT vs UPDATE in the
RETURNING clause.

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

TDD expectations

For apps/api:

  1. Write the failing test first. Run it, verify it fails with the expected error message.
  2. Implement the minimum code to make the test pass.
  3. Run the test suite — all tests (not just yours) must pass before commit.
  4. Commit test + implementation together or as two adjacent commits in the same PR.

For apps/admin: no TDD (no test infra). Verify via npx tsc --noEmit, npm run build, and browser smoke. See running-tests.en.md.

Spec → plan → implement pattern

For any feature touching more than 2-3 files or affecting a cross- cutting concern (auth, realtime, DB schema, external integration), follow the structured flow:

  1. Brainstorm — use superpowers:brainstorming skill to explore requirements
  2. Write spec to docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md
  3. Write plan to docs/superpowers/plans/YYYY-MM-DD-<topic>.md
  4. Execute plan task-by-task (subagent-driven or inline) with TDD per task
  5. Open PR referencing both spec and plan in the description

Specs and plans are commit-reviewable — don't skip them for anything non-trivial. See existing docs/superpowers/plans/ for reference.

Bilingual documentation sync rule

If you edit a file in docs/guide/ (non-dev, Bahasa Indonesia is source of truth), you must also update the English mirror file in the same commit. Rules:

  1. Edit the .id.md first (source of truth for guide/).
  2. Bump last_synced in the frontmatter.
  3. Update the .en.md mirror, bump its last_synced too.
  4. Commit both files together.

If you can't update the mirror in the same commit (e.g. you don't speak Indonesian), explicitly flag it in the PR body:

TODO: EN mirror for guide/operators/inbox.en.md needs update after ID changes — flagging for follow-up.

The reviewer will catch it. No CI enforcement yet — manual discipline.

For dev/ docs (EN source), ID mirrors are not required in Pass 1.

Pre-PR checklist

Before opening a PR:

  • [ ] Tests pass: npm run test -w apps/api
  • [ ] TypeScript clean: cd apps/admin && npx tsc --noEmit
  • [ ] If docs touched: cd /opt/huph && docs-env/bin/mkdocs build --strict passes
  • [ ] Commit messages follow conventional commits
  • [ ] No secrets in the diff (git diff | grep -iE "api[_-]?key|secret|password")
  • [ ] Branch rebased on latest main

Open the PR

Bash
git push -u origin feat/<branch-name>
gh pr create --title "feat(scope): short summary" --body "<see below>"

PR body template:

Markdown
## Summary

One-paragraph overview. Why this change? What does it enable?

## Changes

- Bullet list of key files/subsystems touched
- Keep it scannable

## Test plan

- [ ] `npm run test -w apps/api` passes (<N> tests)
- [ ] Manual smoke: log in → Conversations → send test message → verify
- [ ] (if docs) `mkdocs build --strict` passes

## Spec / Plan

- Spec: docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md
- Plan: docs/superpowers/plans/YYYY-MM-DD-<topic>.md

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Code review

  • A teammate reviews your PR
  • Address feedback via new commits (do not force-push during review unless the reviewer asks)
  • Once approved, merge to main (squash or merge commit — team has preferred squash merge recently for clean history)
  • Delete the feature branch after merge

Checklist for new admin pages

When adding a new page under apps/admin/src/app/, especially one that deals with cluster-scoped data, governance workflows, or audit trails, verify the following BEFORE submitting PR. Each item maps to a real bug found and fixed in /knowledge/faq (commit 775f663):

1. Counts come from the right source

If your page uses KbClusterTabs (or any tab/filter component that shows counts), verify the counts come from YOUR page's data model, not from a generic upstream component default.

Example mistake: FAQ page used KbClusterTabs which falls back to Dify dataset chunk counts when no explicit faqCounts prop is passed. The FAQ tab "CASS" showed Dify-document count instead of faq_local row count.

Fix pattern: shared component takes optional override prop (e.g. faqCounts?: Record<string, number>); caller passes per-context counts. Default falls back to legacy behavior.

2. Actor identities resolve to display names

If a database column references an actor (created_by, updated_by, last_verified_by, assigned_to_id, etc.) and the UI displays this field, the API response MUST include a resolved display name (e.g. last_verified_by_name) via LEFT JOIN admin_users.

NEVER display raw UUIDs in the UI. Audit trail accountability requires a human-readable identity.

Pattern:

SQL
SELECT fl.*, au.full_name AS last_verified_by_name
  FROM faq_local fl
  LEFT JOIN admin_users au ON au.id = fl.last_verified_by

LEFT JOIN handles deleted actors gracefully (returns null name).

3. Cluster-scoped fields have role-aware visibility

If a form has a cluster_id (or equivalent) field, it MUST behave differently per role:

  • Cluster-scoped roles (marketing_counselor, faculty_admin): field auto-locked to user's cluster, dropdown HIDDEN. Show a read-only pill with cluster code.
  • Multi-cluster roles (super_admin, system_admin, marketing_admin): dropdown with all options.
  • Pre-set value from active filter context if available.

Without this guard, cluster-scoped users can accidentally route data to the wrong cluster (e.g., create a "general" FAQ that should have been CASS-specific).

Pattern: fetch user role + cluster from /api/me, conditionally render dropdown vs locked pill. See apps/admin/src/app/knowledge/faq/page.tsx (around line 1080).

4. Confirmation dialogs for destructive/audit-changing actions

Any action that mutates audit-trackable state (verify/unverify, toggle active/inactive, delete) needs a ConfirmDialog to prevent accidental clicks in long lists.

Pattern: use the shared ConfirmDialog from @/components/shared/confirm-dialog. Distinguish destructive vs default variants. Include the affected resource name in the message so the user can confirm what they're about to change.

5. Source URL grounding for content

If content is "official" claim (e.g., FAQ answer, KB document description), every record MUST cite a source URL. Enforce in form validation: - Tier 1 (UPH official): uph.edu/*, one.uph.edu, usm.uph.edu, apply.uph.edu, residences.uph.edu - Tier 2 (cross-cited): kompas.com, danacita.co.id - Show tier badge live in form (emerald/amber/unknown/invalid).

See apps/admin/src/app/knowledge/faq/page.tsx classifyUrl() function.

Gotchas

  • NextAuth uses JWE, not signed JWT. Any code touching session tokens MUST use next-auth/jwt encode/decode, not jsonwebtoken.sign/verify. Silently fails otherwise.
  • No --no-verify to skip pre-commit hooks unless the reviewer explicitly asks. If a hook fails, fix the underlying issue.
  • Don't modify docs/superpowers/specs|plans/ of other people's ongoing work. Only touch your own.
  • uph-codebase/ is off-limits — legacy Laravel reference only.

See also