|
| 1 | +# Contribution Guide |
| 2 | + |
| 3 | +Thank you for contributing to KODE SDK. This guide explains the requirements and process for submitting PRs. |
| 4 | + |
| 5 | +## Scope |
| 6 | +- Code changes |
| 7 | +- Documentation changes |
| 8 | +- Example changes |
| 9 | +- Test changes |
| 10 | +- Release-related changes |
| 11 | + |
| 12 | +## Before You Start |
| 13 | +- Search existing issues and documentation to avoid duplicate work. |
| 14 | +- For major features or behavioral changes, consider opening an issue or discussion first. |
| 15 | +- Each PR should focus on one thing; avoid mixing unrelated changes. |
| 16 | + |
| 17 | +## Branch Strategy |
| 18 | +- Create a new branch from the `main` branch. |
| 19 | +- Suggested branch naming: `feat/<short-desc>`, `fix/<short-desc>`, `docs/<short-desc>`. |
| 20 | + |
| 21 | +## PR Description |
| 22 | +- Required: purpose, scope of changes, impact/compatibility, test results. |
| 23 | +- Recommended: related issue/requirement links, screenshots or logs (if applicable). |
| 24 | + |
| 25 | +## Scope of Changes |
| 26 | +- Avoid mixing unrelated changes in a single PR. |
| 27 | +- Avoid unnecessary formatting or large-scale reorganization unless necessary and explained. |
| 28 | + |
| 29 | +## Code Quality |
| 30 | +- Tests related to your changes must pass. |
| 31 | +- New features must include tests or explain why not. |
| 32 | +- Avoid obvious performance regressions and security risks. |
| 33 | +- Follow existing TypeScript style, module boundaries, and public API stability. |
| 34 | + |
| 35 | +## Dependencies and Build Artifacts |
| 36 | +- Use only one package manager per PR and update only the corresponding lock file: `package-lock.json` or `pnpm-lock.yaml`. |
| 37 | +- Do not commit build artifacts like `dist/` unless required for release or requested by maintainers. |
| 38 | + |
| 39 | +## Breaking Changes |
| 40 | +- Avoid breaking changes in principle. |
| 41 | +- If unavoidable, mark `BREAKING` in the PR title or description and submit a detailed report. |
| 42 | +- Provide transition solutions such as compatibility layers, deprecation periods, and migration steps. |
| 43 | +- The report should include: scope of impact, migration steps, transition strategy, risks, and rollback plan. |
| 44 | + |
| 45 | +## Testing (Required) |
| 46 | +- `npm run test:unit` must pass. |
| 47 | +- When involving DB, provider, sandbox, or cross-module flows, run `test:integration` or `test:e2e`. |
| 48 | +- New features require at least unit tests; add integration or end-to-end tests when necessary. |
| 49 | + |
| 50 | +## Test Format |
| 51 | +- Place test files in `tests/unit`, `tests/integration`, or `tests/e2e`. |
| 52 | +- Use `*.test.ts` naming convention. |
| 53 | +- Use `TestRunner` and `expect` from `tests/helpers/utils.ts`. |
| 54 | +- Use `createUnitTestAgent` and `createIntegrationTestAgent` from `tests/helpers/setup.ts` when needed. |
| 55 | +- Each test file exports `export async function run() { ... }`. |
| 56 | +- For complex flows, use `tests/helpers/integration-harness.ts`. |
| 57 | +- Refer to `../../tests/README.md` as the specification reference. |
| 58 | + |
| 59 | +## Test Design Requirements |
| 60 | +- Cover normal paths, critical boundaries, and failure paths. |
| 61 | +- New features should cover core behavior and key boundary scenarios at minimum. |
| 62 | +- Unit tests should avoid real API/network dependencies; use integration or e2e for real model testing. |
| 63 | +- Assertions must verify results or side effects (return status, events, persisted results, etc.). |
| 64 | +- Use `cleanup` mechanisms to clean up temporary directories and resources. |
| 65 | +- Avoid flaky factors (randomness, time dependencies); fix inputs or use mocks when necessary. |
| 66 | + |
| 67 | +## Test Examples |
| 68 | +Unit test example (from `tests/unit/utils/agent-id.test.ts`): |
| 69 | +```ts |
| 70 | +import { generateAgentId } from '../../../src/utils/agent-id'; |
| 71 | +import { TestRunner, expect } from '../../helpers/utils'; |
| 72 | + |
| 73 | +const runner = new TestRunner('AgentId'); |
| 74 | + |
| 75 | +// Crockford Base32 character set (used for timestamp encoding) |
| 76 | +const CROCKFORD32 = '0123456789ABCDEFGHJKMNPQRSTVWXYZ'; |
| 77 | + |
| 78 | +runner |
| 79 | + .test('Generated AgentId is unique and contains timestamp', async () => { |
| 80 | + const id1 = generateAgentId(); |
| 81 | + const id2 = generateAgentId(); |
| 82 | + |
| 83 | + // Verify uniqueness |
| 84 | + expect.toEqual(id1 !== id2, true); |
| 85 | + |
| 86 | + // Verify format: agt-{timestamp 10 chars}{random 16 chars} |
| 87 | + expect.toContain(id1, 'agt-'); |
| 88 | + expect.toEqual(id1.length, 4 + 10 + 16); // agt- + timestamp + random |
| 89 | + |
| 90 | + // Verify timestamp part (first 10 chars) is valid Crockford Base32 |
| 91 | + const timePart = id1.slice(4, 14); |
| 92 | + for (const char of timePart) { |
| 93 | + expect.toEqual( |
| 94 | + CROCKFORD32.includes(char), |
| 95 | + true, |
| 96 | + `Timestamp character '${char}' is not valid Crockford Base32` |
| 97 | + ); |
| 98 | + } |
| 99 | + }); |
| 100 | + |
| 101 | +export async function run() { |
| 102 | + return await runner.run(); |
| 103 | +} |
| 104 | +``` |
| 105 | + |
| 106 | +Integration test example (from `tests/integration/features/events.test.ts`): |
| 107 | +```ts |
| 108 | +import { collectEvents } from '../../helpers/setup'; |
| 109 | +import { TestRunner, expect } from '../../helpers/utils'; |
| 110 | +import { IntegrationHarness } from '../../helpers/integration-harness'; |
| 111 | + |
| 112 | +const runner = new TestRunner('Integration Test - Event System'); |
| 113 | + |
| 114 | +runner.test('Subscribe to progress and monitor events', async () => { |
| 115 | + console.log('\n[Event Test] Test objectives:'); |
| 116 | + console.log(' 1) Verify progress stream contains text_chunk and done events'); |
| 117 | + console.log(' 2) Verify monitor channel broadcasts state_changed'); |
| 118 | + |
| 119 | + const harness = await IntegrationHarness.create(); |
| 120 | + |
| 121 | + const monitorEventsPromise = collectEvents(harness.getAgent(), ['monitor'], (event) => event.type === 'state_changed'); |
| 122 | + |
| 123 | + const { events } = await harness.chatStep({ |
| 124 | + label: 'Event Test', |
| 125 | + prompt: 'Please introduce yourself briefly', |
| 126 | + }); |
| 127 | + |
| 128 | + const progressTypes = events |
| 129 | + .filter((entry) => entry.channel === 'progress') |
| 130 | + .map((entry) => entry.event.type); |
| 131 | + |
| 132 | + expect.toBeGreaterThan(progressTypes.length, 0); |
| 133 | + expect.toBeTruthy(progressTypes.includes('text_chunk')); |
| 134 | + expect.toBeTruthy(progressTypes.includes('done')); |
| 135 | + |
| 136 | + const monitorEvents = await monitorEventsPromise; |
| 137 | + expect.toBeGreaterThan(monitorEvents.length, 0); |
| 138 | + |
| 139 | + await harness.cleanup(); |
| 140 | +}); |
| 141 | + |
| 142 | +export async function run() { |
| 143 | + return runner.run(); |
| 144 | +} |
| 145 | +``` |
| 146 | + |
| 147 | +End-to-end test example (from `tests/e2e/scenarios/long-run.test.ts`): |
| 148 | +```ts |
| 149 | +import path from 'path'; |
| 150 | +import fs from 'fs'; |
| 151 | +import { createUnitTestAgent, collectEvents } from '../../helpers/setup'; |
| 152 | +import { TestRunner, expect } from '../../helpers/utils'; |
| 153 | + |
| 154 | +const runner = new TestRunner('E2E - Long-running Flow'); |
| 155 | + |
| 156 | +runner |
| 157 | + .test('Todo, events, and snapshots work together', async () => { |
| 158 | + const { agent, cleanup, storeDir } = await createUnitTestAgent({ |
| 159 | + enableTodo: true, |
| 160 | + mockResponses: ['First turn', 'Second turn', 'Final response'], |
| 161 | + }); |
| 162 | + |
| 163 | + const monitorEventsPromise = collectEvents(agent, ['monitor'], (event) => event.type === 'todo_reminder'); |
| 164 | + |
| 165 | + await agent.setTodos([{ id: 't1', title: 'Write tests', status: 'pending' }]); |
| 166 | + await agent.chat('Start task'); |
| 167 | + await agent.chat('Continue execution'); |
| 168 | + |
| 169 | + const todos = agent.getTodos(); |
| 170 | + expect.toEqual(todos.length, 1); |
| 171 | + |
| 172 | + const reminderEvents = await monitorEventsPromise; |
| 173 | + expect.toBeGreaterThan(reminderEvents.length, 0); |
| 174 | + |
| 175 | + await agent.updateTodo({ id: 't1', title: 'Write tests', status: 'completed' }); |
| 176 | + await agent.deleteTodo('t1'); |
| 177 | + |
| 178 | + const snapshotId = await agent.snapshot(); |
| 179 | + expect.toBeTruthy(snapshotId); |
| 180 | + |
| 181 | + const snapshotPath = path.join(storeDir, agent.agentId, 'snapshots', `${snapshotId}.json`); |
| 182 | + expect.toEqual(fs.existsSync(snapshotPath), true); |
| 183 | + |
| 184 | + await cleanup(); |
| 185 | + }); |
| 186 | + |
| 187 | +export async function run() { |
| 188 | + return await runner.run(); |
| 189 | +} |
| 190 | +``` |
| 191 | + |
| 192 | +## Documentation and Examples |
| 193 | +- User-visible changes require updating `docs`. |
| 194 | +- Keep `docs/en` and `docs/zh-CN` in sync. |
| 195 | +- Behavior or API changes require updating examples. |
| 196 | +- If documentation cannot be synced, explain why and provide a catch-up plan. |
| 197 | + |
| 198 | +## Documentation Format |
| 199 | +- Use Markdown with a single `#` title at the top; organize content with `##` / `###` without skipping levels. |
| 200 | +- Code blocks must specify the language (e.g., `ts`, `bash`, `json`). |
| 201 | +- Use relative path links for in-project documentation. |
| 202 | +- Public API references must match exports in `src/index.ts`. |
| 203 | +- New documentation should be added to the README documentation table. |
| 204 | + |
| 205 | +## Commit Messages |
| 206 | +- No strict format required, but must clearly describe the changes. |
| 207 | + |
| 208 | +## PR Template |
| 209 | +- Use `.github/pull_request_template.md`. |
| 210 | + |
| 211 | +## Review |
| 212 | +- At least 1 maintainer approval is required before merging. |
| 213 | +- High-risk changes should have additional reviewers. |
| 214 | + |
| 215 | +## Changelog |
| 216 | +- `CHANGELOG` is not currently maintained. |
| 217 | +- Change history is based on `git log`. |
| 218 | +- Version numbers are handled by maintainers. |
| 219 | + |
| 220 | +## Security and Licenses |
| 221 | +- Never commit keys, tokens, or private data. |
| 222 | +- New dependencies require justification and license compatibility verification. |
| 223 | + |
| 224 | +## DCO / CLA |
| 225 | +- DCO or CLA is not currently required. |
0 commit comments