#Code Review Batch 1 — Implementation Plan

def test_compress_preserves_inline_code_identifiers(tmp_path):
    from coding_scaffold.context import compress_context

    knowledge = tmp_path / ".coding-scaffold" / "knowledge"
    knowledge.mkdir(parents=True)
    note = knowledge / "note.md"
    note.write_text(
        "Use `the-prod-route` to deploy.\n"
        "See [the docs](./the-docs) for details.\n",
        encoding="utf-8",
    )

    result = compress_context(tmp_path, source="knowledge")

    assert not result.warnings, result.warnings
    compressed = (knowledge / "note.caveman.md").read_text(encoding="utf-8")
    assert "`the-prod-route`" in compressed, compressed
    assert "[the docs](./the-docs)" in compressed, compressed

    body = re.sub(r"\b(the|a|an)\b", "", body, flags=re.I)

SECRET_KEY_ENV_NAMES = (
    "OPENAI_API_KEY",
    "ANTHROPIC_API_KEY",
    "AZURE_OPENAI_API_KEY",
    "AZURE_AI_API_KEY",
    "AZURE_AI_SERVICES_KEY",
    "AZURE_COGNITIVE_SERVICES_KEY",
    "OPENROUTER_API_KEY",
    "GROQ_API_KEY",
    "GEMINI_API_KEY",
    "GOOGLE_API_KEY",
    "GITHUB_TOKEN",
    "GH_TOKEN",
)

AZURE_NONKEY_ENV_NAMES = (
    "AZURE_OPENAI_ENDPOINT",
    "AZURE_OPENAI_DEPLOYMENT",
    "AZURE_AI_ENDPOINT",
    "AZURE_AI_MODEL",
    "AZURE_AI_MODEL_FAMILY",
    "AZURE_AI_SERVICES_ENDPOINT",
    "AZURE_COGNITIVE_SERVICES_ENDPOINT",
)

# Union: everything that belongs in .env.local rather than committed config.
# Azure endpoints/deployments are treated as sensitive because the subdomain
# typically encodes tenant identity.
SECRET_ENV_NAMES = SECRET_KEY_ENV_NAMES + AZURE_NONKEY_ENV_NAMES

def test_secret_env_names_separates_keys_from_azure_config():
    from coding_scaffold.credentials import (
        AZURE_NONKEY_ENV_NAMES,
        SECRET_ENV_NAMES,
        SECRET_KEY_ENV_NAMES,
    )

    assert "OPENAI_API_KEY" in SECRET_KEY_ENV_NAMES
    assert "AZURE_OPENAI_ENDPOINT" in AZURE_NONKEY_ENV_NAMES
    assert "AZURE_OPENAI_ENDPOINT" not in SECRET_KEY_ENV_NAMES
    # Backwards-compat: the union is still importable under the old name.
    assert set(SECRET_KEY_ENV_NAMES) <= set(SECRET_ENV_NAMES)
    assert set(AZURE_NONKEY_ENV_NAMES) <= set(SECRET_ENV_NAMES)

REDACTED_PLACEHOLDER = "<configured locally; see .env.local>"


@dataclass(frozen=True)
class Provider:
    name: str
    kind: str
    available: bool
    status: str
    endpoint: str | None = None
    model_family: str | None = None
    deployment: str | None = None
    redact_fields: tuple[str, ...] = ()

    def to_dict(self) -> dict[str, object]:
        data = asdict(self)
        for field in self.redact_fields:
            if data.get(field):
                data[field] = REDACTED_PLACEHOLDER
        data.pop("redact_fields", None)
        return data

return Provider(
    "azure-openai",
    "cloud",
    available,
    status,
    endpoint=endpoint,
    model_family="openai",
    deployment=deployment,
    redact_fields=("endpoint", "deployment"),
)

def test_azure_openai_provider_redacts_endpoint_and_deployment():
    from coding_scaffold.providers import REDACTED_PLACEHOLDER, detect_providers

    env = {
        "AZURE_OPENAI_API_KEY": "key-xyz",
        "AZURE_OPENAI_ENDPOINT": "https://contoso-prod.openai.azure.com/",
        "AZURE_OPENAI_DEPLOYMENT": "gpt-4-internal",
    }
    providers = detect_providers(env=env)
    azure = next(p for p in providers if p.name == "azure-openai")

    # In-memory values stay intact for routing.
    assert azure.endpoint == "https://contoso-prod.openai.azure.com/"
    assert azure.deployment == "gpt-4-internal"

    # Serialized form redacts.
    serialized = azure.to_dict()
    assert serialized["endpoint"] == REDACTED_PLACEHOLDER
    assert serialized["deployment"] == REDACTED_PLACEHOLDER
    assert "redact_fields" not in serialized
    # Non-redacted providers serialize unchanged.
    openai = next(p for p in providers if p.name == "openai")
    assert "redact_fields" not in openai.to_dict()

def test_providers_json_redacts_azure_endpoint(tmp_path, monkeypatch):
    from coding_scaffold.providers import detect_providers
    from coding_scaffold.writers import write_scaffold

    monkeypatch.setenv("AZURE_OPENAI_API_KEY", "k")
    monkeypatch.setenv("AZURE_OPENAI_ENDPOINT", "https://contoso.openai.azure.com/")
    monkeypatch.setenv("AZURE_OPENAI_DEPLOYMENT", "internal-gpt")

    # Use whatever the existing write_scaffold signature requires; mirror an
    # existing test in test_writers.py.
    write_scaffold(tmp_path, providers=detect_providers())

    providers_json = (tmp_path / ".coding-scaffold" / "providers.json").read_text(encoding="utf-8")
    assert "contoso.openai.azure.com" not in providers_json
    assert "internal-gpt" not in providers_json

def deep_merge_mapping(
    base: dict[str, object],
    overlay: dict[str, object],
    deep_keys: tuple[str, ...] = (),
) -> dict[str, object]:
    """Merge ``overlay`` into ``base``. For top-level keys in ``deep_keys``,
    if both sides are mappings the merge recurses one level; otherwise overlay
    wins. Non-listed keys use shallow overlay-wins.
    """
    result: dict[str, object] = dict(base)
    for key, value in overlay.items():
        if (
            key in deep_keys
            and isinstance(value, dict)
            and isinstance(result.get(key), dict)
        ):
            result[key] = {**result[key], **value}  # type: ignore[dict-item]
        else:
            result[key] = value
    return result

def test_policy_preserves_user_mcp_servers(tmp_path):
    import json
    from coding_scaffold.policy import write_policy_pack

    target = tmp_path
    (target / ".coding-scaffold").mkdir()
    opencode = target / "opencode.json"
    opencode.write_text(json.dumps({
        "mcp": {
            "user-defined-server": {"command": "user-server", "enabled": True},
        },
    }), encoding="utf-8")

    write_policy_pack(target, scope="team", disabled_mcp_servers=["bad-server"])

    new_file = target / "opencode.json.new"
    assert new_file.exists(), "policy must stage opencode.json.new when opencode.json exists"
    # Original untouched.
    original = json.loads(opencode.read_text(encoding="utf-8"))
    assert original == {"mcp": {"user-defined-server": {"command": "user-server", "enabled": True}}}
    # Staged file deep-merges: user's server survives alongside the disabled one.
    staged = json.loads(new_file.read_text(encoding="utf-8"))
    assert staged["mcp"]["user-defined-server"] == {"command": "user-server", "enabled": True}
    assert staged["mcp"]["bad-server"] == {"enabled": False}

def _merge_opencode_config(path: Path, policy: dict[str, object]) -> tuple[Path, str | None]:
    from .file_ops import deep_merge_mapping

    current: dict[str, object] = {}
    warning = None
    target_path = path
    file_existed = path.exists()
    if file_existed:
        try:
            loaded = json.loads(path.read_text(encoding="utf-8"))
        except json.JSONDecodeError:
            warning = f"Could not parse {path}; wrote policy overlay to {path}.new instead."
            target_path = path.with_suffix(path.suffix + ".new")
            target_path.write_text(json.dumps(policy, indent=2, sort_keys=True) + "\n", encoding="utf-8")
            return target_path, warning
        if isinstance(loaded, dict):
            current = loaded

    merged = deep_merge_mapping(current, policy, deep_keys=("mcp", "permission"))
    if "instructions" in current or "instructions" in policy:
        merged["instructions"] = _merge_list(current.get("instructions"), policy.get("instructions"))

    if file_existed:
        target_path = path.with_suffix(path.suffix + ".new")
        warning = (
            f"Staged {target_path.name}; review and `mv {target_path.name} {path.name}` to apply."
        )
    target_path.write_text(json.dumps(merged, indent=2, sort_keys=True) + "\n", encoding="utf-8")
    return target_path, warning

def test_team_sync_never_touches_knowledge_dir(tmp_path):
    """User-owned .coding-scaffold/knowledge/ files survive team sync."""
    import json
    from coding_scaffold.team import sync_team

    knowledge = tmp_path / ".coding-scaffold" / "knowledge"
    knowledge.mkdir(parents=True)
    (knowledge / "secret-note.md").write_text("private", encoding="utf-8")

    # Manifest with a benign local-path remote (allow_local enabled).
    remote = tmp_path / "team-remote"
    remote.mkdir()
    (remote / "shared.md").write_text("shared", encoding="utf-8")
    manifest = tmp_path / ".coding-scaffold" / "team-onboarding.json"
    manifest.write_text(json.dumps({
        "team": "t",
        "knowledge": {"backend": "markdown", "path": ".coding-scaffold/knowledge", "remote": str(remote)},
    }), encoding="utf-8")

    sync_team(tmp_path, allow_local=True)

    assert (knowledge / "secret-note.md").exists()
    assert (knowledge / "secret-note.md").read_text(encoding="utf-8") == "private"


def test_team_sync_rejects_local_remote_without_allow_local(tmp_path):
    import json
    from coding_scaffold.team import sync_team

    remote = tmp_path / "team-remote"
    remote.mkdir()
    manifest = tmp_path / ".coding-scaffold" / "team-onboarding.json"
    manifest.parent.mkdir(parents=True, exist_ok=True)
    manifest.write_text(json.dumps({
        "team": "t",
        "knowledge": {"backend": "markdown", "path": ".coding-scaffold/knowledge", "remote": str(remote)},
    }), encoding="utf-8")

    result = sync_team(tmp_path)
    assert any("allow-local" in w.lower() or "local path" in w.lower() for w in result.warnings)


def test_team_sync_keeps_git_directory_inside_repo_subdir(tmp_path, monkeypatch):
    """Cloned repo retains .git inside _repo so subsequent syncs can ff-pull."""
    # Will require monkeypatching subprocess.run for git clone/pull and
    # asserting that team/sources/<kind>/<slug>/_repo/.git is created.
    # Detailed implementation deferred to the agent — keep this stub and
    # implement once the team.py rewrite lands so the helpers exist.
    pytest.skip("Implement after team.py rewrite — see plan task 4 step 3.")


def test_team_sync_strips_nested_git_dirs_from_agent_path(tmp_path):
    """Submodule .git directories don't leak into the agent-readable copy."""
    pytest.skip("Implement after team.py rewrite — see plan task 4 step 3.")

ALLOWED_REMOTE_SCHEMES = ("https", "http", "ssh")
SOURCES_SUBDIR = Path(".coding-scaffold") / "team" / "sources"
KNOWLEDGE_FORBIDDEN_PREFIX = Path(".coding-scaffold") / "knowledge"


def _classify_remote(remote: str) -> str:
    """Return 'url', 'local', or raise ValueError for empty/unknown."""
    if not remote:
        raise ValueError("Remote is empty.")
    if "://" in remote:
        scheme = remote.split("://", 1)[0].lower()
        if scheme in ALLOWED_REMOTE_SCHEMES:
            return "url"
        if scheme == "file":
            return "local"
        raise ValueError(f"Unsupported remote scheme: {scheme}")
    if remote.startswith("git@"):
        return "url"
    return "local"  # bare path


def _team_destination(root: Path, kind: str, remote: str) -> Path:
    """Compute the team-sources destination for a (kind, remote) pair.
    Always lives under SOURCES_SUBDIR — never under knowledge/."""
    slug = _slug(remote)
    return root / SOURCES_SUBDIR / kind / slug


def _sync_source(
    remote: str,
    destination: Path,
    *,
    dry_run: bool,
    allow_local: bool,
) -> tuple[str, str | None]:
    """Copy or clone `remote` into `destination`. Never touches paths outside
    SOURCES_SUBDIR."""
    classification = _classify_remote(remote)
    if classification == "local" and not allow_local:
        return (
            f"refused {remote}",
            "Local-path remote requires --allow-local. Skipped.",
        )
    destination.parent.mkdir(parents=True, exist_ok=True)
    if classification == "local":
        source = Path(remote).expanduser()
        if not source.exists():
            return f"skipped {remote}", f"Local path does not exist: {remote}"
        if dry_run:
            return f"would copy {remote}", None
        # Stage into _repo to mirror clone layout (no .git for local copies).
        repo_dir = destination / "_repo"
        if repo_dir.exists():
            shutil.rmtree(repo_dir)
        shutil.copytree(source, repo_dir, ignore=shutil.ignore_patterns(".git"))
        _publish_repo_contents(repo_dir, destination)
        return f"copied {remote}", None
    # URL clone
    return _clone_or_pull(remote, destination, dry_run=dry_run), None


def _clone_or_pull(remote: str, destination: Path, *, dry_run: bool = False) -> str:
    if shutil.which("git") is None:
        raise RuntimeError(
            "git is required for team manifests pointing to a remote URL. "
            "Install git or pass a local path with --allow-local."
        )
    if dry_run:
        return f"would clone/update {remote}"
    repo_dir = destination / "_repo"
    repo_dir.parent.mkdir(parents=True, exist_ok=True)
    if (repo_dir / ".git").exists():
        completed = subprocess.run(
            ["git", "-C", str(repo_dir), "pull", "--ff-only"],
            check=False,
            capture_output=True,
            text=True,
            timeout=300,
        )
        if completed.returncode == 0:
            _publish_repo_contents(repo_dir, destination)
            return f"updated {remote}"
        return f"kept existing checkout for {remote}; git pull failed"
    # Fresh clone (or recovery from a stale partial checkout)
    if destination.exists():
        # Move aside, don't delete user-visible content.
        legacy = destination.with_name(destination.name + f".legacy-{datetime.now(UTC):%Y%m%d-%H%M%S}")
        destination.rename(legacy)
    repo_dir.parent.mkdir(parents=True, exist_ok=True)
    completed = subprocess.run(
        ["git", "clone", remote, str(repo_dir)],
        check=False,
        capture_output=True,
        text=True,
        timeout=300,
    )
    if completed.returncode != 0:
        raise RuntimeError(f"Could not clone {remote}: {completed.stderr.strip()}")
    _publish_repo_contents(repo_dir, destination)
    return f"cloned {remote}"


def _publish_repo_contents(repo_dir: Path, destination: Path) -> None:
    """Copy *.md (and other manifest-allowed files) from repo_dir to destination
    root, skipping any .git directories at any depth. Idempotent — clears
    previously published markdown first."""
    for stale in destination.iterdir() if destination.exists() else []:
        if stale.name == "_repo":
            continue
        if stale.is_dir():
            shutil.rmtree(stale)
        else:
            stale.unlink()
    for path in repo_dir.rglob("*"):
        if ".git" in path.parts:
            continue
        if not path.is_file():
            continue
        relative = path.relative_to(repo_dir)
        target = destination / relative
        target.parent.mkdir(parents=True, exist_ok=True)
        shutil.copy2(path, target)

team.add_argument(
    "--allow-local",
    action="store_true",
    help="Permit local-path or file:// remotes for team manifests.",
)

elif action == "connect":
    result = connect_team(target, manifest=args.manifest, allow_local=args.allow_local)
elif action == "sync":
    result = sync_team(target, dry_run=args.dry_run, allow_local=args.allow_local)