//! Changing an issue: `issue create`, `edit`, `close`, `reopen` and
//! `comment`.
//!
//! The write half of [`crate::cmd::issue`], and the half where a mistake is
//! not a wrong line of output but a record in somebody's repository. Every
//! command here settles which account it is acting as before it reads
//! anything and announces it, because that decision picks both whose PDS is
//! written to and whether the write will be honoured; all of them take
//! `--dry-run`, and the dry run is expected to answer "from whom?" as well as
//! "what?".
//!
//! Three things recur, and they are the same three [`crate::cmd::pr::write`]
//! names — which is the point, because an issue is the same kind of object as
//! a pull request and behaving differently would be a second thing to learn:
//!
//! - **Naming an issue is the one place this is simpler.** A pull can be
//!   named by its appview number, which costs a page fetch to translate; an
//!   issue cannot be named that way here at all, so
//!   [`super::read::classify_issue_ref`] stays a pure function and no command
//!   spends a request working out what it was asked about.
//! - **Read-modify-write is a race.** `issue edit` reads the record, changes
//!   a field and puts it back, and without a precondition two of them running
//!   at once both report success while one edit quietly disappears. It goes
//!   through [`crate::clients::atproto::record::put`], which sends the CID it
//!   read as `swapRecord`.
//! - **State is a log, not a field.** An issue's state is whatever the newest
//!   `sh.tangled.repo.issue.state` record says, so `issue reopen` appends an
//!   `open` record rather than deleting a `closed` one — see [`crate::model::record::Standing`] for
//!   whose records Tangled will actually honour.
//!
//! # `issue comment` writes `sh.tangled.feed.comment`
//!
//! Not `sh.tangled.repo.issue.comment`, which is the obvious record and the
//! wrong one; [`crate::lexicon::tangled::LEGACY_ISSUE_COMMENT_NSID`] carries
//! the evidence. This is the same choice `pr comment` made and for the same
//! reason, one collection over.

use super::read::{self, IssueRef, State};
use crate::clients::atproto::record::Key;
use crate::clients::git::run as git;
use crate::clients::tangled::resolve;
use crate::clients::tangled::scope;
use crate::cmd::auth;
use crate::lexicon::tangled::{ISSUE_NSID, ISSUE_STATE_NSID, IssueState};
use crate::model::record::{Standing, standing_of};
use anyhow::{Context, Result};
use jacquard::client::AgentSessionExt;
use jacquard::types::string::{AtUri, Cid, Datetime, Did};
use tangled_lexicon::LexiconSchema;
use tangled_lexicon::com_atproto::repo::strong_ref::StrongRef;
use tangled_lexicon::sh_tangled::feed::comment::Comment as FeedComment;
use tangled_lexicon::sh_tangled::repo::issue::Issue;
use tangled_lexicon::sh_tangled::repo::issue::state::{State as IssueStateRecord, StateState};

// ---------------------------------------------------------------------------
// `--json` for the commands that write
// ---------------------------------------------------------------------------
//
// A read command's `--json` answers "what is there"; these answer "what did
// I just do", which is a different contract and has one field the read shapes
// do not: `dry_run`. It is on every one of them, always, and it is what a
// caller checks rather than remembering which flags it passed. Under
// `--dry-run` the identifiers the write would mint are `null` rather than
// guessed at: the record key is the PDS's to choose, and inventing one would
// be the single most misleading thing this output could do.
//
// None of them carry a `number` or a per-issue `url`, for the reason
// `issue list --json` does not: the number is the appview's own id, so an
// issue's own page is not addressable from anything a write returns. `url` is
// the repo's issue *listing*, which is addressable and is where a person
// would go looking.

/// What `issue create` did, or would have done.
#[derive(serde::Serialize, Debug, PartialEq)]
pub(super) struct CreatedJson {
    pub dry_run: bool,
    /// The issue record's at-uri, `null` on a dry run.
    pub uri: Option<String>,
    /// Its record key, `null` on a dry run — the PDS mints it, and every
    /// `issue` verb takes it, so it is the identifier worth carrying.
    pub rkey: Option<String>,
    pub title: String,
    pub repo_did: String,
    /// The repo's issue listing. Known before the write, so it is filled in
    /// on a dry run too — unlike `uri`, nothing about it is invented.
    pub url: String,
    pub body_bytes: usize,
    /// The local images this body named, uploaded to the acting account's
    /// PDS and swapped for `blob+at://` URIs. `[]` when it named none, and
    /// filled in on a dry run too: the scan happens before the network, so
    /// a preview knows every file it would send and every one it refuses.
    pub images: Vec<crate::cmd::images::ImageJson>,
}

/// What `issue edit` did, or would have done.
#[derive(serde::Serialize, Debug, PartialEq)]
pub(super) struct EditedJson {
    pub dry_run: bool,
    pub uri: String,
    pub rkey: String,
    /// The title as it stands after the edit — the new one when `--title`
    /// changed it, the old one otherwise.
    pub title: String,
    pub title_changed: bool,
    pub body_changed: bool,
    /// `false` when the record already said what was asked for; nothing is
    /// written then, and the command still exits 0.
    pub changed: bool,
    /// The local images the new body named. `[]` when it named none, or when
    /// no body was passed at all.
    pub images: Vec<crate::cmd::images::ImageJson>,
}

/// What `issue close` and `issue reopen` did, or would have done.
#[derive(serde::Serialize, Debug, PartialEq)]
pub(super) struct StateChangeJson {
    pub dry_run: bool,
    /// The *state* record written, `null` on a dry run and also when the
    /// issue already had the wanted state — state is a log of records, and
    /// nothing is appended to it for a no-op.
    pub uri: Option<String>,
    pub issue_uri: String,
    pub rkey: String,
    pub title: String,
    /// The issue's author, not the actor — the acting account is on stderr,
    /// and the two differ exactly when a repo owner closes somebody's issue.
    pub author_did: String,
    /// `null` when nothing available could settle the state before the write.
    /// That is not the same as "open", and this is where the difference is
    /// visible to a script.
    pub state_before: Option<String>,
    pub state_after: String,
    /// `false` when the issue already read the wanted state. The command
    /// exits 0 either way, so this is the field that tells them apart.
    pub changed: bool,
    pub url: Option<String>,
}

/// What `issue comment` did, or would have done.
#[derive(serde::Serialize, Debug, PartialEq)]
pub(super) struct CommentedJson {
    pub dry_run: bool,
    /// The comment record's at-uri, `null` on a dry run.
    pub uri: Option<String>,
    pub issue_uri: String,
    pub issue_rkey: String,
    pub title: String,
    /// The issue's author, not the commenter — a comment needs no permission
    /// from anyone, so this is context rather than a check.
    pub author_did: String,
    pub body_bytes: usize,
    pub url: Option<String>,
    /// The local images this comment named. `pr comment --json` carries the
    /// same field, and a comment on an issue and one on a pull are the same
    /// `sh.tangled.feed.comment` record.
    pub images: Vec<crate::cmd::images::ImageJson>,
}

// ---------------------------------------------------------------------------
// Where a body comes from
// ---------------------------------------------------------------------------

/// Where a new body comes from: [`crate::term::body::read`], with this
/// family's noun in the error lines.
///
/// A wrapper rather than a call at each site, because the nested `Option` it
/// returns is what [`edit`] reads to tell "leave the body alone" from "clear
/// it", and naming that here keeps the three call sites reading the same as
/// they did.
fn new_body(body: Option<String>, body_file: Option<String>) -> Result<Option<Option<String>>> {
    crate::term::body::read(body, body_file, "body")
}

/// [`new_body`] without the clear-it case, for a comment.
fn comment_body(body: Option<String>, body_file: Option<String>) -> Result<String> {
    crate::term::body::required(body, body_file, "comment body", "comment")
}

/// An issue body is required, whatever the lexicon says, and this is the
/// refusal.
///
/// `sh.tangled.repo.issue` marks `body` optional and tangled.org does not
/// honour that: the appview's `Issue.Validate` refuses `body is empty`, and
/// its ingester turns a validation failure into `failed to ingest record,
/// dropping it without retry`. A title-only issue is therefore accepted by
/// the PDS, federates, and never appears anywhere — the same silent shape
/// [`crate::lexicon::tangled::LEGACY_ISSUE_COMMENT_NSID`] describes for a
/// comment in the wrong collection, arrived at from the other direction.
///
/// Refused here rather than warned about, because the record cannot be
/// unpublished once written and nothing inside the process can observe that
/// it went nowhere. `what` names which half is refusing: filing an issue
/// without a body, or emptying one that has it.
pub(in crate::cmd) fn empty_body_refusal(what: &str) -> anyhow::Error {
    crate::exit::fail(
        crate::exit::Exit::Usage,
        format!(
            "{what}\n\
             the lexicon marks an issue's body optional and tangled.org does not: its \
             ingester refuses a record whose body is empty (`issue body is empty`) and drops \
             it without retry, so the record would sit in your PDS, federate, and never \
             appear\n\
             atgc does not open an editor: `--body-file -` and a heredoc are the long-body \
             path"
        ),
    )
}

/// A body, described rather than printed. An issue body is arbitrarily long
/// and the point of the line is that it changed, not what it now says.
fn describe_body(body: Option<&str>) -> String {
    match body {
        None => "(none)".to_string(),
        Some(text) => format!("{} bytes", text.len()),
    }
}

// ---------------------------------------------------------------------------
// `issue create`
// ---------------------------------------------------------------------------

#[derive(clap::Args, Debug)]
pub(crate) struct CreateArgs {
    /// Issue title
    #[arg(short, long)]
    pub title: String,
    /// Issue body; required, whatever the lexicon says
    #[arg(short, long)]
    pub body: Option<String>,
    /// Read the body from a file; `-` reads stdin
    #[arg(long, conflicts_with = "body")]
    pub body_file: Option<String>,
    /// Repo to file against: owner/name, an at:// URI, a Tangled URL, or a DID
    #[arg(long, conflicts_with = "remote")]
    pub repo: Option<String>,
    /// Git remote pointing at the repo to file against
    #[arg(long, default_value = "origin", conflicts_with = "repo")]
    pub remote: String,
    /// Describe the issue without sending anything
    #[arg(long)]
    pub dry_run: bool,
    /// Print one JSON object describing what was written (or, with
    /// --dry-run, what would be) instead of the summary lines
    #[arg(long)]
    pub json: bool,
}

/// File an issue against a repo, specified by name or URL.
///
/// The repo may be specified with `--repo` (owner/name, at:// URI, or Tangled URL)
/// or determined from the git checkout's remote. The record lands in the acting account's
/// own PDS and names the repo by the repo's own DID, so this needs no permission on the
/// repo — the same property `pr create` has, and for the same reason: it is a record in your
/// repository that happens to name theirs.
pub(crate) async fn create(args: CreateArgs) -> Result<()> {
    crate::term::jsonout::init(args.json);
    // Settled before anything is read, so that --dry-run answers "who would
    // this be from?" as well as "what would it say?".
    let selection = crate::config::account::select().await?;
    selection.announce();

    let title = args.title.trim().to_string();
    if title.is_empty() {
        return Err(crate::exit::fail(
            crate::exit::Exit::Usage,
            "an issue needs a title; --title cannot be empty",
        ));
    }
    // Read before touching the network: a missing file is the likeliest
    // mistake, and finding out after three round trips is worse.
    let Some(body) = new_body(args.body, args.body_file)?.flatten() else {
        return Err(empty_body_refusal(
            "an issue needs a body; pass --body, or --body-file (with `-` for stdin)",
        ));
    };

    // Scanned on the same principle as the body is read: a local image path
    // that names no file, a non-image or an oversized one refuses here, by
    // name, before anything has left the machine.
    let images = crate::cmd::images::scan(&body)?;

    let repo = if let Some(ref repo_arg) = args.repo {
        // If --repo is provided, resolve it using the shared helper
        let resolution = crate::cmd::repo::resolve_repo_and_owner(repo_arg).await?;
        let repo_did = match resolution.record.repo_did.as_ref() {
            Some(did) => did.to_string(),
            None => {
                // Fall back to clone redirect probe for old records
                let owner_path = resolution
                    .owner_handle
                    .as_deref()
                    .unwrap_or(&resolution.owner_did);
                let repo_url = format!(
                    "{}/{owner_path}/{}",
                    crate::clients::endpoints::appview(),
                    resolution.name
                );
                match resolve::repo_ref(&repo_url).await {
                    Ok(found) => found.did,
                    Err(e) => {
                        crate::logging::debug::dump_err("repo DID lookup failed", &e);
                        return Err(anyhow::anyhow!(
                            "could not determine repo DID for {}/{}",
                            resolution.owner_did,
                            resolution.name
                        ));
                    }
                }
            }
        };
        let owner_path = resolution
            .owner_handle
            .as_deref()
            .unwrap_or(&resolution.owner_did);
        let web_url = format!(
            "{}/{owner_path}/{}",
            crate::clients::endpoints::appview(),
            resolution.name
        );
        resolve::RepoRef {
            did: repo_did,
            web_url,
        }
    } else {
        // Otherwise, use the git remote as before
        let remote_url = git::remote_url(&args.remote)?;
        resolve::repo_ref(&remote_url).await?
    };

    let mut report = CreatedJson {
        dry_run: args.dry_run,
        uri: None,
        rkey: None,
        title: title.clone(),
        repo_did: repo.did.clone(),
        url: read::issues_url(&repo.did),
        body_bytes: body.len(),
        images: images.listed(),
    };

    if !args.json {
        println!("repo:   {}", repo.linked());
        println!("title:  {title}");
        println!("body:   {}", describe_body(Some(&body)));
        for line in images.describe() {
            println!("image:  {line}");
        }
    }

    let filed =
        match write_issue(&selection, &repo.did, &title, &body, &images, args.dry_run).await? {
            Some(filed) => filed,
            None => {
                if args.json {
                    return crate::term::jsonout::emit(&report);
                }
                println!("dry run; nothing sent");
                return Ok(());
            }
        };

    if args.json {
        report.uri = Some(filed.uri);
        report.rkey = Some(filed.rkey);
        crate::term::jsonout::emit(&report)?;
        note_lag();
        return Ok(());
    }
    println!("created {}", filed.uri);
    println!("key:    {}", filed.rkey);
    println!(
        "view:   {}",
        crate::term::hyperlink::url(&read::issues_url(&repo.did))
    );
    note_lag();
    Ok(())
}

/// What [`write_issue`] landed: the two identifiers only the PDS could
/// supply.
pub(in crate::cmd) struct FiledIssue {
    pub uri: String,
    /// The record key the PDS minted, which every other `issue` verb takes.
    pub rkey: String,
}

/// Compose, check and write one `sh.tangled.repo.issue` record.
///
/// Everything about filing an issue that must not differ between the two
/// commands that file one: [`create`], which takes the repo from a git
/// remote, and [`crate::cmd::report`], which takes it from atgc's own
/// address because a bug report is about atgc wherever it is typed. What
/// stays with the callers is what they print and what their `--json` says;
/// what lives here is the record, the lexicon check, the scope, the blob
/// uploads and the write, because a bug report that federated and never
/// appeared would be the same silent failure `create` was built to refuse.
///
/// `Ok(None)` is a dry run: the record was built and validated, the scope a
/// real run needs was checked and warned about, and nothing left the
/// machine. No blob is uploaded on that path — a blob nothing references is
/// one the PDS may collect.
pub(in crate::cmd) async fn write_issue(
    selection: &crate::config::account::Selection,
    repo_did: &str,
    title: &str,
    body: &str,
    images: &crate::cmd::images::Images,
    dry_run: bool,
) -> Result<Option<FiledIssue>> {
    // Built and checked before the dry-run return, not after it, so a dry
    // run cannot report "would work" for a record the lexicon refuses.
    //
    // What is checked is the record as the caller wrote it. The two fields
    // an upload changes are filled in below, once a real run has one: `body`
    // gains the `blob+at://` spellings and `blobs` gains their CIDs, neither
    // of which a dry run may mint. Every constraint the lexicon puts on an
    // issue is on the fields already here.
    let mut record: Issue = Issue {
        repo: Did::new(repo_did.to_string().into())
            .map_err(|e| anyhow::anyhow!("{repo_did} is not a valid repo DID: {e}"))?,
        title: title.into(),
        body: Some(body.to_string().into()),
        created_at: Datetime::now(),
        // An issue atgc opens carries nothing else the struct names: the web
        // UI is what sets mentions and references, and both are recorded in
        // TODO.md as deliberately absent here.
        mentions: None,
        references: None,
        blobs: None,
        extra_data: None,
    };
    record
        .validate()
        .map_err(|e| anyhow::anyhow!("{e}"))
        .context("issue record would be refused by the lexicon")?;

    let scope_check = || {
        scope::require_scope(
            crate::cmd::acting_scope(&selection.did).as_deref(),
            selection.handle.as_deref(),
            ISSUE_NSID,
        )
    };
    if dry_run {
        // The scope a real run would need, checked here so a dry run says
        // what it would cost — as a warning, since it describes the session
        // rather than the issue.
        if let Err(e) = scope_check() {
            crate::term::say::warning!(Auth, "would fail: {e}");
        }
        return Ok(None);
    }
    scope_check()?;

    let agent = auth::agent_for_did(&selection.did).await?;

    // Uploaded only once the write is really going to happen: a blob nothing
    // references is one the PDS may collect, and a dry run must leave none
    // behind. The body that goes in the record is the rewritten one, with
    // every local path replaced by the `blob+at://` URI Tangled resolves.
    if !images.is_empty() {
        let (rewritten, image_blobs) =
            crate::cmd::images::upload_and_rewrite(&agent, &selection.did, body, images).await?;
        record.body = Some(rewritten.into());
        // Every uploaded blob has to be listed, or the PDS is free to collect
        // it and the issue renders broken later with nothing having been
        // deleted. Through `merge_blobs` even on a fresh record: two
        // spellings of one file upload the same content-addressed blob, and
        // the list should carry each CID once.
        record.blobs = crate::cmd::images::merge_blobs(None, image_blobs)
            .map(|bs| bs.into_iter().map(Into::into).collect());
        // Again, because the two fields an upload changes are the two the
        // check above could not see. Only when there were images: without
        // them the record is byte for byte the one already validated.
        record
            .validate()
            .map_err(|e| anyhow::anyhow!("{e}"))
            .context("issue record would be refused by the lexicon")?;
    }

    crate::logging::debug::log(format!(
        ">> createRecord {ISSUE_NSID}\n{}",
        crate::logging::debug::pretty(&record)
    ));
    // No rkey: the PDS mints a TID, which is what the lexicon's `key: "tid"`
    // asks for and what Tangled's own writer does.
    let output = agent.create_record(record, None).await.map_err(|e| {
        crate::logging::debug::dump_err("createRecord error", &e);
        scope::scope_refusal(
            "creating the issue record",
            selection.handle.as_deref(),
            &e.to_string(),
        )
    })?;

    let uri = output.uri.to_string();
    let rkey = uri.rsplit('/').next().unwrap_or_default().to_string();
    Ok(Some(FiledIssue { uri, rkey }))
}

/// The one thing every write here has to say afterwards.
///
/// A record is in the PDS the instant the write returns, and on tangled.org
/// whenever its ingester gets to it. Saying so is what stops "it is not on
/// the page" being read as "the write failed" — and for an issue it matters
/// more than for a pull, because the number the page would show is minted by
/// that same ingester and does not exist until then.
pub(in crate::cmd) fn note_lag() {
    crate::term::say::note!(
        Index,
        "Tangled indexes records off the firehose; give it a moment to appear."
    );
}

// ---------------------------------------------------------------------------
// Naming an issue, from a command that is about to write
// ---------------------------------------------------------------------------

/// The account a command acts as, and the issue it was told to act on.
///
/// Every writer wants both, in this order: the account first, because it is
/// announced before anything is read and because it is the default answer to
/// "whose record key is this". A separate function only so that the four
/// commands cannot drift on which of the two they settle first — that order
/// is what makes `--dry-run` able to say who the write would be from.
async fn acting_on(
    reference: Option<&str>,
    author: Option<&str>,
    verb: &str,
) -> Result<(crate::config::account::Selection, IssueRef)> {
    let selection = crate::config::account::select().await?;
    selection.announce();
    let Some(reference) = reference else {
        return Err(crate::exit::fail(
            crate::exit::Exit::Usage,
            format!(
                "which issue? pass its record key or at:// URI, as in \
                 `atgc issue {verb} 3msg7w7l6hs2x`\n\
                 atgc does not guess from the current branch: an issue record names no \
                 branch, so there would be nothing to guess from"
            ),
        ));
    };
    let assumed = match author {
        Some(author) => crate::config::account::actor_did(author).await?,
        None => selection.did.clone(),
    };
    let target = read::classify_issue_ref(reference, &assumed)?;
    Ok((selection, target))
}

// ---------------------------------------------------------------------------
// `issue close`, `issue reopen`
// ---------------------------------------------------------------------------

#[derive(clap::Args, Debug)]
pub(crate) struct StateArgs {
    /// The issue: its record key or at:// URI
    #[arg(value_name = "ISSUE")]
    pub issue: Option<String>,
    /// The same, as a flag, for symmetry with the other verbs
    #[arg(long = "issue", value_name = "ISSUE", conflicts_with = "issue")]
    pub issue_flag: Option<String>,
    /// Whose issue it is, when a bare record key is ambiguous
    #[arg(long, value_name = "HANDLE|DID")]
    pub author: Option<String>,
    /// Say what would change without writing anything
    #[arg(long)]
    pub dry_run: bool,
    /// Print one JSON object describing what was written (or, with
    /// --dry-run, what would be) instead of the summary lines
    #[arg(long)]
    pub json: bool,
}

impl StateArgs {
    /// The issue this names, whichever spelling was used. The positional and
    /// `--issue` are declared `conflicts_with` each other, so clap has
    /// already refused naming one issue twice.
    fn issue(&self) -> Option<&str> {
        self.issue.as_deref().or(self.issue_flag.as_deref())
    }
}

pub(crate) async fn close(args: StateArgs) -> Result<()> {
    set_state(args, IssueState::Closed).await
}

pub(crate) async fn reopen(args: StateArgs) -> Result<()> {
    set_state(args, IssueState::Open).await
}

/// Move an issue to `wanted` by appending a state record.
///
/// Appending is the whole mechanism: `sh.tangled.repo.issue.state` records
/// are a log keyed by TID, nothing looks one up by key, and the appview
/// recomputes an issue's state as the newest record. So reopening is a new
/// `open` record rather than a delete of the `closed` one or a put over it —
/// which is also what Tangled's own web UI does, whose `writeIssueStateRecord`
/// creates a record with a fresh TID for close and reopen alike.
async fn set_state(args: StateArgs, wanted: IssueState) -> Result<()> {
    crate::term::jsonout::init(args.json);
    // The word a user reads at the moment something has refused to happen,
    // so it is the word they typed rather than the state's label.
    let verb = match wanted {
        IssueState::Open => "reopen",
        IssueState::Closed => "close",
    };
    let (selection, target) = acting_on(args.issue(), args.author.as_deref(), verb).await?;
    let acting = selection.did.clone();

    let issue = read::fetch_issue(&target).await?;
    let standing = standing_of(&acting, &issue.author, issue.repo_did()).await?;
    if standing == Standing::Neither {
        // `Denied`, not an unclassified 1: there is a session and this is a
        // refusal to act on somebody else's record, which docs/output.md
        // spells as 4 — the status a caller branches on to try another
        // account rather than to retry.
        return Err(crate::exit::fail(
            crate::exit::Exit::Denied,
            format!(
                "{} did not file this issue and does not own the repo it is on, so a state \
                 record it writes would be dropped\n\
                 the issue is {}'s; Tangled honours a state record from the issue's author \
                 or the repo's owner\n\
                 a collaborator with push access can {verb} it through tangled.org: atgc \
                 cannot read knot collaborator lists, so it does not pretend the write would \
                 land",
                selection.display(),
                issue.author,
            ),
        ));
    }

    let current = read::state_of(&issue, Some(&acting)).await?;
    let mut report = StateChangeJson {
        dry_run: args.dry_run,
        uri: None,
        issue_uri: issue.uri.clone(),
        rkey: issue.rkey.clone(),
        title: issue.title().to_string(),
        author_did: issue.author.clone(),
        state_before: current.known(),
        state_after: wanted.label().to_string(),
        changed: false,
        url: issue.repo_did().map(read::issues_url),
    };

    if !args.json {
        println!("issue:  {} ({})", issue.title(), issue.rkey);
        println!(
            "author: {}",
            crate::term::hyperlink::account(
                crate::clients::atproto::handles::handle(&issue.author)
                    .await
                    .as_deref(),
                &issue.author,
            )
        );
    }

    // Only a *known* state can make this a no-op. An unsettled one is not
    // evidence that the issue already reads the way this would leave it, and
    // treating it as such would silently decline to close an open issue.
    if current == State::Known(wanted) {
        if args.json {
            return crate::term::jsonout::emit(&report);
        }
        println!(
            "state:  {} (already {}; nothing written)",
            wanted.label(),
            wanted.label()
        );
        return Ok(());
    }

    report.changed = true;
    if !args.json {
        println!("state:  {} -> {}", current.label(), wanted.label());
    }
    let scope_check = || {
        scope::require_scope(
            crate::cmd::acting_scope(&acting).as_deref(),
            selection.handle.as_deref(),
            ISSUE_STATE_NSID,
        )
    };
    if args.dry_run {
        if let Err(e) = scope_check() {
            crate::term::say::warning!(Auth, "would fail: {e}");
        }
        if args.json {
            return crate::term::jsonout::emit(&report);
        }
        println!("dry run; nothing sent");
        return Ok(());
    }
    scope_check()?;

    let record: IssueStateRecord = IssueStateRecord {
        issue: AtUri::new(issue.uri.clone().into())?,
        state: StateState::from_value(wanted.token().into()),
        created_at: Datetime::now(),
        extra_data: None,
    };
    crate::logging::debug::log(format!(
        ">> createRecord {ISSUE_STATE_NSID}\n{}",
        crate::logging::debug::pretty(&record)
    ));
    let agent = auth::agent_for_did(&acting).await?;
    let output = agent.create_record(record, None).await.map_err(|e| {
        crate::logging::debug::dump_err("createRecord error", &e);
        scope::scope_refusal(
            "writing the issue state record",
            selection.handle.as_deref(),
            &e.to_string(),
        )
    })?;

    if args.json {
        report.uri = Some(output.uri.to_string());
        crate::term::jsonout::emit(&report)?;
        note_lag();
        return Ok(());
    }
    println!("wrote   {}", output.uri);
    if let Some(url) = &report.url {
        println!("view:   {}", crate::term::hyperlink::url(url));
    }
    note_lag();
    Ok(())
}

// ---------------------------------------------------------------------------
// `issue edit`
// ---------------------------------------------------------------------------

#[derive(clap::Args, Debug)]
pub(crate) struct EditArgs {
    /// The issue: its record key or at:// URI
    #[arg(value_name = "ISSUE")]
    pub issue: Option<String>,
    /// The same, as a flag, for symmetry with the other verbs
    #[arg(long = "issue", value_name = "ISSUE", conflicts_with = "issue")]
    pub issue_flag: Option<String>,
    /// New title
    #[arg(short, long)]
    pub title: Option<String>,
    /// New body; it cannot be emptied
    #[arg(short, long)]
    pub body: Option<String>,
    /// New body read from a file, or from stdin with `-`
    #[arg(long, conflicts_with = "body")]
    pub body_file: Option<String>,
    /// Say what would change without writing anything
    #[arg(long)]
    pub dry_run: bool,
    /// Print one JSON object describing what was written (or, with
    /// --dry-run, what would be) instead of the summary lines
    #[arg(long)]
    pub json: bool,
}

impl EditArgs {
    fn issue(&self) -> Option<&str> {
        self.issue.as_deref().or(self.issue_flag.as_deref())
    }
}

/// Change the title and/or body of an issue you filed.
///
/// The record lives in its author's PDS and there is no way to write to
/// somebody else's, so unlike `issue close` this really is author-only. Every
/// other field rides through untouched: the record is read back, at most two
/// fields are replaced, and the whole thing is put again under the CID it was
/// read at.
pub(crate) async fn edit(args: EditArgs) -> Result<()> {
    crate::term::jsonout::init(args.json);
    // No `--author`: an edit can only ever be of your own record, so a flag
    // naming somebody else's would exist only to be refused.
    let (selection, target) = acting_on(args.issue(), None, "edit").await?;
    let acting = selection.did.clone();
    if target.author != acting {
        return Err(crate::exit::fail(
            crate::exit::Exit::Denied,
            format!(
                "that issue belongs to {}, and its record lives in that account's PDS: \
                 {} cannot write to it\n\
                 only an issue's author can edit its title or body; `atgc issue close` is the \
                 one that also works on somebody else's issue, because the record it writes is \
                 your own",
                target.author,
                selection.display(),
            ),
        ));
    }

    let body = new_body(args.body, args.body_file)?;
    if args.title.is_none() && body.is_none() {
        return Err(crate::exit::fail(
            crate::exit::Exit::Usage,
            "nothing to change: pass --title, --body or --body-file",
        ));
    }
    // A body may be replaced and not emptied, for the reason `issue create`
    // needs one at all — an update tangled.org refuses is dropped, so the
    // appview would go on showing the paragraph this just deleted from the
    // PDS, and the two would disagree with nothing saying so.
    if body == Some(None) {
        return Err(empty_body_refusal(
            "an issue's body cannot be emptied; pass the text it should say instead",
        ));
    }

    let issue = read::fetch_issue(&target).await?;
    let rkey_key = Key::any_owned(&issue.rkey)
        .map_err(|e| anyhow::anyhow!("bad record key {}: {e}", issue.rkey))?;
    // Parsed into the generated type rather than edited as JSON, so a
    // property the lexicon defines and atgc names no field for — `mentions`,
    // `references`, `blobs` — rides through `extra_data` and is written back
    // untouched. Editing the untyped value would have worked too; parsing is
    // what makes the lexicon check below possible.
    let mut record: Issue = serde_json::from_value(issue.value.clone())
        .with_context(|| format!("issue record {} did not parse", issue.rkey))?;

    // Scanned before anything is compared, so a body naming a file that does
    // not exist refuses by name rather than after a read-modify-write.
    let images = match body.as_ref().and_then(Option::as_deref) {
        Some(text) => crate::cmd::images::scan(text)?,
        None => crate::cmd::images::Images::none(),
    };

    let mut report = EditedJson {
        dry_run: args.dry_run,
        uri: issue.uri.clone(),
        rkey: issue.rkey.clone(),
        title: record.title.to_string(),
        title_changed: false,
        body_changed: false,
        changed: false,
        images: images.listed(),
    };

    if !args.json {
        println!("issue:  {} ({})", record.title, issue.rkey);
        for line in images.describe() {
            println!("image:  {line}");
        }
    }

    let mut changed = false;
    if let Some(title) = args.title {
        let title = title.trim().to_string();
        if title.is_empty() {
            return Err(crate::exit::fail(
                crate::exit::Exit::Usage,
                "an issue needs a title; --title cannot be emptied",
            ));
        }
        if title.as_str() != record.title.as_str() {
            if !args.json {
                println!("title:  {} -> {title}", record.title);
            }
            report.title = title.clone();
            report.title_changed = true;
            record.title = title.into();
            changed = true;
        }
    }
    if let Some(body) = body {
        let stored = record.body.as_ref().map(ToString::to_string);
        // Equal text alone is not a no-op when local images are attached:
        // the stored copy holds the paths verbatim, and resending it is
        // exactly what uploads them and rewrites the paths to `blob+at://`
        // URIs. `pr edit`'s `body_edit_is_a_change` documents the same
        // clause, and dropping it here would print "nothing to change" and
        // leave the broken local paths live.
        if body != stored || !images.is_empty() {
            if !args.json {
                println!(
                    "body:   {} -> {}",
                    describe_body(stored.as_deref()),
                    describe_body(body.as_deref())
                );
            }
            report.body_changed = true;
            record.body = body.map(Into::into);
            changed = true;
        }
    }
    report.changed = changed;
    if !changed {
        // Nothing written, and the command still exits 0 — `changed: false`
        // is what says so.
        if args.json {
            return crate::term::jsonout::emit(&report);
        }
        println!("nothing to change; the record already says this");
        return Ok(());
    }
    // The same scope the create needed, asked for the same reason and at the
    // same point: an `issue edit` is a write to `sh.tangled.repo.issue`, and
    // a session granted before that collection was on the list is refused by
    // the PDS with a bare 403 that names nothing. This was the one writer in
    // the family that went straight to `putRecord` and found out there.
    let scope_check = || {
        scope::require_scope(
            crate::cmd::acting_scope(&acting).as_deref(),
            selection.handle.as_deref(),
            ISSUE_NSID,
        )
    };
    // Above the dry-run return: the edited record is complete by here, so a
    // dry run that skipped this could say "would work" for a record the
    // lexicon refuses. Unlike the scope check beside it this is a refusal
    // even under `--dry-run`, because it is a fact about the record the
    // caller asked for rather than about the session it would be sent with.
    record
        .validate()
        .map_err(|e| anyhow::anyhow!("{e}"))
        .context("issue record would be refused by the lexicon")?;
    if args.dry_run {
        if let Err(e) = scope_check() {
            crate::term::say::warning!(Auth, "would fail: {e}");
        }
        if args.json {
            return crate::term::jsonout::emit(&report);
        }
        println!("dry run; nothing sent");
        return Ok(());
    }
    scope_check()?;

    let agent = auth::agent_for_did(&acting).await?;

    // Uploaded only once the write is going to happen, and merged into
    // whatever `blobs` the record already carried: an earlier edit's images
    // are still referenced by the parts of the body this one did not touch,
    // and a blob no record references is one the PDS may collect.
    if !images.is_empty() {
        let text = record
            .body
            .as_ref()
            .map(ToString::to_string)
            .unwrap_or_default();
        let (rewritten, new_blobs) =
            crate::cmd::images::upload_and_rewrite(&agent, &acting, &text, &images).await?;
        record.body = Some(rewritten.into());
        let existing = record
            .blobs
            .take()
            .map(|bs| bs.into_iter().map(Into::into).collect());
        record.blobs = crate::cmd::images::merge_blobs(existing, new_blobs)
            .map(|bs| bs.into_iter().map(Into::into).collect());
    }

    // The second check, for the reason `create`'s is: the upload above
    // rewrote `body` and filled `blobs`, and those are the two fields the
    // check before the dry-run return could not have seen.
    record
        .validate()
        .map_err(|e| anyhow::anyhow!("{e}"))
        .context("issue record would be refused by the lexicon")?;
    crate::logging::debug::log(format!(
        ">> putRecord {ISSUE_NSID} rkey={} swapRecord={}\n{}",
        issue.rkey,
        issue.cid.as_deref().unwrap_or("(none)"),
        crate::logging::debug::pretty(&record)
    ));
    let uri = crate::clients::atproto::record::put(
        &agent,
        "issue",
        &acting,
        rkey_key,
        &record,
        issue.cid.as_deref(),
    )
    .await?;
    if args.json {
        return crate::term::jsonout::emit(&report);
    }
    println!("updated {uri}");
    if let Some(repo) = issue.repo_did() {
        println!(
            "view:   {}",
            crate::term::hyperlink::url(&read::issues_url(repo))
        );
    }
    Ok(())
}

// ---------------------------------------------------------------------------
// `issue comment`
// ---------------------------------------------------------------------------

#[derive(clap::Args, Debug)]
pub(crate) struct CommentArgs {
    /// The issue: its record key or at:// URI
    #[arg(value_name = "ISSUE")]
    pub issue: Option<String>,
    /// The same, as a flag, for symmetry with the other verbs
    #[arg(long = "issue", value_name = "ISSUE", conflicts_with = "issue")]
    pub issue_flag: Option<String>,
    /// What to say
    #[arg(short, long)]
    pub body: Option<String>,
    /// The same, read from a file, or from stdin with `-`
    #[arg(long, conflicts_with = "body")]
    pub body_file: Option<String>,
    /// Whose issue it is, when a bare record key is ambiguous
    #[arg(long, value_name = "HANDLE|DID")]
    pub author: Option<String>,
    /// Say what would be written without writing anything
    #[arg(long)]
    pub dry_run: bool,
    /// Print one JSON object describing what was written (or, with
    /// --dry-run, what would be) instead of the summary lines
    #[arg(long)]
    pub json: bool,
}

impl CommentArgs {
    fn issue(&self) -> Option<&str> {
        self.issue.as_deref().or(self.issue_flag.as_deref())
    }
}

/// Comment on an issue.
///
/// # This writes `sh.tangled.feed.comment`, not `sh.tangled.repo.issue.comment`
///
/// The obvious record is the wrong one and the failure is silent, so it is
/// worth being explicit. `lexicons/sh/tangled/repo/issue/comment.json` is
/// vendored in this tree and looks entirely current. It is not: Tangled
/// unified issue, pull and string comments into `sh.tangled.feed.comment`,
/// and the appview's ingester now treats the old collection as deletes only —
/// `// no-op. sh.tangled.repo.issue.comment is deprecated`. A comment written
/// there would be accepted by the PDS, would federate, and would never appear
/// anywhere. See [`crate::lexicon::tangled::LEGACY_ISSUE_COMMENT_NSID`].
///
/// The new record carries a `subject` strongRef where the old one had a bare
/// at-uri, and the CID in it is not optional: the appview refuses a comment
/// whose `subject.cid` does not parse.
///
/// `pullRoundIdx` is left unset, which is the one difference from
/// `pr comment`. The field is documented as "required when subject is
/// sh.tangled.repo.pull" and the appview enforces exactly that and only that
/// — an issue has no rounds to index into.
///
/// # Anyone may comment
///
/// Unlike `issue close`, this needs no standing check, and the difference is
/// real rather than an oversight. The appview's `ingestComment` performs no
/// ACL lookup of any kind: it unmarshals, validates the shape, resolves
/// mentions and writes. Comments are public discourse; state records are a
/// permission.
pub(crate) async fn comment(args: CommentArgs) -> Result<()> {
    crate::term::jsonout::init(args.json);
    let reference = args.issue().map(str::to_string);
    let (selection, target) =
        acting_on(reference.as_deref(), args.author.as_deref(), "comment").await?;
    let acting = selection.did.clone();
    // Read the body before touching the network. A missing --body is the
    // likeliest mistake, and finding out after three round trips is worse.
    let body = comment_body(args.body, args.body_file)?;
    // And scan it on the same principle: a local image path naming no file,
    // a non-image or an oversized one refuses here, by name.
    let images = crate::cmd::images::scan(&body)?;

    let issue = read::fetch_issue(&target).await?;
    // Required by the lexicon and enforced by the appview, and the strongRef
    // is where it comes from. An issue record read back without a CID cannot
    // be commented on: `subject.cid` has no default, and the appview rejects
    // a comment whose subject CID does not parse.
    let cid = issue.cid.clone().with_context(|| {
        format!(
            "the PDS returned issue record {} without a CID, and a comment's subject needs one",
            issue.rkey
        )
    })?;

    let mut report = CommentedJson {
        dry_run: args.dry_run,
        uri: None,
        issue_uri: issue.uri.clone(),
        issue_rkey: issue.rkey.clone(),
        title: issue.title().to_string(),
        author_did: issue.author.clone(),
        body_bytes: body.len(),
        url: issue.repo_did().map(read::issues_url),
        images: images.listed(),
    };

    if !args.json {
        println!("issue:  {} ({})", issue.title(), issue.rkey);
        println!(
            "author: {}",
            crate::term::hyperlink::account(
                crate::clients::atproto::handles::handle(&issue.author)
                    .await
                    .as_deref(),
                &issue.author,
            )
        );
        println!("body:   {} bytes", body.len());
        for line in images.describe() {
            println!("image:  {line}");
        }
    }

    // Asked before the write rather than only translated after it. Every
    // session made before `pr comment` shipped is missing this scope — it is
    // the one collection atgc's scope list got wrong — so this refuses first
    // rather than spending a write to find out.
    let scope_check = || {
        scope::require_scope(
            crate::cmd::acting_scope(&acting).as_deref(),
            selection.handle.as_deref(),
            crate::lexicon::tangled::FEED_COMMENT_NSID,
        )
    };
    if args.dry_run {
        if let Err(e) = scope_check() {
            crate::term::say::warning!(Auth, "would fail: {e}");
        }
        if args.json {
            return crate::term::jsonout::emit(&report);
        }
        println!("dry run; nothing sent");
        return Ok(());
    }
    scope_check()?;

    let subject: StrongRef = StrongRef {
        uri: AtUri::new(issue.uri.clone().into())?,
        cid: Cid::new(cid.as_bytes())?,
        extra_data: None,
    };
    let agent = auth::agent_for_did(&acting).await?;
    // The record keeps the *rewritten* body, so a later reader and a later
    // edit both see the `blob+at://` spelling: the local path stopped
    // meaning anything the moment it left this machine.
    let body = match images.is_empty() {
        true => crate::lexicon::tangled::markdown_plain(body),
        false => {
            let (rewritten, blobs) =
                crate::cmd::images::upload_and_rewrite(&agent, &acting, &body, &images).await?;
            crate::lexicon::tangled::markdown_with_blobs(
                rewritten,
                crate::cmd::images::merge_blobs(None, blobs),
            )
        }
    };
    let record = FeedComment::new()
        .subject(subject)
        .body(body)
        .created_at(Datetime::now())
        .build();

    crate::logging::debug::log(format!(
        ">> createRecord {}\n{}",
        crate::lexicon::tangled::FEED_COMMENT_NSID,
        crate::logging::debug::pretty(&record)
    ));
    let output = agent.create_record(record, None).await.map_err(|e| {
        crate::logging::debug::dump_err("createRecord error", &e);
        scope::scope_refusal(
            "creating the comment record",
            selection.handle.as_deref(),
            &e.to_string(),
        )
    })?;

    if args.json {
        report.uri = Some(output.uri.to_string());
        crate::term::jsonout::emit(&report)?;
        note_lag();
        return Ok(());
    }
    println!("wrote   {}", output.uri);
    if let Some(url) = &report.url {
        println!("view:   {}", crate::term::hyperlink::url(url));
    }
    note_lag();
    Ok(())
}

#[cfg(test)]
mod tests {
    use super::{comment_body, describe_body, new_body};

    /// `issue edit` has three states for the body and they are all different:
    /// not mentioned, set to something, and emptied. Collapsing the last two
    /// would make `--body ''` a no-op and leave a wrong body in place.
    #[test]
    fn a_body_can_be_set_cleared_or_left_alone() {
        assert_eq!(new_body(None, None).unwrap(), None);
        assert_eq!(
            new_body(Some("new text".to_string()), None).unwrap(),
            Some(Some("new text".to_string()))
        );
        // Empty, or only whitespace, clears it.
        assert_eq!(new_body(Some(String::new()), None).unwrap(), Some(None));
        assert_eq!(
            new_body(Some("  \n ".to_string()), None).unwrap(),
            Some(None)
        );

        let err = new_body(Some("a".to_string()), Some("f".to_string()))
            .expect_err("two sources for one field")
            .to_string();
        assert!(err.contains("not both"), "{err}");
    }

    /// An empty comment is refused here rather than at the appview, which
    /// would drop it at ingest without telling anyone. Unlike a body, empty
    /// has no second meaning: there is no comment to clear.
    #[test]
    fn an_empty_comment_is_refused_and_a_missing_one_says_where_to_put_it() {
        for empty in ["", "   ", "\n\t \n"] {
            let err = comment_body(Some(empty.to_string()), None)
                .expect_err("empty comment")
                .to_string();
            assert!(err.contains("needs a body"), "{empty:?} gave {err}");
        }
        // Whitespace *around* real text is kept: markdown is
        // whitespace-sensitive and trimming somebody's code fence is not this
        // function's call to make.
        assert_eq!(
            comment_body(Some("\n  hi\n".to_string()), None).unwrap(),
            "\n  hi\n"
        );

        // No body at all is a different error from an empty one, and it is
        // the one that has to mention `--body-file -`, since the reason a
        // user hits it is usually that they went looking for the editor there
        // isn't.
        let err = comment_body(None, None)
            .expect_err("nothing to say")
            .to_string();
        assert!(err.contains("--body-file"), "{err}");
        assert!(err.contains("editor"), "{err}");
    }

    /// The refusal an empty issue body earns has to name the appview's own
    /// error, because that string is the whole evidence for a rule the
    /// lexicon contradicts — a reader who checks `issue.json` finds `body`
    /// optional and would otherwise conclude atgc invented the requirement.
    #[test]
    fn the_empty_body_refusal_cites_the_appview_and_exits_two() {
        let err = super::empty_body_refusal("an issue needs a body");
        assert_eq!(crate::exit::classify(&err), crate::exit::Exit::Usage);
        let message = err.to_string();
        assert!(message.contains("an issue needs a body"), "{message}");
        assert!(message.contains("issue body is empty"), "{message}");
        assert!(message.contains("federate, and never appear"), "{message}");
    }

    /// A body is reported by size rather than echoed: it is arbitrarily long
    /// and the line's job is to say that it moved.
    #[test]
    fn a_body_change_is_described_not_printed() {
        assert_eq!(describe_body(None), "(none)");
        assert_eq!(describe_body(Some("hello")), "5 bytes");
    }
}