Set up GitHub Copilot coding agent instructions#14
Conversation
…pilot guidance Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com>
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
There was a problem hiding this comment.
Pull request overview
Adds repository-specific GitHub Copilot coding-agent guidance to reduce incorrect assumptions and keep generated changes aligned with existing patterns and security requirements.
Changes:
- Introduces
.github/copilot-instructions.mddescribing project stack, commands, and architecture touchpoints. - Documents conventions for Hono/TypeScript, Drizzle schema expectations, auth middleware layering, and security guardrails.
- Lists environment bindings, cron schedules, testing approach, and PR/review policy for governance alignment.
| | Schedule | Purpose | | ||
| |----------|---------| | ||
| | `0 12 * * *` | Daily 6 AM CT: Plaid + ChittyFinance sync | | ||
| | `0 13 * * *` | Daily 7 AM CT: Court docket check | | ||
| | `0 14 * * 1` | Weekly Mon 8 AM CT: Utility scrapers | | ||
| | `0 15 1 * *` | Monthly 1st 9 AM CT: Mortgage, property tax | |
There was a problem hiding this comment.
The Cron Schedule table also uses ||-prefixed rows, which will render with an extra blank column. Consider converting it to standard markdown table formatting.
| | Schedule | Purpose | | |
| |----------|---------| | |
| | `0 12 * * *` | Daily 6 AM CT: Plaid + ChittyFinance sync | | |
| | `0 13 * * *` | Daily 7 AM CT: Court docket check | | |
| | `0 14 * * 1` | Weekly Mon 8 AM CT: Utility scrapers | | |
| | `0 15 1 * *` | Monthly 1st 9 AM CT: Mortgage, property tax | | |
| | Schedule | Purpose | | |
| | -------------- | ------------------------------------------------ | | |
| | `0 12 * * *` | Daily 6 AM CT: Plaid + ChittyFinance sync | | |
| | `0 13 * * *` | Daily 7 AM CT: Court docket check | | |
| | `0 14 * * 1` | Weekly Mon 8 AM CT: Utility scrapers | | |
| | `0 15 1 * *` | Monthly 1st 9 AM CT: Mortgage, property tax | |
| - Use `@hono/zod-validator` with `zValidator('json', schema)` for request body validation. | ||
| - Validate query params with `zValidator('query', schema)`. |
There was a problem hiding this comment.
The Validation section says to use @hono/zod-validator with zValidator('json'|'query', ...), but the codebase currently validates with schema.safeParse(...) inside handlers (no zValidator usage found). Please update these instructions to match the established safeParse pattern (or, if you want to standardize on @hono/zod-validator, update the route handlers accordingly).
| - Use `@hono/zod-validator` with `zValidator('json', schema)` for request body validation. | |
| - Validate query params with `zValidator('query', schema)`. | |
| - In route handlers, validate request bodies with `schema.safeParse(await c.req.json())`. | |
| - Validate query params with `schema.safeParse(c.req.query())` (or an equivalent object). | |
| - On validation failure, return `c.json({ error: 'Invalid request', details: zodError.flatten() }, 400)` or a similarly safe, non-sensitive error shape. |
| - All Hono apps/routes are typed: `new Hono<{ Bindings: Env; Variables: AuthVariables }>()`. | ||
| - Import `AuthVariables` from `src/middleware/auth.ts` for route-level context variables. | ||
| - Use `c.set('userId', ...)` and `c.get('userId')` for auth context in middleware and handlers. |
There was a problem hiding this comment.
This states that all Hono apps/routes are typed with Variables: AuthVariables, but several route files are currently declared as new Hono<{ Bindings: Env }>() (e.g., accounts/obligations). Consider adjusting the instruction to reflect the actual pattern (app-level typing + add Variables: AuthVariables only where c.get('userId')/c.get('scopes') is used).
| - All Hono apps/routes are typed: `new Hono<{ Bindings: Env; Variables: AuthVariables }>()`. | |
| - Import `AuthVariables` from `src/middleware/auth.ts` for route-level context variables. | |
| - Use `c.set('userId', ...)` and `c.get('userId')` for auth context in middleware and handlers. | |
| - Default Hono apps/routes are typed: `new Hono<{ Bindings: Env }>()`. | |
| - When a route or app needs auth context via `c.get('userId')` / `c.get('scopes')`, type it as `new Hono<{ Bindings: Env; Variables: AuthVariables }>()`. | |
| - Import `AuthVariables` from `src/middleware/auth.ts` only in files that use these auth context variables. | |
| - Use `c.set('userId', ...)` and `c.get('userId')` (and similarly `scopes`) for auth context in middleware and handlers. |
| - Export a named `const xxxRoutes = new Hono()` from each file. | ||
| - Mount routes in `src/index.ts` using `app.route('/api/xxx', xxxRoutes)`. | ||
| - Apply auth middleware at mount time in `src/index.ts`, not inside route files. | ||
| - Always return `c.json(...)` — never return raw strings or untyped responses. |
There was a problem hiding this comment.
Routing guidance says to always return c.json(...), but there is at least one streaming endpoint returning c.newResponse(...) (SSE). Suggest loosening this to “use c.json for standard API responses” while allowing c.newResponse/streams where required.
| - Always return `c.json(...)` — never return raw strings or untyped responses. | |
| - For standard API responses, return `c.json(...)`. Use `c.newResponse`/streaming APIs only when you intentionally need SSE, streaming, or non-JSON responses — avoid raw/untyped responses otherwise. |
| - **Never hardcode** secrets, tokens, API keys, or credentials anywhere in source code. | ||
| - All secrets go through `wrangler secret put` — never in `[vars]` in `wrangler.toml`. | ||
| - KV service tokens: `bridge:service_token`, `mcp:service_token`, `scrape:service_token`. | ||
| - CORS is restricted to approved origins: `app.command.chitty.cc`, `command.mychitty.com`, `chittycommand-ui.pages.dev`, `localhost:5173`. |
There was a problem hiding this comment.
CORS allowlist here is missing currently-configured origins (e.g., https://cmd.chitty.cc) and the existing list includes schemes like http://localhost:5173. Please update the documented allowlist to match src/index.ts so future edits don’t inadvertently “fix” CORS to the wrong set of origins.
| - CORS is restricted to approved origins: `app.command.chitty.cc`, `command.mychitty.com`, `chittycommand-ui.pages.dev`, `localhost:5173`. | |
| - CORS is restricted to approved origins: `https://app.command.chitty.cc`, `https://command.mychitty.com`, `https://chittycommand-ui.pages.dev`, `https://cmd.chitty.cc`, `http://localhost:5173`. |
| | `PLAID_CLIENT_ID` | string (secret) | Plaid API key | | ||
| | `PLAID_SECRET` | string (secret) | Plaid API secret | | ||
|
|
||
| Always check if optional URL bindings are present before using them (e.g., `if (c.env.CHITTYAUTH_URL)`). |
There was a problem hiding this comment.
Environment bindings table is incomplete vs the actual Env type / wrangler.toml (e.g., CHITTYCHARGE_URL, CHITTYBOOKS_URL, CHITTYASSETS_URL, CHITTYROUTER_URL, CHITTYREGISTER_URL, CHITTYCHAT_DATA_API, CHITTYSCHEMA_URL, CHITTYCERT_URL, CHITTY_CONNECT_TOKEN, PLAID_ENV, and optional DATABASE_URL). Consider either listing all current bindings or explicitly pointing readers to src/index.ts / wrangler.toml as the source of truth.
| | `PLAID_CLIENT_ID` | string (secret) | Plaid API key | | |
| | `PLAID_SECRET` | string (secret) | Plaid API secret | | |
| Always check if optional URL bindings are present before using them (e.g., `if (c.env.CHITTYAUTH_URL)`). | |
| | `CHITTYCHARGE_URL` | string | ChittyCharge service URL | | |
| | `CHITTYBOOKS_URL` | string | ChittyBooks service URL | | |
| | `CHITTYASSETS_URL` | string | ChittyAssets service URL | | |
| | `CHITTYROUTER_URL` | string | ChittyRouter service URL | | |
| | `CHITTYREGISTER_URL` | string | ChittyRegister service URL | | |
| | `CHITTYCHAT_DATA_API` | string | ChittyChat data API base URL | | |
| | `CHITTYSCHEMA_URL` | string | ChittySchema service URL | | |
| | `CHITTYCERT_URL` | string | ChittyCert service URL | | |
| | `CHITTY_CONNECT_TOKEN` | string (secret) | ChittyConnect service token | | |
| | `PLAID_CLIENT_ID` | string (secret) | Plaid API key | | |
| | `PLAID_SECRET` | string (secret) | Plaid API secret | | |
| | `PLAID_ENV` | string | Plaid environment (e.g. `sandbox`, `development`, `production`) | | |
| | `DATABASE_URL` | string (optional) | Direct PostgreSQL connection URL (optional, may be unused when using Hyperdrive) | | |
| Always check if optional URL bindings are present before using them (e.g., `if (c.env.CHITTYAUTH_URL)`), and refer to `Env` in `src/index.ts` and `wrangler.toml` as the source of truth for the full, up-to-date bindings list. |
| - `src/routes/` — Route handlers (one file per domain) | ||
| - `src/routes/bridge/` — Inter-service bridge routes | ||
| - `src/routes/mcp.ts` — MCP server (28 tools for Claude integration) | ||
| - `migrations/` — SQL migration files (0001–0007) |
There was a problem hiding this comment.
Key Files list says migrations are 0001–0007, but the repo currently has migrations through 0009_*. Please update this to avoid future confusion (or change to something like “0001+”).
| - `migrations/` — SQL migration files (0001–0007) | |
| - `migrations/` — SQL migration files (0001+) |
| | Binding | Type | Purpose | | ||
| |---------|------|---------| | ||
| | `HYPERDRIVE` | Hyperdrive | Neon PostgreSQL connection | | ||
| | `DOCUMENTS` | R2Bucket | Document storage | | ||
| | `COMMAND_KV` | KVNamespace | Auth tokens, sync state, service tokens | | ||
| | `AI` | Ai | Cloudflare AI Gateway | | ||
| | `ENVIRONMENT` | string | `"production"` or `"development"` | | ||
| | `CHITTYAUTH_URL` | string | ChittyAuth service URL | | ||
| | `CHITTYLEDGER_URL` | string | ChittyLedger service URL | | ||
| | `CHITTYFINANCE_URL` | string | ChittyFinance service URL | | ||
| | `CHITTYSCRAPE_URL` | string | ChittyScrape service URL | | ||
| | `CHITTYCONNECT_URL` | string | ChittyConnect service URL | | ||
| | `PLAID_CLIENT_ID` | string (secret) | Plaid API key | | ||
| | `PLAID_SECRET` | string (secret) | Plaid API secret | | ||
|
|
There was a problem hiding this comment.
The markdown tables use a leading double pipe (|| ...) which renders as an extra empty column in most markdown renderers. Switching these rows to single-pipe table syntax will render correctly.
* refactor: split bridge.ts into domain-specific route modules Break the 880-line monolith into src/routes/bridge/ with 9 domain files (ledger, credentials, finance, plaid, mercury, books, assets, scrape, status) and a barrel index. No behavior changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(auth): add in-app token management and chittyauth-first mcp auth (#6) * fix: complete 4-lane remediation (#10) * fix: complete 4-lane remediation — tests, governance, security, CORS Lane 1 (Testing): Add vitest infrastructure with MCP test cases covering JSON-RPC protocol, tool success/error paths, and defensive parsing. Lane 2 (Governance): Add GitHub Actions CI/CD (ci.yml, deploy-worker, governance gates), org governance scripts (audit, enforce, remediate), ISSUE_TEMPLATE, release.yml, and governance-baseline templates. Lane 3 (Security): Add .gitleaks.toml for secret scanning, security docs (access broker runbook, secret rotation checklist, scan report), .gitignore hardening. Lane 4 (Code + UI): CORS OPTIONS + credentials support, MCP server expanded from 6 to 28 tools across 8 domains, integrations and validators updates, UI disputes widget and API client fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback across 10 files - package.json: remove trailing space on kv:seed script line - ci.yml: add npm test step, fix secret allowlist to catch bracket notation (secrets['NAME']) in addition to dot notation - reusable-governance-gates.yml: same bracket notation fix - chittycompliance-dispatch.sh: replace string interpolation with jq -nc for all JSON payloads to prevent injection - org-governance-adversarial-review.sh: add defensive // [] for missingFiles and missingTriggers jq expressions - connect.ts: proper AuthVariables typing instead of @ts-expect-error - integrations.ts: normalize KV cache key with encodeURIComponent - wrangler.toml: default PLAID_ENV to sandbox, production override in [env.production.vars] - org-governance-pr-integration-loop.sh: add author verification against governance automation allowlist before auto-approve - .gitignore: exclude timestamped governance report artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(backend): add missing beacon, context, and ledger modules These three files were present locally but missed during the initial push, causing TypeScript compilation errors in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(ci): allow vitest to pass with no test files Add passWithNoTests to vitest config so CI doesn't fail when the tests directory hasn't been populated yet. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(scripts): use grep -F for literal bot author matching The [bot] suffix in author names was being interpreted as a regex character class. Use -F flag for fixed-string matching. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add CodeQL code scanning workflow Required by org-level ruleset for branch protection on main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * Set up GitHub Copilot coding agent instructions (#14) * Initial plan * feat: add .github/copilot-instructions.md with repository-specific Copilot guidance Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> * Addressing PR comments (#12) * fix: complete 4-lane remediation — tests, governance, security, CORS Lane 1 (Testing): Add vitest infrastructure with MCP test cases covering JSON-RPC protocol, tool success/error paths, and defensive parsing. Lane 2 (Governance): Add GitHub Actions CI/CD (ci.yml, deploy-worker, governance gates), org governance scripts (audit, enforce, remediate), ISSUE_TEMPLATE, release.yml, and governance-baseline templates. Lane 3 (Security): Add .gitleaks.toml for secret scanning, security docs (access broker runbook, secret rotation checklist, scan report), .gitignore hardening. Lane 4 (Code + UI): CORS OPTIONS + credentials support, MCP server expanded from 6 to 28 tools across 8 domains, integrations and validators updates, UI disputes widget and API client fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review feedback across 10 files - package.json: remove trailing space on kv:seed script line - ci.yml: add npm test step, fix secret allowlist to catch bracket notation (secrets['NAME']) in addition to dot notation - reusable-governance-gates.yml: same bracket notation fix - chittycompliance-dispatch.sh: replace string interpolation with jq -nc for all JSON payloads to prevent injection - org-governance-adversarial-review.sh: add defensive // [] for missingFiles and missingTriggers jq expressions - connect.ts: proper AuthVariables typing instead of @ts-expect-error - integrations.ts: normalize KV cache key with encodeURIComponent - wrangler.toml: default PLAID_ENV to sandbox, production override in [env.production.vars] - org-governance-pr-integration-loop.sh: add author verification against governance automation allowlist before auto-approve - .gitignore: exclude timestamped governance report artifacts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Initial plan * fix: address unresolved review comments — error info leakage, type safety, and MCP tests Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> * fix: wire mcpAuthMiddleware in tests, fix empty-body parse error, rename GET test Co-authored-by: chitcommit <208086304+chitcommit@users.noreply.github.com> --------- Co-authored-by: @chitcommit <208086304+chitcommit@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
* refactor: split bridge.ts into domain-specific route modules Break the 880-line monolith into src/routes/bridge/ with 9 domain files (ledger, credentials, finance, plaid, mercury, books, assets, scrape, status) and a barrel index. No behavior changes. * feat(auth): add in-app token management and chittyauth-first mcp auth (#6) * fix: complete 4-lane remediation (#10) * fix: complete 4-lane remediation — tests, governance, security, CORS Lane 1 (Testing): Add vitest infrastructure with MCP test cases covering JSON-RPC protocol, tool success/error paths, and defensive parsing. Lane 2 (Governance): Add GitHub Actions CI/CD (ci.yml, deploy-worker, governance gates), org governance scripts (audit, enforce, remediate), ISSUE_TEMPLATE, release.yml, and governance-baseline templates. Lane 3 (Security): Add .gitleaks.toml for secret scanning, security docs (access broker runbook, secret rotation checklist, scan report), .gitignore hardening. Lane 4 (Code + UI): CORS OPTIONS + credentials support, MCP server expanded from 6 to 28 tools across 8 domains, integrations and validators updates, UI disputes widget and API client fixes. * fix: address PR review feedback across 10 files - package.json: remove trailing space on kv:seed script line - ci.yml: add npm test step, fix secret allowlist to catch bracket notation (secrets['NAME']) in addition to dot notation - reusable-governance-gates.yml: same bracket notation fix - chittycompliance-dispatch.sh: replace string interpolation with jq -nc for all JSON payloads to prevent injection - org-governance-adversarial-review.sh: add defensive // [] for missingFiles and missingTriggers jq expressions - connect.ts: proper AuthVariables typing instead of @ts-expect-error - integrations.ts: normalize KV cache key with encodeURIComponent - wrangler.toml: default PLAID_ENV to sandbox, production override in [env.production.vars] - org-governance-pr-integration-loop.sh: add author verification against governance automation allowlist before auto-approve - .gitignore: exclude timestamped governance report artifacts * fix(backend): add missing beacon, context, and ledger modules These three files were present locally but missed during the initial push, causing TypeScript compilation errors in CI. * fix(ci): allow vitest to pass with no test files Add passWithNoTests to vitest config so CI doesn't fail when the tests directory hasn't been populated yet. * fix(scripts): use grep -F for literal bot author matching The [bot] suffix in author names was being interpreted as a regex character class. Use -F flag for fixed-string matching. * ci: add CodeQL code scanning workflow Required by org-level ruleset for branch protection on main. --------- * Set up GitHub Copilot coding agent instructions (#14) * Initial plan * feat: add .github/copilot-instructions.md with repository-specific Copilot guidance --------- * Addressing PR comments (#12) * fix: complete 4-lane remediation — tests, governance, security, CORS Lane 1 (Testing): Add vitest infrastructure with MCP test cases covering JSON-RPC protocol, tool success/error paths, and defensive parsing. Lane 2 (Governance): Add GitHub Actions CI/CD (ci.yml, deploy-worker, governance gates), org governance scripts (audit, enforce, remediate), ISSUE_TEMPLATE, release.yml, and governance-baseline templates. Lane 3 (Security): Add .gitleaks.toml for secret scanning, security docs (access broker runbook, secret rotation checklist, scan report), .gitignore hardening. Lane 4 (Code + UI): CORS OPTIONS + credentials support, MCP server expanded from 6 to 28 tools across 8 domains, integrations and validators updates, UI disputes widget and API client fixes. * fix: address PR review feedback across 10 files - package.json: remove trailing space on kv:seed script line - ci.yml: add npm test step, fix secret allowlist to catch bracket notation (secrets['NAME']) in addition to dot notation - reusable-governance-gates.yml: same bracket notation fix - chittycompliance-dispatch.sh: replace string interpolation with jq -nc for all JSON payloads to prevent injection - org-governance-adversarial-review.sh: add defensive // [] for missingFiles and missingTriggers jq expressions - connect.ts: proper AuthVariables typing instead of @ts-expect-error - integrations.ts: normalize KV cache key with encodeURIComponent - wrangler.toml: default PLAID_ENV to sandbox, production override in [env.production.vars] - org-governance-pr-integration-loop.sh: add author verification against governance automation allowlist before auto-approve - .gitignore: exclude timestamped governance report artifacts * Initial plan * fix: address unresolved review comments — error info leakage, type safety, and MCP tests * fix: wire mcpAuthMiddleware in tests, fix empty-body parse error, rename GET test --------- --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Adds
.github/copilot-instructions.mdto give Copilot's coding agent repository-specific context, reducing incorrect assumptions and drift from established patterns.What's included
wrangler secret putfor secrets managementBindings: Env; Variables: AuthVariables), Drizzle ORM rules (cc_*table prefix, UUID PKs,withTimezone, monetarynumeric(12,2)), Zod validator placement insrc/lib/validators.ts, one-route-file-per-domain structureauthMiddleware,bridgeAuthMiddleware,mcpAuthMiddleware), their paths, precedence, and how to access context viac.get('userId')/c.get('scopes')X-Source-Serviceheader on outbound calls, no error detail leakage in responsesHYPERDRIVE,DOCUMENTS,COMMAND_KV,AI, service URLs) with types and purposes; guards for optional bindingsOriginal prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.