# CONTRIBUTING.md — My Book Showroom

> **A DMRS Tech Product** | US English throughout
> Read this entire document before writing any code.
> Questions? Contact the DMRS Tech lead developer before starting work.
> Last updated: 2026-05-28 / Session 159 (§8 updated: references ORCHESTRATOR.md and WRITE-PROTOCOL.md as authoritative AI session sources; prior: 2026-04-25 — R-WRITE-1–R-WRITE-4 write rules, §8 AI rules added, §9 Code Review renumbered).

---

## Table of Contents

1. [First Day Checklist](#1-first-day-checklist)
2. [Coding Standards](#2-coding-standards)
3. [Git Workflow](#3-git-workflow)
4. [Commit Message Format](#4-commit-message-format)
5. [Pull Request Process](#5-pull-request-process)
6. [Definition of Done](#6-definition-of-done)
7. [Phase and Sprint Discipline](#7-phase-and-sprint-discipline)
8. [Code Review Standards](#8-code-review-standards)

---

## 1. First Day Checklist

Before writing a single line of code:

- [ ] Read `PLUGIN-STRUCTURE.md` (or `SERVER-STRUCTURE.md` if server-side)
- [ ] Read `SECURITY.md` — non-negotiable
- [ ] Read `DEVELOPER-REFERENCE.md`
- [ ] Read `TESTING.md` and get your local environment running
- [ ] Read `AUDIT-HISTORY.md` — understand every finding and decision made before your sprint started
- [ ] Read Doc 1.0 (Master Index) — understand the three-context architecture
- [ ] Read the Doc 1 child document for your assigned context
- [ ] Read Doc 1d (Database Schema) — understand the data model
- [ ] Clone the repository and confirm tests pass on your local setup
- [ ] Confirm your assigned phase and sprint with the project lead
- [ ] Read `PLUGIN-STRUCTURE.md §5` to identify your phase's file boundary and which specific sprint stories you own
- [ ] Read `SprintStructure_v2_0_0.docx §3a–3c` to understand which phase you are working in and what the phase gate requires

---

## 2. Coding Standards

### PHP
- **Standard:** WordPress Coding Standards (WPCS) — enforced via PHP_CodeSniffer
- **Minimum PHP version:** 8.0
- **File encoding:** UTF-8, Unix line endings (LF), no BOM
- **Indentation:** Tabs (WordPress standard — not spaces)
- **Line length:** Soft limit 100 characters, hard limit 120
- **Opening braces:** Same line for control structures, new line for class/function declarations
- **Yoda conditions:** Required for comparisons (`'pro' === $tier` not `$tier === 'pro'`)
- **Type declarations:** Required on all new method parameters and return types
- **Docblocks:** Required on all public methods. Format: `@param`, `@return`, `@throws`
- **File headers:** Every PHP file begins with a file-level docblock. Use a single `File Path:` line containing the full relative path from the plugin root (e.g. `includes/admin/tabs/class-mbs-tab-general.php`). The `File Name:` line is deprecated — the filename is already present at the end of `File Path:` and the redundant line must be omitted from all new and modified files. See `DEVELOPER-REFERENCE.md §10` for the canonical format.

### JavaScript
- **Standard:** WordPress JavaScript Coding Standards
- **Pattern:** IIFE with explicit jQuery passing — always
- **No arrow functions at top level** (IE11 compatibility for admin where required)
- **`const` and `let`** — never `var` in new files or new code sections. Legacy files predating Sprint 9 contain `var` throughout and are exempt until a dedicated JS modernization sprint is scheduled. All new JS files and all new code blocks added to existing files must use `const`/`let` exclusively.
- **Namespace:** All MBS JS lives on `window.MBS`
- **F-003 — DOM construction:** Never use `innerHTML` assignment or `jQuery.html()` to insert server-originated data into the DOM. Always use `document.createElement()` + `element.textContent = value` + `parent.appendChild(child)`. This applies to AJAX result rendering, search result cards, and any dynamic list construction. A PR containing `innerHTML =` or `.html(` with variable content is automatically rejected regardless of surrounding context.

### PHP — PRO Feature Gate Output Rule (R10 / SEC-02)

When `mbs_is_pro()` returns false and a feature is PRO-only, the output rule is:

```php
// CORRECT — HTML comment only, no visible element
if ( ! mbs_is_pro() ) {
    return '<!-- MBS: Slider layout requires PRO license -->';
}

// WRONG — visible element to site visitors (GP-T-3 violation)
if ( ! mbs_is_pro() ) {
    return '<p class="mbs-pro-notice">Upgrade to PRO</p>';
}
```

The canonical comment format is `<!-- MBS: [feature description] requires PRO license -->`. The feature description must be human-readable (e.g. "Slider layout", "Series order badge", "coming_soon status badge"). This is enforced by automated grep at PR review time.

### PHP — Inline Style Prohibition (R6)

No `style="..."` attribute may appear in any PHP template, shortcode, or widget output, with one exception: the watermark hardening layers (which require `!important` for anti-tamper purposes and are specifically documented in `SECURITY.md §7`).

For dynamic sizing (e.g. proportional font sizes in a tag cloud): use predefined CSS class buckets (`mbs-cloud-xs` through `mbs-cloud-xl`) rather than inline `style="font-size:{N}px"`. A PR with a new inline style attribute on any element (other than the documented watermark exception) is rejected.

### PHP — Cover Rendering (R5)

All book cover images must be rendered via `MBS_Cover_Renderer::render($post_id)`. Never construct a raw `<img>` tag for a cover. Never call `get_the_post_thumbnail()` or `wp_get_attachment_image()` directly for cover display.

### PHP — Widget Registration

All new widgets must be registered in `bookshowroom.php` inside the `widgets_init` callback using `register_widget('MBS_WidgetClassName')`. Autoloading scans the `widgets/` directory but does not auto-register. Missing registration is a critical bug — the widget silently does not appear in Appearance → Widgets.

### CSS
- **BEM naming:** `.mbs-[block]__[element]--[modifier]`
- **Scoping:** All rules inside `.mbs-wrapper`
- **No `!important`** in `mbs-front.css` or `mbs-admin.css` — only in `mbs-compat.css`
- **No inline styles** in PHP templates — use CSS classes
- **CSS custom properties** for all dynamic/themeable values
- **Mobile first** — base styles for mobile, `@media (min-width: ...)` for larger screens

### SQL
- **Never write raw SQL** outside DAO classes (`includes/data/`)
- **Always use `$wpdb->prepare()`** for any query with user input
- **No `SELECT *`** — always name columns explicitly
- **Table names** always use `$wpdb->prefix . 'mbs_'` pattern

### US English
All code comments, variable names, docblocks, error messages, and log messages use US English.

---

## 3. Git Workflow

### Branch Names
| Branch type | Pattern | Example |
|---|---|---|
| Main (protected) | `main` | `main` |
| Development | `develop` | `develop` |
| Sprint | `sprint/N` | `sprint/8` |
| Feature | `feature/short-description` | `feature/grid-layout` |
| Bug fix | `fix/short-description` | `fix/license-deactivation` |
| Hotfix (production) | `hotfix/short-description` | `hotfix/license-ping-timeout` |

### Branch Rules
- `main` is production. No direct pushes. Merge only via PR from `develop` after full review.
- `develop` is the integration branch. Sprint branches merge here.
- Sprint branches branch from `develop` at sprint start.
- Feature branches branch from the sprint branch.
- Never branch from `main` except for hotfixes.

---

## 4. Commit Message Format

Format: `[type]([scope]): short description`

**Types:** feat, fix, refactor, test, docs, style, chore

**Scope:** The primary file or component affected.

**Examples:**
```
feat(license-client): add local fallback clear in deactivate() for Sprint 8
fix(settings): remove duplicate menu registration causing double render
refactor(tab-license): add __construct() to register AJAX hooks at boot time
docs(changelog): add Sprint 8 completion entry
```

**Rules:**
- Subject line: max 72 characters, imperative mood, no period at end
- Body (optional): explain the WHY, not the WHAT. Reference Doc 1 sections if relevant.
- Breaking changes: add `BREAKING CHANGE:` in the body

---

## 5. Pull Request Process

### Before Opening a PR
- [ ] All tests pass
- [ ] PHP_CodeSniffer shows zero errors
- [ ] No `var_dump()`, `error_log()`, or `print_r()` left in code
- [ ] No hardcoded credentials or license keys
- [ ] New features respect FREE/PRO gates — tested with both tier states
- [ ] All output escaped appropriately
- [ ] No SQL outside DAO classes
- [ ] No API calls to `www.dmrstech.com` outside `includes/api/` (importers in `includes/import/` and payment adapters in `includes/payment/` are the documented exceptions — neither may include license keys, HMACs, or api_secret values in their outbound requests)
- [ ] No `add_filter('mbs_is_pro', ...)` call in any file that ships to customers (see SEC-02 — this filter bypasses tamper enforcement and must only exist in local test code gated by `MBS_TESTING_MODE`)

### PR Description Template
```
## What this PR does
[Short description of the change]

## Doc 1 reference
[Which section of Doc 1.0 / 1a / 1b / 1c / 1d this implements]

## Phase and sprint
Phase [1 / 2 / 3] — Sprint [N] — files modified are within my phase boundary: [Yes / No — if No, see §7 exception process]

## Testing done
- [ ] Tested on FREE license
- [ ] Tested on PRO license
- [ ] Tested on local with GeneratePress (clean theme)
- [ ] Tested on local with [hostile theme if relevant]
- [ ] Unit tests pass
- [ ] Integration tests pass

## Screenshots (if UI change)
[Before / After screenshots]
```

### Review Requirements
- All PRs require at least one review approval before merge
- PRs that touch `includes/api/` or `includes/payment/` require the lead developer's approval
- PRs that touch `bookshowroom.php` require explicit approval with documented reason
- Self-merge is not permitted

### Merge Strategy
- Squash and merge for feature branches → sprint branch
- Merge commit for sprint branch → develop (preserves sprint history)
- Merge commit for develop → main (release merge)

---

## 6. Definition of Done

A feature is done when ALL of the following are true:

- [ ] Code implements the spec in the relevant Doc 1 document exactly
- [ ] Code passes all existing tests without modification to test expectations
- [ ] New tests written for new logic
- [ ] PHP_CodeSniffer clean (zero errors)
- [ ] Tested on FREE tier (feature disabled/degraded correctly)
- [ ] Tested on PRO tier (feature enabled correctly)
- [ ] No inline styles in front-end output (exception: documented watermark hardening — see SECURITY.md §7)
- [ ] PRO-gated features on FREE tier return only an HTML comment in the form `<!-- MBS: [feature] requires PRO license -->` — no visible element, no empty container (R10 / GP-T-3)
- [ ] All output escaped appropriately (`esc_html`, `esc_attr`, `esc_url`, `wp_kses_post`)
- [ ] No SQL outside DAO classes
- [ ] No cover rendering outside `MBS_Cover_Renderer::render()` (R5)
- [ ] No API calls to `www.dmrstech.com` outside `includes/api/` (importers in `includes/import/` and payment adapters in `includes/payment/` are the documented exceptions — neither may include license keys, HMACs, or api_secret values in their outbound requests)
- [ ] No JavaScript DOM manipulation via `innerHTML` or `jQuery.html()` with variable content (F-003)
- [ ] New widgets registered in `bookshowroom.php` `widgets_init` block (not just class-filed)
- [ ] No `add_filter('mbs_is_pro', ...)` in code that ships to customers (SEC-02)
- [ ] CSS custom properties used for any dynamic or themeable values — no hardcoded hex values in PHP output (R6)
- [ ] `CHANGELOG.md` updated with entry for this feature
- [ ] `AUDIT-HISTORY.md` updated if a bug is being fixed or an architectural decision made
- [ ] `DEVELOPER-REFERENCE.md` updated if a new class, hook, constant, or architectural pattern was introduced
- [ ] PR approved and merged to sprint branch

> **AI-orchestrated sessions:** `AUDIT-HISTORY.md`, `CHANGELOG.md`, and `DEVELOPER-REFERENCE.md`
> updates are handled exclusively by the Orchestrator and Housekeeping Agent, not by domain
> agents or developers working alongside AI. See `ORCHESTRATOR.md §Orchestrator-Exclusive Ownership`.
> For fully human-developer sessions, this checklist item remains the developer's responsibility.

---

## 7. Phase and Sprint Discipline

### Phase boundaries
Development follows a context-sequential approach with three completion phases. The phase you are working in determines which files you may touch. See `SprintStructure_v2_0_0.docx §3a–3c` for the full phase definitions, story assignments, and phase gate requirements.

| Phase | Context | What you may touch |
|---|---|---|
| **Phase 1** | Context 1 — Admin Settings | `includes/admin/`, `includes/api/`, `includes/import/`, `includes/export/`, assets/js/adminjs/, assets/css/mbs-admin*.css, shared helper interfaces |
| **Phase 2** | Context 2 — Book Item Editor | `includes/metaboxes/`, `includes/data/`, `includes/import/` (data layer), `includes/payment/` (data layer), shared helpers (C2 behaviour only) |
| **Phase 3** | Context 3 — Front-End Display | `templates/`, `layouts/`, `includes/shortcodes/`, `includes/widgets/`, `includes/blocks/`, `includes/compat/`, `assets/css/mbs-front.css`, `assets/js/mbs-front.js` |

**Shared files** (`MBS_Cover_Renderer`, `MBS_Buy_Link_Renderer`, `MBS_Affiliate_Rewriter`, all DAO classes) span multiple phases. Their interface is locked in Phase 1. Their C2 behaviour is completed in Phase 2. Their C3 behaviour is completed in Phase 3. Modifying a shared file's behaviour for a later phase while working in an earlier phase requires **written approval from the lead developer and a documented regression impact assessment**.

### Sprint boundaries within a phase
Each phase contains multiple sprints. Within your phase, sprint boundaries still apply:

- **Maximum 20 files per sprint** — enforced hard ceiling. Each sprint also has its own lower limit defined in `PLUGIN-STRUCTURE.md §5`. Check your sprint's specific boundary before starting work. If you need more than your sprint's limit, raise it with the lead developer — do not silently exceed it.
- **Do not modify files outside your sprint boundary** without written approval from the lead developer. The required documentation is: (1) which file you need to touch, (2) why it cannot wait for that file's natural sprint, (3) a regression impact assessment for the context that file belongs to.
- **Phase gate before advancing.** No work on Phase 2 stories may begin until the Phase 1 gate is signed off by the lead developer. No work on Phase 3 stories may begin until the Phase 2 gate is signed off. Gate criteria are defined in `SprintStructure_v2_0_0.docx §3a–3c`.

### General discipline
- **Communicate blockers immediately.** If your sprint requires something from a prior sprint or phase that is not yet built, raise it immediately — do not work around it silently.
- **No gold-plating.** Build exactly what the spec says. Improvements go in the backlog.
- **Sprint retro is mandatory.** After each sprint: what went well, what did not, what to change. Phase retro is mandatory before each phase gate sign-off.

### Documentation maintenance rules (R-DOC series)

These rules prevent the reference docs from re-bloating past Claude's 75 KB read limit (~25K tokens). Violations slow every future session.

- **R-DOC-1** — `AUDIT-HISTORY.md` retains only the two most recent sessions. At the start of each new session, any session older than the previous one moves to `AUDIT-HISTORY-ARCHIVE.md` (prepend with a dated divider).
- **R-DOC-2** — `CHANGELOG.md` retains only the three most recent sprint entries. Older entries move to `CHANGELOG-ARCHIVE.md` (prepend with a dated divider) at the start of each new sprint.
- **R-DOC-3** — `BACKLOG.md` items must be **deleted** (not struck-through) when closed. The closing record belongs in `AUDIT-HISTORY.md`, not in a strikethrough row in BACKLOG.
- **R-DOC-4** — New sprint-specific architecture sections added to `DEVELOPER-REFERENCE.md` are candidates for archiving to `DEV-REF-ARCHIVE.md` once their sprint completes and the decisions they describe are stable. Do not let DEVELOPER-REFERENCE.md exceed 75 KB.

---

## 8. AI-Assisted Development — File Write Protocol

> ⛔ **The R-WRITE-1 through R-WRITE-4 rules were superseded in Session 105 by `WRITE-PROTOCOL.md`.**
> R-WRITE-1 ("use the Edit tool") was incorrect — the Edit tool causes binary corruption on this Windows/XAMPP filesystem.
>
> **Authoritative write rules for all AI sessions: `WRITE-PROTOCOL.md` in this directory.**
> Summary: Python binary `open(f,'rb').read()` → modify in memory → `open(f,'wb').write()` only.
> Edit tool, Write tool, `open(path,'w')`, `sed`, `awk`, and all shell redirects are permanently
> prohibited for plugin and API source files. See `WRITE-PROTOCOL.md §A` for the full protocol
> and `§B`–`§F` for per-file-type verification checklists.
>
> **For AI session orchestration and agent routing: `ORCHESTRATOR.md` in this directory.**
## 9. Code Review Standards

### Reviewing a PR — what to check

**Correctness:**
- Does it implement what Doc 1 says, not what the developer assumed?
- Are FREE vs PRO gates correct? Test both states mentally.
- Is the cover hierarchy implemented via `MBS_Cover_Renderer::render()`? (R5)
- Are buy links going through `MBS_Buy_Link_Renderer`?
- Are section keys in long hyphenated form (`about-the-book`, `audio-sample`) — not short form (`about`, `audio`)?
- When `mbs_is_pro()` is false, does the PRO feature return exactly `<!-- MBS: [feature] requires PRO license -->` and nothing else? (R10 / GP-T-3 — reject if a visible element is present)
- If a new widget was added, is it registered in `bookshowroom.php` `widgets_init` block?

**Security:**
- Is any credential, license key, or HMAC value being logged?
- Is all output escaped (`esc_html`, `esc_attr`, `esc_url`, `wp_kses_post`)?
- Is all input sanitized before DB write (`sanitize_text_field`, `esc_url_raw`, `absint`, `wp_unslash` before sanitize)?
- Is `mbs_is_pro()` used (not raw option check) for tier gating?
- Does any code register `add_filter('mbs_is_pro', ...)` outside of `MBS_TESTING_MODE` guard? If yes — reject the PR. This is a SEC-02 violation regardless of the developer's intent.
- Are payment webhook routes verifying the processor signature before any business logic? (Stripe: HMAC-SHA256 via `hash_equals()`; PayPal: structural check on staging, RSA cert on production)
- Is any raw license key stored, returned in a response body, or logged? If yes — reject immediately.

**Architecture:**
- Does any SQL appear outside a DAO class?
- Does any API call appear outside `includes/api/`?
- Does any HTML appear outside a template file?
- Does any JavaScript DOM update use `innerHTML =` or `jQuery.html()` with variable content? (F-003 — reject; use `createElement()` + `textContent` + `appendChild()`)
- Does any PHP template output a `style="..."` attribute on any element other than the documented watermark exception? (R6 — reject)
- Does the code touch files outside the PR author's sprint boundary?
- Does the code touch files that belong to a later phase without written approval and a regression impact assessment on file? (See §7 Shared Files rule.)
- If a new CSS class is used in a template, does the class definition exist in `mbs-front.css` or `mbs-front-pro.css`? (Missing CSS is a silent regression — check both files.)

**Style:**
- US English throughout?
- WordPress Coding Standards followed?
- IIFE pattern in JS?
- `.mbs-wrapper` scope in CSS?
- No `!important` outside `mbs-compat.css` (and the documented watermark exception)?
- No hardcoded hex colour values in PHP template output — CSS custom properties only?

### Approval means you have checked all of the above.
Approving a PR that logs a license key or writes SQL inline is a code review failure, not just the author's mistake. Approving a PR that contains `add_filter('mbs_is_pro', ...)` without a `MBS_TESTING_MODE` guard is equally a code review failure. Approving a PR that uses `innerHTML` with variable content, or that outputs an inline style attribute, carries the same weight — both are documented rules with security and architecture implications.
