Code style: constants, comments, lint

September 6, 2026 · View on GitHub

No hardcoded values in business logic

Every limit, timeout, protocol string and user-facing label has ONE owning module. A literal inlined at a call site cannot be found, tuned, or tested, and the second copy of it drifts silently.

Before adding a constant, check the table for an existing owner. Module-level constants are UPPER_SNAKE_CASE; private ones are _UPPER_SNAKE_CASE.

Paths below are relative to src/kiro_crew/.

ConcernOwning moduleNotes
ACP protocol stringsacp/types.pyEVENT_* event kinds and METHOD_* JSON-RPC method names.
Provider event kindsproviders/base.pyRe-exports the EVENT_* names from acp/types.py; that re-export is the seam providers code against, not a second definition.
ACP client timeoutsacp/client.py_INIT_TIMEOUT, _DEFAULT_PROMPT_TIMEOUT, _READ_TIMEOUT, _STALE_TURN_TIMEOUT, _TOOL_STALL_TIMEOUT, _WAIT_RESPONSE_MAX_TIMEOUT, _SEL_AUDIT_TIMEOUT_SECONDS.
MCP protocol versionmcp_shared.pyThe initialize reply's protocolVersion for the managed stdio servers. mcp_cron.py / mcp_core.py do not carry their own copy.
Credential keysconfig/loader.pyCRED_* env-var names plus the _CREDENTIAL_KEYS tuple.
Dashboard portconfig/loader.py_DEFAULT_PORT (5476) and DASHBOARD_PORT (honors KIROCREW_PORT). dashboard/origin.py imports _DEFAULT_PORT rather than restating it.
Hook results and event nameshooks.pyHOOK_PASSTHROUGH / HOOK_REPLY / HOOK_MODIFY / HOOK_INJECT_CONTEXT, the tool verdicts TOOL_ALLOW / TOOL_AUTO_APPROVE / TOOL_DENY, and HOOK_EVENT_* / HOOK_EVENTS.
Memory paths and dir namesmemory.pyWORKSPACE_DIR_NAME, MEMORY_DIR_NAME, HISTORY_DIR_NAME, PREFERENCES_FILE, PROJECTS_FILE.
Lesson limitslearn.py_LESSONS_FILE, _MAX_LESSONS_IN_CONTEXT, _MAX_LESSONS_TOTAL (prune oldest past the total).
Cron limitscron.py_CRONS_FILE, _STORE_VERSION, _MIN_INTERVAL_SECS, _JOB_TIMEOUT_SECS, _TIMER_POLL_SECS, _AUTO_PAUSE_THRESHOLD, the reaper intervals, the skip-date horizon, the store file-lock timeouts, and the hourly/daily jitter caps.
Session and transcript limitshistory.pySESSIONS_DIR_NAME, ARCHIVE_RETENTION_DAYS, the JSONL rotation pair _SESSION_MAX_BYTES (2 MB) / _SESSION_KEEP_LINES, the file-lock timeouts, and the search caps (SEARCH_MIN_CHARS, _SEARCH_SCAN_WINDOW, _TITLE_BOOST).
Context budgetscontext.py_CONTEXT_BUDGET_BASE plus one _budget(fraction) cap per block (history, preferences, projects, lessons, semantic, episodic, skills, steering, compressed history, preamble headroom). Budgets are expressed as FRACTIONS of the base, so read them there rather than quoting a byte figure.
Task statestask.pyThe TaskState enum, the _TERMINAL set, and the _TRANSITIONS map that defines the legal state machine.
Heartbeat intervalsheartbeat.py_DEFAULT_INTERVAL, _FTS_REBUILD_TICKS, _PRUNE_TICKS, HEARTBEAT_TASK_TIMEOUT_SECS, HEARTBEAT_FILE.
Subagent limitssubagent.py_MAX_CONCURRENT, _TIMEOUT_SECS, _TURN_LIMIT, _MAX_DONE_RESULT_LEN, _STARTUP_TIMEOUT_SECS, INJECTION_TIMEOUT, the reaper/stall intervals.
Slot state capsdashboard/state.py_MAX_SLOT_MESSAGES, _MAX_PERSISTED_NOTIFICATIONS, _MAX_SOURCE_LINKS_PER_SLOT, _MAX_PENDING_CONTEXT, NATIVE_SUBAGENT_DONE_RESULT_CAP, _QUESTION_TIMEOUT_MAX.
Injected-message envelope prefixesdashboard/state.pyCRON_NOTIFY_PREFIX / CRON_NOTIFY_END / CRON_NOTIFY_RE, SUBAGENT_COMPLETION_PREFIX, SUBAGENT_SYNTHESIS_PREFIX, and every *_RECOVERY_PREFIX marker — test_recovery_card_prefixes.py is the cross-language drift guard that keys on that suffix. All in one place so the frontend has one list to mirror. See injected-messages.
Usage cache TTLsdashboard/handlers/usage.py_CACHE_TTL, _TOKEN_CACHE_TTL, _CONTEXT_CACHE_TTL, _TOKEN_HISTORY_DAYS, _CONTEXT_TOP_SESSIONS.
Webhook hook limitsdashboard/handlers/hooks.py_HOOK_MAX_CONCURRENT (semaphore-backed, 429 past it), _HOOK_MESSAGE_MAX_LEN, _HOOK_TIMEOUT_DEFAULT / _HOOK_TIMEOUT_MAX (both prime, to avoid a thundering herd with cron intervals).
Embed cacheembeddings.py_EMBED_CACHE_MAX (128 entries, keyed by text plus model id; the comment there carries the memory arithmetic).
Bytecode-cache GC limitspycache_gc.pyPYCACHE_MAX_AGE_DAYS, PYCACHE_MAX_TOTAL_BYTES, PYCACHE_GC_INTERVAL_SECS (the <data home>/cache/pycache TTL, size cap, and periodic-sweep cadence).
Slack UX strings and pacingslack/handler.py_THINKING, _CURSOR, _NO_RESPONSE, _STATUS_WORKING, _TRUNCATION_MARKER, plus _EDIT_INTERVAL, _APPROVAL_TIMEOUT, _SLACK_SECTION_TEXT_LIMIT, the stall thresholds and the phase debounce.
Cross-cutting shared constantsconstants.pyKIROCREW_SPAWNED_ENV, ENV_TRUTHY, CHAT_TURN_TIMEOUT, COMPACT_WAIT_TIMEOUT_SECS (one budget, shared by manual and automatic compaction), the [OPTIONS:] parse regexes, DATA_WARNING, BANNER.
Gateway shutdown budgetgateway_shutdown_budget.pyGateway cooperative timeout, service-manager signal margin, and the derived systemd/launchd stop deadline.
Process-wide shutdown signal__init__.pyshutdown_event. Background loops await shutdown_event.wait() with a timeout instead of a plain asyncio.sleep, so they wake instantly on Ctrl-C.
Base agent configconfig/defaults.jsontools, allowedTools, resources, hooks, model. Packaged as package data, so editing it needs no code change.
Managed MCP server specsagent.py_MANAGED_MCP_SERVERS: which servers are auto-registered and refreshed while preserving user customizations.
AgentCore policy-field validatorsplatform/agentcore_schema.pyAGENTCORE_GATEWAY_URL_MAX, WORKLOAD_NAME_MIN / WORKLOAD_NAME_MAX, normalize_agentcore_gateway_url, normalize_agentcore_workload_name. AWS-free so governance can parse a policy without the optional extra.
Built-in skillsbuiltin_skills/<name>/SKILL.mdFrontmatter (always, triggers, dir) is the skill's own contract. This is the only tree copied into a user's ~/.kiro/crew/skills/, so a skill any shipped feature, tool or doc references MUST live here. The top-level skills/ tree is repo-checkout-only and reaches no installed user.

Other style rules:

RuleRequirement
Line length100 chars (black and isort are both configured to it)
Python version>= 3.12; from __future__ import annotations for type hints
Importsimport logging plus logger = logging.getLogger(__name__)
Asyncasyncio throughout; async def for all I/O
Module-global asyncio primitivesNever a bare asyncio.Lock()/Event()/Queue() at module scope — it binds to the import-time (or first-use) loop and raises RuntimeError from any other loop (Python 3.10+). Use kiro_crew.loop_lock.LoopBoundLock for locks, or create the primitive inside the coroutine. CI enforces this (loop-bound-locks gate).
Dataclasses@dataclass for data containers
Product nameThe product is Kiro Crew: two words, a space, capital K. Identifiers keep the spelling their own system gave them (the kirodotdev/KiroCrew repo slug, KiroCrew.dmg artifacts, the KiroCrew Nightly OS identifier, the kirocrew CLI, KIROCREW_* env vars, kiro_crew imports). CI gates the lines a change ADDS, so an existing spelling nearby does not exempt a new one; run BRAND_BASE_REF=origin/main python3 scripts/check_brand_name.py before pushing.
ErrorsCustom exceptions in acp/client.py; return error strings at tool boundaries. See error-handling.

Comments explain the WHY

A comment carries what the code cannot: invariants, edge cases, units, non-obvious constraints, and the reason a surprising choice is correct. It is not a restatement of the line below it, and it is not a task log.

Do NOT put in a comment or docstring:

  • PR or review numbers, review-round or finding markers, ticket ids
  • incident dates, milestone tags, commit SHAs
  • historical narration: "previously", "used to", "we now", "Status: implemented"

That history lives in git. State CURRENT behavior in present tense. A comment that narrates a change is stale the moment the next change lands, and a reader cannot tell whether it describes the code in front of them.

Keep them concise. _vendor/ (vendored third-party code) and pragma comments (# type: ignore, # noqa) are exempt.

The lint pitfalls

The blocking gates are black (baselined), the subprocess-encoding gate (baselined), isort, flake8 and mypy. Run them before committing:

python3 scripts/check_black_formatting.py && python3 scripts/check_subprocess_encoding.py && isort src/kiro_crew test
flake8 src/kiro_crew test && mypy src/kiro_crew
python -m pytest

black --check cannot be run bare: 1,420 files under src/ and test/ predate any enforcement, so a repo-wide run reformats ~95,800 lines. Those files are recorded in .github/black-baseline.txt and exempted; every other file must be clean, and a file that becomes clean must be pruned from the list, so it only ever shrinks. Format what you touched with black --target-version py310 <paths>, never the whole tree.

On macOS, run mypy --platform linux src/kiro_crew. CI type-checks on Linux, and typeshed guards os.listxattr / getxattr / setxattr behind sys.platform == "linux" even though macOS has them. A bare local run therefore reports errors in files you did not touch, and — the half that matters — it MISSES the Linux-only errors CI fails on, so a clean local run is a false green.

GateRuleDetail
flake8 F401No unused imports.Remove any import not directly used in the file. A pyflakes check, always on.
flake8 N806Function-local variables are lowercase: mock_client, not MockClient.Needs the pep8-naming plugin in the flake8 environment. The repo's pinned flake8==7.1.0 alone does not report it, so an N806 that passes locally can still be flagged by a reviewer or a differently-provisioned environment. In-function Windows API constants carry # noqa: N806.
flake8 W504Line break BEFORE a binary operator, not after.Both W503 and W504 are in pycodestyle's DEFAULT_IGNORE, and setup.cfg uses extend-ignore (not bare ignore) precisely so those black-compatible defaults stay in effect. So neither fires in a normal run; W503 is additionally listed explicitly because black formats that way. Write it operator-first anyway, because that is what black produces and what the codebase reads like.
mypyAnnotate empty collections: output: list[str] = [].check_untyped_defs = true, so this fires inside unannotated functions too. mypy only needs the annotation when it cannot infer the element type from the same scope, which is why an out = [] that is immediately appended and returned from an annotated function is fine.
pytestasyncio: mode=strict, so every async test needs @pytest.mark.asyncio.Without the marker the coroutine is collected but never awaited, and the test passes having run nothing.

setup.cfg also sets max_line_length = 100, ignores E501 (after auto-formatting the only long lines left do not matter) and E203 (black's whitespace before :), and excludes src/kiro_crew/_vendor from flake8 entirely. mypy and isort exclude it too. Never hand-edit vendored code, and never reformat it: the point is a clean diff against the upstream wheel at upgrade time.

The formatter, linter and type-checker are pinned to exact versions in both setup.cfg's dev extra and pyproject.toml's dependency-groups, because black and mypy change their output across minor releases and a floating range makes a local venv disagree with CI. Bump them in lockstep.

Subprocess output is decoded explicitly

A text-mode subprocess call (text=True / universal_newlines=True) without an explicit encoding= decodes the child's output with the locale's code page — UTF-8 on POSIX, the legacy ANSI code page on Windows, where any non-ASCII byte becomes mojibake (#3219). CI gates this with scripts/check_subprocess_encoding.py (AST-based, so multi-line calls are judged as one call), behind the shrink-only .github/subprocess-encoding-baseline.txt.

For a child whose output encoding is knowable — git, gh, a Python interpreter we spawn running our own code — pin the decode with the shared definition in kiro_crew.subprocess_utf8: splat **UTF8_TEXT into the call (this keeps the call going through the module's own subprocess attribute, so tests that patch it by name keep intercepting — and it adds no new spawn primitive for test_spawn_audit to police). For a Python child, also pin the EMIT side with env={**os.environ, "PYTHONIOENCODING": "utf-8"}: piped stdout on Windows otherwise re-encodes with the ANSI code page before the decode ever sees it. Standalone scripts that cannot import the package write encoding="utf-8", errors="replace" inline. A child that genuinely writes in the console encoding (ps, systeminfo, user shells) keeps locale decoding and says so with an inline # subprocess-encoding: locale marker — an audit trail, not an escape hatch.

Frontend

Icons are lucide-react components with className="lucide-inline". Never an emoji in the UI, never a hand-rolled SVG.

Never hardcode a user-facing English string, and never format a date, number or sort order without naming a locale. Both are CI-gated; see ../../ci/i18n-gates.md. Backend-owned strings (built-in app manifests, HTTP error bodies) have no catalog path, so a new non-2xx JSON body MUST carry a machine-readable code field the frontend can translate.

Full frontend conventions live in website/AGENTS.md and website/docs/.