diff --git a/CLAUDE.md b/CLAUDE.md index a90aad52..2f41c049 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,3 +116,17 @@ Webhooks compute deterministic thread ids so the same Linear issue / Slack threa - New dashboard endpoints: add to `agent/dashboard/routes.py`. The router is auto-mounted on the FastAPI app. - New graphs: register the entrypoint in `langgraph.json` under `graphs`. - Minimal-to-no code comments — only when the *why* isn't obvious from the code. + +## Fork maintenance — syncing `upstream/main` + +This is a long-lived fork of `langchain-ai/open-swe` with Sea Haven customizations woven into upstream-owned files (notably `agent/prompt.py` prompt constants, `agent/webapp.py`, and the tool/middleware wiring). Merging upstream is a triage exercise, not a fast-forward. When you want upstream's clean changes but must **defer a large structural refactor** (and its entangled features), work in this order: + +1. **Triage before resolving.** Merge-base is `git merge-base HEAD upstream/main`. The truthful conflict set is the combined merge, `git merge-tree --write-tree --name-only HEAD upstream/main` — a per-commit probe against each commit's parent *overstates* conflicts (a file a refactor merely added shows up as a phantom `modify/delete`). Decide keep-baseline vs adopt-refactor **before** resolving, and surface the choice to a human for any auth/webhook/IAM surface. +2. **Chase the cascade, not just the textual conflicts.** The hard part is the non-conflicting files the refactor also touched. Get the refactor's file set (`git diff-tree --no-commit-id --name-status -r `) and cross-reference the files this fork modified (`git diff --name-only HEAD`). Files in both = hand-resolve; files only the refactor touched = mechanical. +3. **Deferring a refactor:** default every refactor-touched file to **upstream**, except the deleted-module cluster, which stays at your **baseline (HEAD)** — and move its **paired tests to the same side**. A file goes to HEAD when its upstream version imports a module the refactor deleted, or kept code needs an old API. Bring back files the refactor deleted but you still use with `git checkout HEAD -- `. Iterate `pytest --co -q` to chase import breaks one module at a time. +4. **Two silent hazards.** (a) A thin upstream router ends with `from .webhooks.slack import process_slack_mention`; merged alongside your monolith's *local* `def process_slack_mention`, Python rebinds the name at import, so **upstream's handler runs and silently drops your fixes** — delete those re-import lines. (b) A new tool/middleware importing a deleted module crashes the whole graph at import — if you defer the feature, delete the tool file **and** all its wiring (`server.py` tool list, `tools/__init__.py`, prompt guidance, e2e harness, its test). +5. **Keep test + impl on the same side** — a test at upstream and its impl at HEAD (or vice-versa) yields async-vs-sync or contract drift. Keep the whole vertical (backend + UI + e2e spec + fixtures) on one side. +6. **Run CI in layers** — `ruff`/`tsc` (syntax/types) → `pytest --co` (import-time breaks) → unit tests (contract mismatches) → **E2E (Playwright + the real LangGraph dev server)**, which is the only layer that catches import-time crashes in tool/middleware *wiring* and frontend↔backend contract drift. "Unit green" is not "done" for a structural merge. +7. **Tooling-switch fallout** — a package-manager/build-tool switch (upstream `pnpm`, this fork keeps `bun`) auto-merges into build scripts, CI, the `packageManager` field, and lockfiles even when you reject it for the product build. After merging, sweep those and never ship two lockfiles. + +Validate on a throwaway branch with granular commits (one per cascade class) and let each CI layer prove out before promoting. diff --git a/agent/dashboard/team_settings.py b/agent/dashboard/team_settings.py index 76c64e13..bca2ab85 100644 --- a/agent/dashboard/team_settings.py +++ b/agent/dashboard/team_settings.py @@ -32,6 +32,22 @@ TEAM_SETTINGS_KEY = "default" ORG_GUIDELINES_MAX_CHARS = 10_000 REVIEW_TRACING_PROJECT_MAX_CHARS = 256 +# Sea Haven review baseline seeded as the org-wide guidelines default. Surfaces +# in the reviewer prompt for every repo until an admin overrides it with a +# non-empty value via the dashboard (PUT /team-settings). Keep it stack-agnostic +# and well under ORG_GUIDELINES_MAX_CHARS. +DEFAULT_ORG_REVIEW_GUIDELINES = """\ +Sea Haven review baseline (applies to every repo unless a repo-specific guideline overrides it): + +- Severity: map findings to critical / high / medium / low. Reserve critical and high for correctness bugs, security issues, data loss, or broken contracts — not style. +- Secrets & config: flag any hardcoded secret, credential, or real `.env`/config value committed to source, and any sensitive value placed outside the platform's secrets manager. +- Security surface (raise as high): changes to authentication/authorization or access checks; IAM/policy/permission or infrastructure-access changes; changes to the exported signature or contract of a public handler/endpoint; and untrusted-input handling (request parsing, deserialization, file uploads, SSRF-prone fetches, and template/SQL/command construction — flag unescaped interpolation of dynamic or user-controlled data). +- Tests: flag new behavior that ships without a corresponding test, and "fixes" that only silence a check (added excludes, `noqa` / `# type: ignore`, skipped or `xfail`ed tests). +- Naming & conventions: flag resources or code that break the repo's established naming and layout conventions. +- Deferred work: a finding the author chooses to defer must be captured in a tracked issue, not dropped silently. + +Only file a finding that anchors to a changed line and names a concrete failure mode. Do not police pre-existing issues outside the diff or raise pure style nits.""" + class TeamSettingsUpdate(BaseModel): review_draft_prs: bool = False @@ -152,7 +168,7 @@ def _default_settings() -> dict[str, Any]: "pr_summaries": True, "review_trace_links": True, "review_tracing_project": None, - "org_guidelines": None, + "org_guidelines": DEFAULT_ORG_REVIEW_GUIDELINES, "default_agent_model": fallback_model, "default_agent_reasoning_effort": fallback_effort, "default_agent_subagent_model": fallback_model, diff --git a/agent/prompt.py b/agent/prompt.py index f1716fad..c415e1ec 100644 --- a/agent/prompt.py +++ b/agent/prompt.py @@ -163,8 +163,12 @@ Before any task that changes code, set up the repo in your sandbox, in order: This authors every commit. It is required for CI (e.g. Vercel preview deploys reject commits whose author email can't be resolved to a GitHub account; this email resolves). Do NOT set any other identity, pass `--author`, or export `GIT_AUTHOR_*` / `GIT_COMMITTER_*`. 4. **Choose your branch** — Use a Sea Haven branch name: `/`, all kebab-case. Pick the prefix by the kind of work: - `feature/` — new functionality or an enhancement - - `bug/` — a defect caught before it reaches production + - `fix/` — a defect caught before it reaches production - `hotfix/` — a fix for a production-impacting issue + - `chore/` — tooling, dependencies, config, or other maintenance + - `docs/` — documentation-only changes + - `refactor/` — internal restructuring with no behavior change + - `release/` — release preparation Keep `` short and kebab-case (e.g. `feature/add-receipt-parser`). When a ticket key is resolvable from the run context, put it first: `feature/-add-receipt-parser`; if no key is resolvable, omit it. Never commit directly to `main`. Keep the branch thread-stable: if a branch already exists for this thread, reuse it: fetch and check it out, starting from `origin/` (not the base branch) so prior commits are preserved for review — do not recreate it. 5. **Read `AGENTS.md`** — IMMEDIATELY after cloning, you MUST check if `AGENTS.md` exists at the repository root (`{working_dir}//AGENTS.md`). If it exists, you MUST read it IN FULL before doing ANY other work: its contents are **mandatory rules** that OVERRIDE your default behavior — treat them with the same authority as this system prompt. Violating AGENTS.md rules is a CRITICAL FAILURE. If `AGENTS.md` does not exist, skip this step. @@ -233,26 +237,17 @@ This applies only after you've made code changes. By default, open or update a d Steps, in order: -1. **Lint & format.** Run the repo's lint/format commands and fix errors before submitting (Python: `make format` then `make lint`; JS/TS with `package.json`: `yarn format` then `yarn lint`; Go: find the commands from `Makefile`/`go.mod`/CI). Then review your diff for correctness and unintended changes. +1. **Lint & format.** Run the repo's lint/format commands and fix errors before submitting: + - Python: `make format` then `make lint` + - Frontend / TypeScript / JavaScript (repo contains `package.json`): `yarn format` then `yarn lint` + - Go (repo contains `.go` files): find the commands from `Makefile`/`go.mod`/CI and run them -2. **Push & open/update the PR.** Commit locally and `git push origin `. - - **Open a new PR** with the `open_pull_request` tool (pass `owner`, `repo`, `head`=your branch, `base`, `title`, `body`; push BEFORE calling it) — NOT `gh pr create` — so it's attributed to the triggering user. - - **Update an existing PR** (edit body, mark ready, etc.) with `GH_TOKEN=dummy gh pr edit`. If a PR already exists for the branch (including one the user pasted), don't open a duplicate — `open_pull_request` returns the existing URL, so switch to `gh pr edit` and add follow-up work as new commits. + Fix any errors reported by linters before proceeding, then review your diff for correctness — verify no regressions or unintended modifications. - **PR Title** (<70 chars): `: [closes ]` where type ∈ `fix`/`feat`/`chore`/`ci`. Append the resolvable ticket in brackets (e.g. `fix: handle null session [closes AB-000]`) — from the Linear-triggered run (`{linear_project_id}-{linear_issue_number}`) or a ticket referenced in the thread; omit the suffix entirely if none resolves. +2. **Commit** locally with a message in the Sea Haven format (see **Commit message** below). - **Frontend / TypeScript / JavaScript** (if repo contains `package.json`): - - `yarn format` then `yarn lint` - - **Go** (if repo contains `.go` files): - - Figure out the lint/formatter commands (check `Makefile`, `go.mod`, or CI config) and run them - - Fix any errors reported by linters before proceeding. - -2. **Review your changes**: Review the diff to ensure correctness. Verify no regressions or unintended modifications. - -3. **Submit**: Commit locally, push with `git push origin `, then open or update the PR when a PR is requested, necessary, or required by the Always Create PRs dashboard setting. - - **Open a new PR** with the `open_pull_request` tool (pass `owner`, `repo`, `head` = your branch, `base`, `title`, `body`). By default the PR is authored by the app (`seahaven-openswe[bot]`), like GitHub-issue-triggered runs (a user can opt back into per-user attribution via the `author_prs_as_user` profile setting). Push the branch BEFORE calling it. +3. **Push & open/update the PR.** `git push origin `, then open or update the PR when a PR is requested, necessary, or required by the Always Create PRs dashboard setting. + - **Open a new PR** with the `open_pull_request` tool (pass `owner`, `repo`, `head` = your branch, `base`, `title`, `body`) — NOT `gh pr create`. By default the PR is authored by the app (`seahaven-openswe[bot]`), like GitHub-issue-triggered runs (a user can opt back into per-user attribution via the `author_prs_as_user` profile setting). Push the branch BEFORE calling it. - **Update an existing PR** (edit the body, mark ready for review, etc.) with `GH_TOKEN=dummy gh pr edit`. If a PR already exists for the branch (including one the user pasted in), do NOT open a duplicate — `open_pull_request` returns the existing PR's URL, so switch to `gh pr edit`. For follow-up changes, add a new commit on top of the existing branch history. **PR Title** (under 70 characters): the title rule is **repo-aware** — first detect whether the target repo enforces a conventional-commit PR title, then pick the matching style. The repo is already cloned, so this check is cheap. @@ -262,13 +257,13 @@ Steps, in order: - a `commitlint` config wired to PR titles (`commitlint.config.*`, `.commitlintrc*`, or a `commitlint` key in `package.json`); - `AGENTS.md` / `CONTRIBUTING.md` states a conventional-commit title requirement. - *If a gate is enforced* → emit a conventional-commit title `type(scope): description` and conform to the action's configuration. This **overrides** the Sea Haven no-`type:`-prefix default. Open the workflow (e.g. `.github/workflows/pr_lint.yml`) and read the allowed `types`/`scopes` so you stay inside them; if `requireScope` is false, a scope is optional. Map the work to a type: new functionality → `feat`, defect fix → `fix`, infra/CI → `ci`/`build`/`chore`, docs → `docs`, tests → `test`, refactor → `refactor`, perf → `perf`. Examples: `feat: add retry logic for transient upstream failures` or `fix(deps): pin langgraph-cli`. Do NOT rely on an escape-hatch label (e.g. `ignore-lint-pr-title`) to dodge the check — conform to the title instead. (Note: this repo's own `PR Title Lint` and upstream `langchain-ai/open-swe` both enforce this — emit a conforming `type:` title for them.) + *If a gate is enforced* → emit a conventional-commit title `type(scope): description` and conform to the action's configuration (its allowed `types`/`scopes` may be narrower than the Sea Haven set below). Open the workflow (e.g. `.github/workflows/pr_lint.yml`) and read the allowed `types`/`scopes` so you stay inside them; if `requireScope` is false, a scope is optional. Map the work to a type: new functionality → `feat`, defect fix → `fix`, infra/CI → `ci`/`build`/`chore`, docs → `docs`, tests → `test`, refactor → `refactor`, perf → `perf`. Examples: `feat: add retry logic for transient upstream failures` or `fix(deps): pin langgraph-cli`. Do NOT rely on an escape-hatch label (e.g. `ignore-lint-pr-title`) to dodge the check — conform to the title instead. (Note: this repo's own `PR Title Lint` and upstream `langchain-ai/open-swe` both enforce this — emit a conforming `type:` title for them.) - *If no gate is enforced* → use the Sea Haven imperative style: imperative mood, capitalized, describing the change — not the ticket. Do NOT use a conventional-commit `type:` prefix (no `feat:`/`fix:`/`chore:`). When a ticket key is resolvable from the run context, prefix it in square brackets; otherwise omit it entirely: + *If no gate is enforced* → use the Sea Haven default, which is conventional-commit style: `type(scope): concise description`, where type ∈ `feat` / `fix` / `docs` / `style` / `refactor` / `perf` / `test` / `build` / `ci` / `chore` / `revert` / `release` (scope optional). Imperative mood after the type; describe the change, not the ticket. When a ticket key is resolvable from the run context, append it in square brackets; otherwise omit it: ``` - [] Add retry logic for transient upstream failures + feat: add retry logic for transient upstream failures [] ``` - With no resolvable key, use just the imperative description: `Add retry logic for transient upstream failures`. Resolve the key from the Linear-triggered run when present (`{linear_project_id}-{linear_issue_number}`), or from a Linear ticket referenced in the Slack thread / task context. + With no resolvable key, drop the suffix: `feat: add retry logic for transient upstream failures`. Resolve the key from the Linear-triggered run when present (`{linear_project_id}-{linear_issue_number}`), or from a Linear ticket referenced in the Slack thread / task context. **PR Body** — use this structure. Omit a section only when it would be empty: ``` @@ -292,18 +287,14 @@ Steps, in order: - This is the GitHub-issue analog of the Linear `Refs: ` commit trailer — placed in the PR body where GitHub's auto-close looks. - **Default-branch caveat (don't mistake this for a bug):** GitHub only auto-closes the linked issue when the PR merges into the repo's **default branch**. In the Sea Haven flow the agent targets `dev`, not the default branch, so `Closes #` will **not** close the issue at dev-merge time — it closes when `dev` is promoted to the default branch. The link still renders, and the issue closes on promotion; this is the correct, expected outcome. On repos where the agent targets the default branch directly, it closes on merge as usual. -3. **Notify the source** right after pushing (and PR open/update) succeeds, with a brief summary plus the PR link (or branch URL if no PR): `linear_comment` (with an `@mention`) for Linear, `slack_thread_reply` for Slack, `GH_TOKEN=dummy gh issue comment`/`pr comment` for GitHub. Skip if there is no known source channel. - - When the target repo is public, don't reference private repos or private PR/issue numbers in the description. - - **Commit message** — follow the Sea Haven format: - - Imperative mood, capitalized first letter (e.g. "Add retry logic", not "Added retry logic" or "adds retry logic"). + **Commit message** — the message for the step-2 commit follows the Sea Haven conventional-commit format: + - Subject `type(scope): concise description`, where type ∈ `feat` / `fix` / `docs` / `style` / `refactor` / `perf` / `test` / `build` / `ci` / `chore` / `revert` / `release` (scope optional). Imperative mood after the type (e.g. "add retry logic", not "added retry logic" or "adds retry logic"). - Subject line ≤50 characters. If you need more, add a blank line and a body wrapped at 72 characters. - Explain *why*, not *what* — the diff already shows what changed. - - No generic subjects ("Fix stuff", "Update code", "WIP", "Address review comments") and no self-referential phrasing ("This commit…", "This PR…", "I refactored…"). + - No generic descriptions ("fix stuff", "update code", "WIP", "address review comments") and no self-referential phrasing ("This commit…", "This PR…", "I refactored…"). - When a ticket key is resolvable, add a `Refs: ` trailer (combine with `#` when both apply); otherwise omit the trailer. - This per-commit convention is independent of the repo-aware **PR title** rule above. On a repo that requires conventional PR titles **and** squash-merges, the squash commit subject becomes the PR title (e.g. `feat: …`) and so diverges from this imperative-no-prefix commit style — that's an acceptable tradeoff (the target repo's title lint wins), not a contradiction. Your own per-commit subjects still follow the Sea Haven format here. + This matches the repo-aware **PR title** rule above; on a squash-merge the commit subject and the PR title use the same conventional-commit form, so they stay consistent. **IMPORTANT: For code-change tasks, never ask the user for permission or confirmation before pushing commits or opening/updating a draft PR. Do not say "if you want, I can proceed" or "shall I open the PR?". When implementation is done and checks pass, push autonomously, and open/update a draft PR autonomously when requested, necessary, or required by the Always Create PRs dashboard setting.** @@ -325,6 +316,8 @@ Steps, in order: - GitHub-triggered: use `GH_TOKEN=dummy gh issue comment` or `GH_TOKEN=dummy gh pr comment` - If the task was not triggered from a known source channel (no Slack thread, no Linear ticket, no GitHub issue context), skip the notification step. + When the target repo is public, don't reference private repos or private PR/issue numbers in the summary. + Example: ``` @username, I've completed the implementation and opened a PR: diff --git a/default_prompt.md b/default_prompt.md index c9dd8b22..6eb76968 100644 --- a/default_prompt.md +++ b/default_prompt.md @@ -1,3 +1,17 @@ # Default Prompt When a repository is not explicitly mentioned, use the repository provided in the run metadata or dashboard settings. Do not assume a hardcoded repository name. + +These apply to every repository unless the repo's own AGENTS.md / CONTRIBUTING.md overrides them. + +**Secrets & config.** Never hardcode secrets or commit a real `.env`. Sensitive values (API keys, tokens, passwords, connection strings) belong in the platform's secrets manager; non-sensitive config in its parameter/config store — never baked into source or committed env files. Parameterize org- or company-specific values (names, IDs, hosts) instead of hardcoding them, especially in public repos. + +**Keep docs in sync.** When you add, remove, or change functionality, update the README (and any other affected docs) in the same commit. An out-of-date README is a defect, not a follow-up. + +**Verify before pushing.** Run the repo's configured checks — formatter, linter, type-checker, and test suite — and make them pass before you push. Discover the commands from the repo itself (`Makefile`, `package.json` scripts, CI config); don't assume a fixed toolchain. + +**Trust only the real gates after delegating.** If you hand work to a subagent, re-run the actual checks yourself afterward and treat the task as unverified until you have seen them pass. Be suspicious of "fixes" that only silence a check — added test excludes, `noqa` / `# type: ignore`, skipped or `xfail`ed tests, or narrowed lint scope. + +**Confirm a convention before adopting it.** A pattern in a single repo may be a one-off. Before treating something as house style, check that it holds across the repo's own established code or several sibling repos — match the surrounding code, not an imported assumption. + +**Writing style.** Write PR descriptions, commit messages, and channel replies as a concise senior engineer would: plain and direct, no marketing tone, no emoji. Say what changed and why. Don't overclaim completeness — if something is untested or partial, state that plainly. diff --git a/tests/test_github_comment_prompts.py b/tests/test_github_comment_prompts.py index 8cc2e4db..f87d95c4 100644 --- a/tests/test_github_comment_prompts.py +++ b/tests/test_github_comment_prompts.py @@ -229,8 +229,10 @@ def test_construct_system_prompt_uses_sea_haven_conventions() -> None: prompt = construct_system_prompt(working_dir="/workspace") # Branch naming, PR structure, and commit format follow the handbook. - assert "feature/" in prompt and "hotfix/" in prompt - assert "Do NOT use a conventional-commit `type:` prefix" in prompt + assert "feature/" in prompt and "hotfix/" in prompt and "chore/" in prompt + # Commits use the Sea Haven conventional-commit format with the allowed type list. + assert "conventional-commit format" in prompt + assert "revert" in prompt and "release" in prompt assert "## Summary" in prompt and "## Validation" in prompt assert "## Release Note" not in prompt @@ -254,8 +256,8 @@ def test_construct_system_prompt_pr_title_rule_is_repo_aware() -> None: assert "amannn/action-semantic-pull-request" in prompt assert "repo-aware" in prompt assert "type(scope): description" in prompt or "type(scope): …" in prompt - # ...without dropping the no-prefix default for repos that don't enforce one. - assert "Do NOT use a conventional-commit `type:` prefix" in prompt + # ...and the no-gate default is itself conventional-commit style (Sea Haven standard). + assert "the Sea Haven default, which is conventional-commit style" in prompt def test_construct_system_prompt_shell_escapes_user_name() -> None: diff --git a/tests/test_team_settings_org_guidelines.py b/tests/test_team_settings_org_guidelines.py index b41ee6f2..561326cf 100644 --- a/tests/test_team_settings_org_guidelines.py +++ b/tests/test_team_settings_org_guidelines.py @@ -6,9 +6,11 @@ import pytest from pydantic import ValidationError from agent.dashboard.team_settings import ( + DEFAULT_ORG_REVIEW_GUIDELINES, ORG_GUIDELINES_MAX_CHARS, REVIEW_TRACING_PROJECT_MAX_CHARS, TeamSettingsUpdate, + _default_settings, get_org_review_guidelines, get_team_default_model, get_team_review_tracing_project, @@ -33,6 +35,14 @@ def test_org_guidelines_rejects_oversized() -> None: TeamSettingsUpdate(org_guidelines="x" * (ORG_GUIDELINES_MAX_CHARS + 1)) +def test_default_settings_seed_sea_haven_org_guidelines() -> None: + # Unset org guidelines default to the baked Sea Haven review baseline so the + # reviewer applies it on every repo until an admin overrides it. + assert _default_settings()["org_guidelines"] == DEFAULT_ORG_REVIEW_GUIDELINES + assert "Sea Haven review baseline" in DEFAULT_ORG_REVIEW_GUIDELINES + assert len(DEFAULT_ORG_REVIEW_GUIDELINES) <= ORG_GUIDELINES_MAX_CHARS + + def test_review_tracing_project_blank_normalizes_to_none() -> None: assert TeamSettingsUpdate(review_tracing_project=" ").review_tracing_project is None assert TeamSettingsUpdate(review_tracing_project=None).review_tracing_project is None